[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

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;
   }
 

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.