[PM] Improve the API and comments around the analysis manager proxies.

These are really handles that ensure the analyses get cleared at
appropriate places, and as such copying doesn't really make sense.
Instead, they should look more like unique ownership objects. Make that
the case.

Relatedly, if you create a temporary of one and move out of it
its destructor shouldn't actually clear anything. I don't think there is
any code that can trigger this currently, but it seems like a more
robust implementation.

If folks want, I can add a unittest that forces this to be exercised,
but that seems somewhat pointless -- whether a temporary is ever created
in the innards of AnalysisManager is not really something we should be
adding a reliance on, but I didn't want to leave a timebomb in the code
here.

If anyone has a cleaner way to represent this, I'm all ears, but
I wanted to assure myself that this wasn't in fact responsible for
another bug I'm chasing down (it wasn't) and figured I'd commit that.

llvm-svn: 261594

llvm/include/llvm/Analysis/CGSCCPassManager.h

@@ -49,17 +49,26 @@ typedef AnalysisManager<LazyCallGraph::SCC> CGSCCAnalysisManager;
 /// never use a CGSCC analysis manager from within (transitively) a module
 /// pass manager unless your parent module pass has received a proxy result
 /// object for it.
+///
+/// Note that the proxy's result is a move-only object and represents ownership
+/// of the validity of the analyses in the \c CGSCCAnalysisManager it provides.
 class CGSCCAnalysisManagerModuleProxy {
 public:
   class Result {
   public:
     explicit Result(CGSCCAnalysisManager &CGAM) : CGAM(&CGAM) {}
-    // We have to explicitly define all the special member functions because
-    // MSVC refuses to generate them.
-    Result(const Result &Arg) : CGAM(Arg.CGAM) {}
-    Result(Result &&Arg) : CGAM(std::move(Arg.CGAM)) {}
-    Result &operator=(Result RHS) {
-      std::swap(CGAM, RHS.CGAM);
+    Result(Result &&Arg) : CGAM(std::move(Arg.CGAM)) {
+      // We have to null out the analysis manager in the moved-from state
+      // because we are taking ownership of its responsibilty to clear the
+      // analysis state.
+      Arg.CGAM = nullptr;
+    }
+    Result &operator=(Result &&RHS) {
+      CGAM = RHS.CGAM;
+      // We have to null out the analysis manager in the moved-from state
+      // because we are taking ownership of its responsibilty to clear the
+      // analysis state.
+      RHS.CGAM = nullptr;
       return *this;
     }
     ~Result();
@@ -275,17 +284,27 @@ createModuleToPostOrderCGSCCPassAdaptor(CGSCCPassT Pass) {
 /// never use a function analysis manager from within (transitively) a CGSCC
 /// pass manager unless your parent CGSCC pass has received a proxy result
 /// object for it.
+///
+/// Note that the proxy's result is a move-only object and represents ownership
+/// of the validity of the analyses in the \c FunctionAnalysisManager it
+/// provides.
 class FunctionAnalysisManagerCGSCCProxy {
 public:
   class Result {
   public:
     explicit Result(FunctionAnalysisManager &FAM) : FAM(&FAM) {}
-    // We have to explicitly define all the special member functions because
-    // MSVC refuses to generate them.
-    Result(const Result &Arg) : FAM(Arg.FAM) {}
-    Result(Result &&Arg) : FAM(std::move(Arg.FAM)) {}
-    Result &operator=(Result RHS) {
-      std::swap(FAM, RHS.FAM);
+    Result(Result &&Arg) : FAM(std::move(Arg.FAM)) {
+      // We have to null out the analysis manager in the moved-from state
+      // because we are taking ownership of the responsibilty to clear the
+      // analysis state.
+      Arg.FAM = nullptr;
+    }
+    Result &operator=(Result &&RHS) {
+      FAM = RHS.FAM;
+      // We have to null out the analysis manager in the moved-from state
+      // because we are taking ownership of the responsibilty to clear the
+      // analysis state.
+      RHS.FAM = nullptr;
       return *this;
     }
     ~Result();

llvm/include/llvm/IR/PassManager.h

@@ -618,6 +618,10 @@ typedef AnalysisManager<Function> FunctionAnalysisManager;
 /// never use a function analysis manager from within (transitively) a module
 /// pass manager unless your parent module pass has received a proxy result
 /// object for it.
+///
+/// Note that the proxy's result is a move-only object and represents ownership
+/// of the validity of the analyses in the \c FunctionAnalysisManager it
+/// provides.
 class FunctionAnalysisManagerModuleProxy {
 public:
   class Result;
@@ -665,12 +669,18 @@ private:
 class FunctionAnalysisManagerModuleProxy::Result {
 public:
   explicit Result(FunctionAnalysisManager &FAM) : FAM(&FAM) {}
-  // We have to explicitly define all the special member functions because MSVC
-  // refuses to generate them.
-  Result(const Result &Arg) : FAM(Arg.FAM) {}
-  Result(Result &&Arg) : FAM(std::move(Arg.FAM)) {}
-  Result &operator=(Result RHS) {
-    std::swap(FAM, RHS.FAM);
+  Result(Result &&Arg) : FAM(std::move(Arg.FAM)) {
+    // We have to null out the analysis manager in the moved-from state
+    // because we are taking ownership of the responsibilty to clear the
+    // analysis state.
+    Arg.FAM = nullptr;
+  }
+  Result &operator=(Result &&RHS) {
+    FAM = RHS.FAM;
+    // We have to null out the analysis manager in the moved-from state
+    // because we are taking ownership of the responsibilty to clear the
+    // analysis state.
+    RHS.FAM = nullptr;
     return *this;
   }
   ~Result();

llvm/lib/Analysis/CGSCCPassManager.cpp

@@ -22,6 +22,10 @@ CGSCCAnalysisManagerModuleProxy::run(Module &M) {
 }
 
 CGSCCAnalysisManagerModuleProxy::Result::~Result() {
+  // CGAM is cleared in a moved from state where there is nothing to do.
+  if (!CGAM)
+    return;
+
   // Clear out the analysis manager if we're being destroyed -- it means we
   // didn't even see an invalidate call when we got invalidated.
   CGAM->clear();
@@ -51,6 +55,10 @@ FunctionAnalysisManagerCGSCCProxy::run(LazyCallGraph::SCC &C) {
 }
 
 FunctionAnalysisManagerCGSCCProxy::Result::~Result() {
+  // FAM is cleared in a moved from state where there is nothing to do.
+  if (!FAM)
+    return;
+
   // Clear out the analysis manager if we're being destroyed -- it means we
   // didn't even see an invalidate call when we got invalidated.
   FAM->clear();

llvm/lib/IR/PassManager.cpp

@@ -22,6 +22,10 @@ FunctionAnalysisManagerModuleProxy::run(Module &M) {
 }
 
 FunctionAnalysisManagerModuleProxy::Result::~Result() {
+  // FAM is cleared in a moved from state where there is nothing to do.
+  if (!FAM)
+    return;
+
   // Clear out the analysis manager if we're being destroyed -- it means we
   // didn't even see an invalidate call when we got invalidated.
   FAM->clear();