commit 0a36864bf69609f86a3ccd3bd12e1ce98335799d
author Lang Hames <lhames@gmail.com> Sun Aug 07 11:45:06 2022 -0700
committer Copybara-Service <copybara-worker@google.com> Sun Aug 07 12:22:31 2022 -0700
tree 2b969d463bec6760021eda8d611397ad8f29e2fb
parent d1ee3c7e7fed2153a21dbfb1a33970142e8e1bff

Revert "[JITLink] Fix some C++17 related fixmes."

This reverts commit 6ea5bf436a983ea9e16a5fe7534c87beca0a61b7.

6ea5bf436a983ea9e16a5fe7534c87beca0a61b7 made use of new c++17 rules regarding
order of evaluation (specifically: in function calls the expression naming the
function should be sequenced before the evalution of any operands) to simplify
some continuation-passing calls. Unfortunately this appears to break at least
one MSVC bot: https://lab.llvm.org/buildbot/#/builders/123/builds/12149 .

Includes an update to the comments to note that the workaround is now based on
MSVC limitations, not on LLVM adopting c++17.

GitOrigin-RevId: 41c41fcbc036d75e6b6ea0756857099befbb3313

lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp

diff --git a/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp b/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
index 477f21d..7ad1a10 100644
--- a/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
+++ b/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
@@ -51,7 +51,11 @@
   Ctx->getMemoryManager().allocate(
       Ctx->getJITLinkDylib(), *G,
       [S = std::move(Self)](AllocResult AR) mutable {
-        S->linkPhase2(std::move(S), std::move(AR));
+        // FIXME: Once MSVC implements c++17 order of evaluation rules for calls
+        // this can be simplified to
+        //          S->linkPhase2(std::move(S), std::move(AR));
+        auto *TmpSelf = S.get();
+        TmpSelf->linkPhase2(std::move(S), std::move(AR));
       });
 }
 
@@ -87,7 +91,10 @@
       dbgs() << "No external symbols for " << G->getName()
              << ". Proceeding immediately with link phase 3.\n";
     });
-    Self->linkPhase3(std::move(Self), AsyncLookupResult());
+    // FIXME: Once MSVC implements c++17 order of evaluation rules for calls
+    // this can be simplified. See below.
+    auto &TmpSelf = *Self;
+    TmpSelf.linkPhase3(std::move(Self), AsyncLookupResult());
     return;
   }
 
@@ -99,11 +106,20 @@
 
   // We're about to hand off ownership of ourself to the continuation. Grab a
   // pointer to the context so that we can call it to initiate the lookup.
+  //
+  // FIXME: Once MSVC implements c++17 order of evaluation rules for calls this
+  // can be simplified to:
+  //
+  // Ctx->lookup(std::move(UnresolvedExternals),
+  //             [Self=std::move(Self)](Expected<AsyncLookupResult> Result) {
+  //               Self->linkPhase3(std::move(Self), std::move(Result));
+  //             });
   Ctx->lookup(std::move(ExternalSymbols),
               createLookupContinuation(
                   [S = std::move(Self)](
                       Expected<AsyncLookupResult> LookupResult) mutable {
-                    S->linkPhase3(std::move(S), std::move(LookupResult));
+                    auto &TmpSelf = *S;
+                    TmpSelf.linkPhase3(std::move(S), std::move(LookupResult));
                   }));
 }
 
@@ -148,7 +164,11 @@
     return abandonAllocAndBailOut(std::move(Self), std::move(Err));
 
   Alloc->finalize([S = std::move(Self)](FinalizeResult FR) mutable {
-    S->linkPhase4(std::move(S), std::move(FR));
+    // FIXME: Once MSVC implements c++17 order of evaluation rules for calls
+    // this can be simplified to
+    //          S->linkPhase2(std::move(S), std::move(AR));
+    auto *TmpSelf = S.get();
+    TmpSelf->linkPhase4(std::move(S), std::move(FR));
   });
 }