[llvm] [JITLink] Reformat code due to an strict order of evaluation in C++17 (PR #89472)

[Isaac David via llvm-commits]

https://github.com/orion160 updated https://github.com/llvm/llvm-project/pull/89472

>From bf63f0eff151e970980eaf59215fdbd3421fd9a9 Mon Sep 17 00:00:00 2001
From: orion <idbl64 at outlook.com>
Date: Fri, 19 Apr 2024 17:59:27 -0500
Subject: [PATCH 1/3] Reformat code due to an strict order of evaluation in
 C++17 (LLVM_REQUIRED_CXX_STANDARD=17)

---
 llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h
index e5d05e6b1b7bfc..f9aa73f51468c9 100644
--- a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h
+++ b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h
@@ -115,13 +115,7 @@ template <typename LinkerImpl> class JITLinker : public JITLinkerBase {
 
     // Ownership of the linker is passed into the linker's doLink function to
     // allow it to be passed on to async continuations.
-    //
-    // FIXME: Remove LTmp once we have c++17.
-    // C++17 sequencing rules guarantee that function name expressions are
-    // sequenced before arguments, so L->linkPhase1(std::move(L), ...) will be
-    // well formed.
-    auto &LTmp = *L;
-    LTmp.linkPhase1(std::move(L));
+    L->linkPhase1(std::move(L));
   }
 
 private:

>From 75c8bae5dabed5099240948f4b00786c516056b9 Mon Sep 17 00:00:00 2001
From: orion <idbl64 at outlook.com>
Date: Fri, 19 Apr 2024 19:16:47 -0500
Subject: [PATCH 2/3] complete c++17 ordering feature for JITLink Generic

---
 .../JITLink/JITLinkGeneric.cpp                | 40 +++++--------------
 1 file changed, 11 insertions(+), 29 deletions(-)

diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
index 01144763ac4ca5..01b2484684c218 100644
--- a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
+++ b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp
@@ -59,11 +59,7 @@ void JITLinkerBase::linkPhase1(std::unique_ptr<JITLinkerBase> Self) {
   Ctx->getMemoryManager().allocate(
       Ctx->getJITLinkDylib(), *G,
       [S = std::move(Self)](AllocResult AR) mutable {
-        // 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));
+        S->linkPhase2(std::move(S), std::move(AR));
       });
 }
 
@@ -99,10 +95,8 @@ void JITLinkerBase::linkPhase2(std::unique_ptr<JITLinkerBase> Self,
       dbgs() << "No external symbols for " << G->getName()
              << ". Proceeding immediately with link phase 3.\n";
     });
-    // 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());
+
+    Self->linkPhase3(std::move(Self), AsyncLookupResult());
     return;
   }
 
@@ -114,21 +108,13 @@ void JITLinkerBase::linkPhase2(std::unique_ptr<JITLinkerBase> Self,
 
   // 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 {
-                    auto &TmpSelf = *S;
-                    TmpSelf.linkPhase3(std::move(S), std::move(LookupResult));
-                  }));
+
+  Ctx->lookup(
+      std::move(ExternalSymbols),
+      createLookupContinuation(
+          [Self = std::move(Self)](Expected<AsyncLookupResult> Result) mutable {
+            Self->linkPhase3(std::move(Self), std::move(Result));
+          }));
 }
 
 void JITLinkerBase::linkPhase3(std::unique_ptr<JITLinkerBase> Self,
@@ -178,11 +164,7 @@ void JITLinkerBase::linkPhase3(std::unique_ptr<JITLinkerBase> Self,
   }
 
   Alloc->finalize([S = std::move(Self)](FinalizeResult FR) mutable {
-    // 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));
+    S->linkPhase4(std::move(S), std::move(FR));
   });
 }
 

>From 5837682f94cdcb6e2bca1463256b85edccc643fa Mon Sep 17 00:00:00 2001
From: orion <idbl64 at outlook.com>
Date: Sun, 21 Apr 2024 12:07:40 -0500
Subject: [PATCH 3/3] Remove unnecesary comment

---
 llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h
index f9aa73f51468c9..db8325eba33cbb 100644
--- a/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h
+++ b/llvm/lib/ExecutionEngine/JITLink/JITLinkGeneric.h
@@ -112,9 +112,6 @@ template <typename LinkerImpl> class JITLinker : public JITLinkerBase {
   /// will be forwarded to the constructor.
   template <typename... ArgTs> static void link(ArgTs &&... Args) {
     auto L = std::make_unique<LinkerImpl>(std::forward<ArgTs>(Args)...);
-
-    // Ownership of the linker is passed into the linker's doLink function to
-    // allow it to be passed on to async continuations.
     L->linkPhase1(std::move(L));
   }