[PM] As was pointed out in review, I need to define a custom swap in

order to use the single assignment. That's probably worth doing for
a lot of these types anyways as they may have non-trivial moves and so
getting copy elision in more places seems worthwhile.

I've tried to add some tests that actually catch this mistake, and one
of the types is now well tested but the others' tests still fail to
catch this. I'll keep working on tests, but this gets the core pattern
right.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@203780 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/IR/PassManager.h b/include/llvm/IR/PassManager.h
index 5e5e6537f8afda54edfe94c3236579dc53e90baf..c6c530cce5edc4c1a4b4bbc98008133337b52d18 100644 (file)
--- a/include/llvm/IR/PassManager.h
+++ b/include/llvm/IR/PassManager.h
@@ -71,8 +71,12 @@ public:
       : PreservedPassIDs(Arg.PreservedPassIDs) {}
   PreservedAnalyses(PreservedAnalyses &&Arg)
       : PreservedPassIDs(std::move(Arg.PreservedPassIDs)) {}
+  friend void swap(PreservedAnalyses &LHS, PreservedAnalyses &RHS) {
+    using std::swap;
+    swap(LHS.PreservedPassIDs, RHS.PreservedPassIDs);
+  }
   PreservedAnalyses &operator=(PreservedAnalyses RHS) {
-    std::swap(*this, RHS);
+    swap(*this, RHS);
     return *this;
   }
 
@@ -212,8 +216,12 @@ struct PassModel<IRUnitT, AnalysisManagerT, PassT, true>
   // refuses to generate them.
   PassModel(const PassModel &Arg) : Pass(Arg.Pass) {}
   PassModel(PassModel &&Arg) : Pass(std::move(Arg.Pass)) {}
+  friend void swap(PassModel &LHS, PassModel &RHS) {
+    using std::swap;
+    swap(LHS.Pass, RHS.Pass);
+  }
   PassModel &operator=(PassModel RHS) {
-    std::swap(*this, RHS);
+    swap(*this, RHS);
     return *this;
   }
 
@@ -234,8 +242,12 @@ struct PassModel<IRUnitT, AnalysisManagerT, PassT, false>
   // refuses to generate them.
   PassModel(const PassModel &Arg) : Pass(Arg.Pass) {}
   PassModel(PassModel &&Arg) : Pass(std::move(Arg.Pass)) {}
+  friend void swap(PassModel &LHS, PassModel &RHS) {
+    using std::swap;
+    swap(LHS.Pass, RHS.Pass);
+  }
   PassModel &operator=(PassModel RHS) {
-    std::swap(*this, RHS);
+    swap(*this, RHS);
     return *this;
   }
 
@@ -306,8 +318,12 @@ struct AnalysisResultModel<IRUnitT, PassT, ResultT, false>
   AnalysisResultModel(const AnalysisResultModel &Arg) : Result(Arg.Result) {}
   AnalysisResultModel(AnalysisResultModel &&Arg)
       : Result(std::move(Arg.Result)) {}
+  friend void swap(AnalysisResultModel &LHS, AnalysisResultModel &RHS) {
+    using std::swap;
+    swap(LHS.Result, RHS.Result);
+  }
   AnalysisResultModel &operator=(AnalysisResultModel RHS) {
-    std::swap(*this, RHS);
+    swap(*this, RHS);
     return *this;
   }
 
@@ -334,8 +350,12 @@ struct AnalysisResultModel<IRUnitT, PassT, ResultT, true>
   AnalysisResultModel(const AnalysisResultModel &Arg) : Result(Arg.Result) {}
   AnalysisResultModel(AnalysisResultModel &&Arg)
       : Result(std::move(Arg.Result)) {}
+  friend void swap(AnalysisResultModel &LHS, AnalysisResultModel &RHS) {
+    using std::swap;
+    swap(LHS.Result, RHS.Result);
+  }
   AnalysisResultModel &operator=(AnalysisResultModel RHS) {
-    std::swap(*this, RHS);
+    swap(*this, RHS);
     return *this;
   }
 
@@ -382,8 +402,12 @@ struct AnalysisPassModel<IRUnitT, AnalysisManagerT, PassT, true>
   // refuses to generate them.
   AnalysisPassModel(const AnalysisPassModel &Arg) : Pass(Arg.Pass) {}
   AnalysisPassModel(AnalysisPassModel &&Arg) : Pass(std::move(Arg.Pass)) {}
+  friend void swap(AnalysisPassModel &LHS, AnalysisPassModel &RHS) {
+    using std::swap;
+    swap(LHS.Pass, RHS.Pass);
+  }
   AnalysisPassModel &operator=(AnalysisPassModel RHS) {
-    std::swap(*this, RHS);
+    swap(*this, RHS);
     return *this;
   }
 
@@ -412,8 +436,12 @@ struct AnalysisPassModel<IRUnitT, AnalysisManagerT, PassT, false>
   // refuses to generate them.
   AnalysisPassModel(const AnalysisPassModel &Arg) : Pass(Arg.Pass) {}
   AnalysisPassModel(AnalysisPassModel &&Arg) : Pass(std::move(Arg.Pass)) {}
+  friend void swap(AnalysisPassModel &LHS, AnalysisPassModel &RHS) {
+    using std::swap;
+    swap(LHS.Pass, RHS.Pass);
+  }
   AnalysisPassModel &operator=(AnalysisPassModel RHS) {
-    std::swap(*this, RHS);
+    swap(*this, RHS);
     return *this;
   }
 
@@ -809,7 +837,7 @@ public:
       : FAM(std::move(Arg.FAM)) {}
   FunctionAnalysisManagerModuleProxy &
   operator=(FunctionAnalysisManagerModuleProxy RHS) {
-    std::swap(*this, RHS);
+    std::swap(FAM, RHS.FAM);
     return *this;
   }
 
@@ -842,7 +870,7 @@ public:
   Result(const Result &Arg) : FAM(Arg.FAM) {}
   Result(Result &&Arg) : FAM(std::move(Arg.FAM)) {}
   Result &operator=(Result RHS) {
-    std::swap(*this, RHS);
+    std::swap(FAM, RHS.FAM);
     return *this;
   }
   ~Result();
@@ -889,7 +917,7 @@ public:
     Result(const Result &Arg) : MAM(Arg.MAM) {}
     Result(Result &&Arg) : MAM(std::move(Arg.MAM)) {}
     Result &operator=(Result RHS) {
-      std::swap(*this, RHS);
+      std::swap(MAM, RHS.MAM);
       return *this;
     }
 
@@ -915,7 +943,7 @@ public:
       : MAM(std::move(Arg.MAM)) {}
   ModuleAnalysisManagerFunctionProxy &
   operator=(ModuleAnalysisManagerFunctionProxy RHS) {
-    std::swap(*this, RHS);
+    std::swap(MAM, RHS.MAM);
     return *this;
   }
 
@@ -948,8 +976,12 @@ public:
       : Pass(Arg.Pass) {}
   ModuleToFunctionPassAdaptor(ModuleToFunctionPassAdaptor &&Arg)
       : Pass(std::move(Arg.Pass)) {}
+  friend void swap(ModuleToFunctionPassAdaptor &LHS, ModuleToFunctionPassAdaptor &RHS) {
+    using std::swap;
+    swap(LHS.Pass, RHS.Pass);
+  }
   ModuleToFunctionPassAdaptor &operator=(ModuleToFunctionPassAdaptor RHS) {
-    std::swap(*this, RHS);
+    swap(*this, RHS);
     return *this;
   }
 

diff --git a/unittests/IR/PassManagerTest.cpp b/unittests/IR/PassManagerTest.cpp
index a2d59d31a40de3ce85851c215bd7d2ee6b7136e5..310e48fc3b8e3f69d0fc72218888fe99e7f3e933 100644 (file)
--- a/unittests/IR/PassManagerTest.cpp
+++ b/unittests/IR/PassManagerTest.cpp
@@ -191,6 +191,39 @@ public:
                   "}\n")) {}
 };
 
+TEST_F(PassManagerTest, BasicPreservedAnalyses) {
+  PreservedAnalyses PA1 = PreservedAnalyses();
+  EXPECT_FALSE(PA1.preserved<TestFunctionAnalysis>());
+  EXPECT_FALSE(PA1.preserved<TestModuleAnalysis>());
+  PreservedAnalyses PA2 = PreservedAnalyses::none();
+  EXPECT_FALSE(PA2.preserved<TestFunctionAnalysis>());
+  EXPECT_FALSE(PA2.preserved<TestModuleAnalysis>());
+  PreservedAnalyses PA3 = PreservedAnalyses::all();
+  EXPECT_TRUE(PA3.preserved<TestFunctionAnalysis>());
+  EXPECT_TRUE(PA3.preserved<TestModuleAnalysis>());
+  PreservedAnalyses PA4 = PA1;
+  EXPECT_FALSE(PA4.preserved<TestFunctionAnalysis>());
+  EXPECT_FALSE(PA4.preserved<TestModuleAnalysis>());
+  PA4 = PA3;
+  EXPECT_TRUE(PA4.preserved<TestFunctionAnalysis>());
+  EXPECT_TRUE(PA4.preserved<TestModuleAnalysis>());
+  PA4 = std::move(PA2);
+  EXPECT_FALSE(PA4.preserved<TestFunctionAnalysis>());
+  EXPECT_FALSE(PA4.preserved<TestModuleAnalysis>());
+  PA4.preserve<TestFunctionAnalysis>();
+  EXPECT_TRUE(PA4.preserved<TestFunctionAnalysis>());
+  EXPECT_FALSE(PA4.preserved<TestModuleAnalysis>());
+  PA1.preserve<TestModuleAnalysis>();
+  EXPECT_FALSE(PA1.preserved<TestFunctionAnalysis>());
+  EXPECT_TRUE(PA1.preserved<TestModuleAnalysis>());
+  PA1.preserve<TestFunctionAnalysis>();
+  EXPECT_TRUE(PA1.preserved<TestFunctionAnalysis>());
+  EXPECT_TRUE(PA1.preserved<TestModuleAnalysis>());
+  PA1.intersect(PA4);
+  EXPECT_TRUE(PA1.preserved<TestFunctionAnalysis>());
+  EXPECT_FALSE(PA1.preserved<TestModuleAnalysis>());
+}
+
 TEST_F(PassManagerTest, Basic) {
   FunctionAnalysisManager FAM;
   int FunctionAnalysisRuns = 0;
@@ -209,10 +242,18 @@ TEST_F(PassManagerTest, Basic) {
   int AnalyzedInstrCount1 = 0;
   int AnalyzedFunctionCount1 = 0;
   {
+    // Pointless scoped copy to test move assignment.
+    ModulePassManager NestedMPM;
     FunctionPassManager FPM;
-    FPM.addPass(TestFunctionPass(FunctionPassRunCount1, AnalyzedInstrCount1,
-                                 AnalyzedFunctionCount1));
-    MPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
+    {
+      // Pointless scope to test move assignment.
+      FunctionPassManager NestedFPM;
+      NestedFPM.addPass(TestFunctionPass(FunctionPassRunCount1, AnalyzedInstrCount1,
+                                   AnalyzedFunctionCount1));
+      FPM = std::move(NestedFPM);
+    }
+    NestedMPM.addPass(createModuleToFunctionPassAdaptor(std::move(FPM)));
+    MPM = std::move(NestedMPM);
   }
 
   // Count the runs over a module.