[clang] [clang-repl] Expose RuntimeInterfaceBuilder to allow customization (PR #83126)

Stefan Gränitz via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 28 02:16:02 PST 2024


https://github.com/weliveindetail updated https://github.com/llvm/llvm-project/pull/83126

>From 3a6fdd006f00b0530e7fe6ead1fcd2765c1bfe07 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Mon, 26 Feb 2024 19:12:41 +0100
Subject: [PATCH 1/4] [clang-repl] Split out TypeVisitor from
 RuntimeInterfaceBuilder

---
 clang/lib/Interpreter/Interpreter.cpp | 171 ++++++++++++++------------
 1 file changed, 92 insertions(+), 79 deletions(-)

diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 9f97a3c6b0be9e..7afe55bbb266a3 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -541,21 +541,104 @@ bool Interpreter::FindRuntimeInterface() {
 
 namespace {
 
-class RuntimeInterfaceBuilder
-    : public TypeVisitor<RuntimeInterfaceBuilder, Interpreter::InterfaceKind> {
-  clang::Interpreter &Interp;
+class InterfaceKindVisitor
+    : public TypeVisitor<InterfaceKindVisitor, Interpreter::InterfaceKind> {
+  friend class RuntimeInterfaceBuilder;
+
   ASTContext &Ctx;
   Sema &S;
   Expr *E;
   llvm::SmallVector<Expr *, 3> Args;
 
+public:
+  InterfaceKindVisitor(ASTContext &Ctx, Sema &S, Expr *E)
+      : Ctx(Ctx), S(S), E(E) {}
+
+  Interpreter::InterfaceKind VisitRecordType(const RecordType *Ty) {
+    return Interpreter::InterfaceKind::WithAlloc;
+  }
+
+  Interpreter::InterfaceKind
+  VisitMemberPointerType(const MemberPointerType *Ty) {
+    return Interpreter::InterfaceKind::WithAlloc;
+  }
+
+  Interpreter::InterfaceKind
+  VisitConstantArrayType(const ConstantArrayType *Ty) {
+    return Interpreter::InterfaceKind::CopyArray;
+  }
+
+  Interpreter::InterfaceKind
+  VisitFunctionProtoType(const FunctionProtoType *Ty) {
+    HandlePtrType(Ty);
+    return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitPointerType(const PointerType *Ty) {
+    HandlePtrType(Ty);
+    return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitReferenceType(const ReferenceType *Ty) {
+    ExprResult AddrOfE = S.CreateBuiltinUnaryOp(SourceLocation(), UO_AddrOf, E);
+    assert(!AddrOfE.isInvalid() && "Can not create unary expression");
+    Args.push_back(AddrOfE.get());
+    return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitBuiltinType(const BuiltinType *Ty) {
+    if (Ty->isNullPtrType())
+      Args.push_back(E);
+    else if (Ty->isFloatingType())
+      Args.push_back(E);
+    else if (Ty->isIntegralOrEnumerationType())
+      HandleIntegralOrEnumType(Ty);
+    else if (Ty->isVoidType()) {
+      // Do we need to still run `E`?
+    }
+
+    return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+  Interpreter::InterfaceKind VisitEnumType(const EnumType *Ty) {
+    HandleIntegralOrEnumType(Ty);
+    return Interpreter::InterfaceKind::NoAlloc;
+  }
+
+private:
+  // Force cast these types to uint64 to reduce the number of overloads of
+  // `__clang_Interpreter_SetValueNoAlloc`.
+  void HandleIntegralOrEnumType(const Type *Ty) {
+    TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.UnsignedLongLongTy);
+    ExprResult CastedExpr =
+        S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
+    assert(!CastedExpr.isInvalid() && "Cannot create cstyle cast expr");
+    Args.push_back(CastedExpr.get());
+  }
+
+  void HandlePtrType(const Type *Ty) {
+    TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.VoidPtrTy);
+    ExprResult CastedExpr =
+        S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
+    assert(!CastedExpr.isInvalid() && "Can not create cstyle cast expression");
+    Args.push_back(CastedExpr.get());
+  }
+};
+
+class RuntimeInterfaceBuilder {
+  clang::Interpreter &Interp;
+  ASTContext &Ctx;
+  Sema &S;
+  Expr *E;
+  InterfaceKindVisitor Visitor;
+
 public:
   RuntimeInterfaceBuilder(clang::Interpreter &In, ASTContext &C, Sema &SemaRef,
                           Expr *VE, ArrayRef<Expr *> FixedArgs)
-      : Interp(In), Ctx(C), S(SemaRef), E(VE) {
+      : Interp(In), Ctx(C), S(SemaRef), E(VE), Visitor(C, SemaRef, VE) {
     // The Interpreter* parameter and the out parameter `OutVal`.
     for (Expr *E : FixedArgs)
-      Args.push_back(E);
+      Visitor.Args.push_back(E);
 
     // Get rid of ExprWithCleanups.
     if (auto *EWC = llvm::dyn_cast_if_present<ExprWithCleanups>(E))
@@ -575,11 +658,11 @@ class RuntimeInterfaceBuilder
     Expr *TypeArg =
         CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)Ty.getAsOpaquePtr());
     // The QualType parameter `OpaqueType`, represented as `void*`.
-    Args.push_back(TypeArg);
+    Visitor.Args.push_back(TypeArg);
 
     // We push the last parameter based on the type of the Expr. Note we need
     // special care for rvalue struct.
-    Interpreter::InterfaceKind Kind = Visit(&*DesugaredTy);
+    Interpreter::InterfaceKind Kind = Visitor.Visit(&*DesugaredTy);
     switch (Kind) {
     case Interpreter::InterfaceKind::WithAlloc:
     case Interpreter::InterfaceKind::CopyArray: {
@@ -587,7 +670,7 @@ class RuntimeInterfaceBuilder
       ExprResult AllocCall = S.ActOnCallExpr(
           /*Scope=*/nullptr,
           Interp.getValuePrintingInfo()[Interpreter::InterfaceKind::WithAlloc],
-          E->getBeginLoc(), Args, E->getEndLoc());
+          E->getBeginLoc(), Visitor.Args, E->getEndLoc());
       assert(!AllocCall.isInvalid() && "Can't create runtime interface call!");
 
       TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ty, SourceLocation());
@@ -634,82 +717,12 @@ class RuntimeInterfaceBuilder
       return S.ActOnCallExpr(
           /*Scope=*/nullptr,
           Interp.getValuePrintingInfo()[Interpreter::InterfaceKind::NoAlloc],
-          E->getBeginLoc(), Args, E->getEndLoc());
+          E->getBeginLoc(), Visitor.Args, E->getEndLoc());
     }
     default:
       llvm_unreachable("Unhandled Interpreter::InterfaceKind");
     }
   }
-
-  Interpreter::InterfaceKind VisitRecordType(const RecordType *Ty) {
-    return Interpreter::InterfaceKind::WithAlloc;
-  }
-
-  Interpreter::InterfaceKind
-  VisitMemberPointerType(const MemberPointerType *Ty) {
-    return Interpreter::InterfaceKind::WithAlloc;
-  }
-
-  Interpreter::InterfaceKind
-  VisitConstantArrayType(const ConstantArrayType *Ty) {
-    return Interpreter::InterfaceKind::CopyArray;
-  }
-
-  Interpreter::InterfaceKind
-  VisitFunctionProtoType(const FunctionProtoType *Ty) {
-    HandlePtrType(Ty);
-    return Interpreter::InterfaceKind::NoAlloc;
-  }
-
-  Interpreter::InterfaceKind VisitPointerType(const PointerType *Ty) {
-    HandlePtrType(Ty);
-    return Interpreter::InterfaceKind::NoAlloc;
-  }
-
-  Interpreter::InterfaceKind VisitReferenceType(const ReferenceType *Ty) {
-    ExprResult AddrOfE = S.CreateBuiltinUnaryOp(SourceLocation(), UO_AddrOf, E);
-    assert(!AddrOfE.isInvalid() && "Can not create unary expression");
-    Args.push_back(AddrOfE.get());
-    return Interpreter::InterfaceKind::NoAlloc;
-  }
-
-  Interpreter::InterfaceKind VisitBuiltinType(const BuiltinType *Ty) {
-    if (Ty->isNullPtrType())
-      Args.push_back(E);
-    else if (Ty->isFloatingType())
-      Args.push_back(E);
-    else if (Ty->isIntegralOrEnumerationType())
-      HandleIntegralOrEnumType(Ty);
-    else if (Ty->isVoidType()) {
-      // Do we need to still run `E`?
-    }
-
-    return Interpreter::InterfaceKind::NoAlloc;
-  }
-
-  Interpreter::InterfaceKind VisitEnumType(const EnumType *Ty) {
-    HandleIntegralOrEnumType(Ty);
-    return Interpreter::InterfaceKind::NoAlloc;
-  }
-
-private:
-  // Force cast these types to uint64 to reduce the number of overloads of
-  // `__clang_Interpreter_SetValueNoAlloc`.
-  void HandleIntegralOrEnumType(const Type *Ty) {
-    TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.UnsignedLongLongTy);
-    ExprResult CastedExpr =
-        S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
-    assert(!CastedExpr.isInvalid() && "Cannot create cstyle cast expr");
-    Args.push_back(CastedExpr.get());
-  }
-
-  void HandlePtrType(const Type *Ty) {
-    TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ctx.VoidPtrTy);
-    ExprResult CastedExpr =
-        S.BuildCStyleCastExpr(SourceLocation(), TSI, SourceLocation(), E);
-    assert(!CastedExpr.isInvalid() && "Can not create cstyle cast expression");
-    Args.push_back(CastedExpr.get());
-  }
 };
 } // namespace
 

>From 78bea3a4ec2401c3043bff86aeb7a136f280f5ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Mon, 26 Feb 2024 19:26:14 +0100
Subject: [PATCH 2/4] [clang-repl] Construct RuntimeInterfaceBuilder early and
 pass invocation-specific params in getCall()

---
 clang/lib/Interpreter/Interpreter.cpp | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 7afe55bbb266a3..f0b9195b684ba6 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -629,23 +629,22 @@ class RuntimeInterfaceBuilder {
   clang::Interpreter &Interp;
   ASTContext &Ctx;
   Sema &S;
-  Expr *E;
-  InterfaceKindVisitor Visitor;
 
 public:
-  RuntimeInterfaceBuilder(clang::Interpreter &In, ASTContext &C, Sema &SemaRef,
-                          Expr *VE, ArrayRef<Expr *> FixedArgs)
-      : Interp(In), Ctx(C), S(SemaRef), E(VE), Visitor(C, SemaRef, VE) {
-    // The Interpreter* parameter and the out parameter `OutVal`.
-    for (Expr *E : FixedArgs)
-      Visitor.Args.push_back(E);
+  RuntimeInterfaceBuilder(clang::Interpreter &In, ASTContext &C, Sema &SemaRef)
+      : Interp(In), Ctx(C), S(SemaRef) {}
 
+  ExprResult getCall(Expr *E, ArrayRef<Expr *> FixedArgs) {
     // Get rid of ExprWithCleanups.
     if (auto *EWC = llvm::dyn_cast_if_present<ExprWithCleanups>(E))
       E = EWC->getSubExpr();
-  }
 
-  ExprResult getCall() {
+    InterfaceKindVisitor Visitor(Ctx, S, E);
+
+    // The Interpreter* parameter and the out parameter `OutVal`.
+    for (Expr *E : FixedArgs)
+      Visitor.Args.push_back(E);
+
     QualType Ty = E->getType();
     QualType DesugaredTy = Ty.getDesugaredType(Ctx);
 
@@ -747,6 +746,8 @@ Expr *Interpreter::SynthesizeExpr(Expr *E) {
   if (!FindRuntimeInterface())
     llvm_unreachable("We can't find the runtime iterface for pretty print!");
 
+  RuntimeInterfaceBuilder Builder(*this, Ctx, S);
+
   // Create parameter `ThisInterp`.
   auto *ThisInterp = CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)this);
 
@@ -754,9 +755,8 @@ Expr *Interpreter::SynthesizeExpr(Expr *E) {
   auto *OutValue = CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)&LastValue);
 
   // Build `__clang_Interpreter_SetValue*` call.
-  RuntimeInterfaceBuilder Builder(*this, Ctx, S, E, {ThisInterp, OutValue});
+  ExprResult Result = Builder.getCall(E, {ThisInterp, OutValue});
 
-  ExprResult Result = Builder.getCall();
   // It could fail, like printing an array type in C. (not supported)
   if (Result.isInvalid())
     return E;

>From 0f9469987aefdf1a76da27a2352da80de7a5561b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Mon, 26 Feb 2024 20:04:17 +0100
Subject: [PATCH 3/4] [clang-repl] Let FindRuntimeInterface() return
 RuntimeInterfaceBuilder and make it virtual

---
 clang/include/clang/Interpreter/Interpreter.h | 22 +++++--
 clang/lib/Interpreter/Interpreter.cpp         | 58 +++++++++++++------
 2 files changed, 56 insertions(+), 24 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 292fa566ae7037..787357ea7f20c3 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -18,6 +18,7 @@
 #include "clang/AST/GlobalDecl.h"
 #include "clang/Interpreter/PartialTranslationUnit.h"
 #include "clang/Interpreter/Value.h"
+#include "clang/Sema/Ownership.h"
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ExecutionEngine/JITSymbol.h"
@@ -72,17 +73,22 @@ class IncrementalCompilerBuilder {
   llvm::StringRef CudaSDKPath;
 };
 
+class RuntimeInterfaceBuilder {
+public:
+  virtual ~RuntimeInterfaceBuilder() = default;
+  virtual ExprResult getCall(Expr *E, ArrayRef<Expr *> FixedArgs) = 0;
+};
+
 /// Provides top-level interfaces for incremental compilation and execution.
 class Interpreter {
   std::unique_ptr<llvm::orc::ThreadSafeContext> TSCtx;
   std::unique_ptr<IncrementalParser> IncrParser;
   std::unique_ptr<IncrementalExecutor> IncrExecutor;
+  std::unique_ptr<RuntimeInterfaceBuilder> RuntimeIB;
 
   // An optional parser for CUDA offloading
   std::unique_ptr<IncrementalParser> DeviceParser;
 
-  Interpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &Err);
-
   llvm::Error CreateExecutor();
   unsigned InitPTUSize = 0;
 
@@ -91,8 +97,13 @@ class Interpreter {
   // printing happens, it's in an invalid state.
   Value LastValue;
 
+protected:
+  Interpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &Err);
+
+  void finalizeInitPTUStack();
+
 public:
-  ~Interpreter();
+  virtual ~Interpreter();
   static llvm::Expected<std::unique_ptr<Interpreter>>
   create(std::unique_ptr<CompilerInstance> CI);
   static llvm::Expected<std::unique_ptr<Interpreter>>
@@ -137,11 +148,12 @@ class Interpreter {
 
   Expr *SynthesizeExpr(Expr *E);
 
+protected:
+  virtual RuntimeInterfaceBuilder *FindRuntimeInterface();
+
 private:
   size_t getEffectivePTUSize() const;
 
-  bool FindRuntimeInterface();
-
   llvm::DenseMap<CXXRecordDecl *, llvm::orc::ExecutorAddr> Dtors;
 
   llvm::SmallVector<Expr *, 4> ValuePrintingInfo;
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index f0b9195b684ba6..4dc15f5c735fc3 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -283,10 +283,12 @@ Interpreter::create(std::unique_ptr<CompilerInstance> CI) {
     return PTU.takeError();
 
   Interp->ValuePrintingInfo.resize(4);
+
   // FIXME: This is a ugly hack. Undo command checks its availability by looking
   // at the size of the PTU list. However we have parsed something in the
   // beginning of the REPL so we have to mark them as 'Irrevocable'.
-  Interp->InitPTUSize = Interp->IncrParser->getPTUs().size();
+  Interp->finalizeInitPTUStack();
+
   return std::move(Interp);
 }
 
@@ -343,6 +345,11 @@ const ASTContext &Interpreter::getASTContext() const {
   return getCompilerInstance()->getASTContext();
 }
 
+void Interpreter::finalizeInitPTUStack() {
+  assert(!InitPTUSize && "We only do this once");
+  InitPTUSize = IncrParser->getPTUs().size();
+}
+
 size_t Interpreter::getEffectivePTUSize() const {
   std::list<PartialTranslationUnit> &PTUs = IncrParser->getPTUs();
   assert(PTUs.size() >= InitPTUSize && "empty PTU list?");
@@ -505,9 +512,16 @@ static constexpr llvm::StringRef MagicRuntimeInterface[] = {
     "__clang_Interpreter_SetValueWithAlloc",
     "__clang_Interpreter_SetValueCopyArr", "__ci_newtag"};
 
-bool Interpreter::FindRuntimeInterface() {
+static std::unique_ptr<RuntimeInterfaceBuilder>
+createInProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &Ctx,
+                                       Sema &S);
+
+RuntimeInterfaceBuilder *Interpreter::FindRuntimeInterface() {
+  if (RuntimeIB)
+    return RuntimeIB.get();
+
   if (llvm::all_of(ValuePrintingInfo, [](Expr *E) { return E != nullptr; }))
-    return true;
+    return nullptr;
 
   Sema &S = getCompilerInstance()->getSema();
   ASTContext &Ctx = S.getASTContext();
@@ -526,24 +540,26 @@ bool Interpreter::FindRuntimeInterface() {
 
   if (!LookupInterface(ValuePrintingInfo[NoAlloc],
                        MagicRuntimeInterface[NoAlloc]))
-    return false;
+    return nullptr;
   if (!LookupInterface(ValuePrintingInfo[WithAlloc],
                        MagicRuntimeInterface[WithAlloc]))
-    return false;
+    return nullptr;
   if (!LookupInterface(ValuePrintingInfo[CopyArray],
                        MagicRuntimeInterface[CopyArray]))
-    return false;
+    return nullptr;
   if (!LookupInterface(ValuePrintingInfo[NewTag],
                        MagicRuntimeInterface[NewTag]))
-    return false;
-  return true;
+    return nullptr;
+
+  RuntimeIB = createInProcessRuntimeInterfaceBuilder(*this, Ctx, S);
+  return RuntimeIB.get();
 }
 
 namespace {
 
 class InterfaceKindVisitor
     : public TypeVisitor<InterfaceKindVisitor, Interpreter::InterfaceKind> {
-  friend class RuntimeInterfaceBuilder;
+  friend class InProcessRuntimeInterfaceBuilder;
 
   ASTContext &Ctx;
   Sema &S;
@@ -625,16 +641,16 @@ class InterfaceKindVisitor
   }
 };
 
-class RuntimeInterfaceBuilder {
-  clang::Interpreter &Interp;
+class InProcessRuntimeInterfaceBuilder : public RuntimeInterfaceBuilder {
+  Interpreter &Interp;
   ASTContext &Ctx;
   Sema &S;
 
 public:
-  RuntimeInterfaceBuilder(clang::Interpreter &In, ASTContext &C, Sema &SemaRef)
-      : Interp(In), Ctx(C), S(SemaRef) {}
+  InProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &C, Sema &S)
+      : Interp(Interp), Ctx(C), S(S) {}
 
-  ExprResult getCall(Expr *E, ArrayRef<Expr *> FixedArgs) {
+  ExprResult getCall(Expr *E, ArrayRef<Expr *> FixedArgs) override {
     // Get rid of ExprWithCleanups.
     if (auto *EWC = llvm::dyn_cast_if_present<ExprWithCleanups>(E))
       E = EWC->getSubExpr();
@@ -725,6 +741,12 @@ class RuntimeInterfaceBuilder {
 };
 } // namespace
 
+static std::unique_ptr<RuntimeInterfaceBuilder>
+createInProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &Ctx,
+                                       Sema &S) {
+  return std::make_unique<InProcessRuntimeInterfaceBuilder>(Interp, Ctx, S);
+}
+
 // This synthesizes a call expression to a speciall
 // function that is responsible for generating the Value.
 // In general, we transform:
@@ -743,10 +765,8 @@ Expr *Interpreter::SynthesizeExpr(Expr *E) {
   Sema &S = getCompilerInstance()->getSema();
   ASTContext &Ctx = S.getASTContext();
 
-  if (!FindRuntimeInterface())
-    llvm_unreachable("We can't find the runtime iterface for pretty print!");
-
-  RuntimeInterfaceBuilder Builder(*this, Ctx, S);
+  RuntimeInterfaceBuilder *Builder = FindRuntimeInterface();
+  assert(Builder && "We can't find the runtime interface for pretty print!");
 
   // Create parameter `ThisInterp`.
   auto *ThisInterp = CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)this);
@@ -755,7 +775,7 @@ Expr *Interpreter::SynthesizeExpr(Expr *E) {
   auto *OutValue = CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)&LastValue);
 
   // Build `__clang_Interpreter_SetValue*` call.
-  ExprResult Result = Builder.getCall(E, {ThisInterp, OutValue});
+  ExprResult Result = Builder->getCall(E, {ThisInterp, OutValue});
 
   // It could fail, like printing an array type in C. (not supported)
   if (Result.isInvalid())

>From f069bafd8a3c597934f17236b530ffbcfe8d0f92 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20Gr=C3=A4nitz?= <stefan.graenitz at gmail.com>
Date: Wed, 28 Feb 2024 11:13:48 +0100
Subject: [PATCH 4/4] [clang-repl] Avoid virtual function calls in
 RuntimeInterfaceBuilder abstraction

---
 clang/include/clang/Interpreter/Interpreter.h |   8 +-
 clang/lib/Interpreter/Interpreter.cpp         | 180 +++++++++---------
 2 files changed, 98 insertions(+), 90 deletions(-)

diff --git a/clang/include/clang/Interpreter/Interpreter.h b/clang/include/clang/Interpreter/Interpreter.h
index 787357ea7f20c3..0aae00ddd216f7 100644
--- a/clang/include/clang/Interpreter/Interpreter.h
+++ b/clang/include/clang/Interpreter/Interpreter.h
@@ -76,7 +76,9 @@ class IncrementalCompilerBuilder {
 class RuntimeInterfaceBuilder {
 public:
   virtual ~RuntimeInterfaceBuilder() = default;
-  virtual ExprResult getCall(Expr *E, ArrayRef<Expr *> FixedArgs) = 0;
+
+  using AddCallToRuntimeInterfaceFunction = ExprResult(RuntimeInterfaceBuilder *Builder, Expr *, ArrayRef<Expr *>);
+  virtual AddCallToRuntimeInterfaceFunction *getCallCallback() = 0;
 };
 
 /// Provides top-level interfaces for incremental compilation and execution.
@@ -100,6 +102,8 @@ class Interpreter {
 protected:
   Interpreter(std::unique_ptr<CompilerInstance> CI, llvm::Error &Err);
 
+  RuntimeInterfaceBuilder::AddCallToRuntimeInterfaceFunction* AddCallToRuntimeInterface = nullptr;
+
   void finalizeInitPTUStack();
 
 public:
@@ -149,7 +153,7 @@ class Interpreter {
   Expr *SynthesizeExpr(Expr *E);
 
 protected:
-  virtual RuntimeInterfaceBuilder *FindRuntimeInterface();
+  virtual std::unique_ptr<RuntimeInterfaceBuilder> FindRuntimeInterface();
 
 private:
   size_t getEffectivePTUSize() const;
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 4dc15f5c735fc3..a1c577bd2863b1 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -516,10 +516,7 @@ static std::unique_ptr<RuntimeInterfaceBuilder>
 createInProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &Ctx,
                                        Sema &S);
 
-RuntimeInterfaceBuilder *Interpreter::FindRuntimeInterface() {
-  if (RuntimeIB)
-    return RuntimeIB.get();
-
+std::unique_ptr<RuntimeInterfaceBuilder> Interpreter::FindRuntimeInterface() {
   if (llvm::all_of(ValuePrintingInfo, [](Expr *E) { return E != nullptr; }))
     return nullptr;
 
@@ -551,8 +548,7 @@ RuntimeInterfaceBuilder *Interpreter::FindRuntimeInterface() {
                        MagicRuntimeInterface[NewTag]))
     return nullptr;
 
-  RuntimeIB = createInProcessRuntimeInterfaceBuilder(*this, Ctx, S);
-  return RuntimeIB.get();
+  return createInProcessRuntimeInterfaceBuilder(*this, Ctx, S);
 }
 
 namespace {
@@ -650,93 +646,97 @@ class InProcessRuntimeInterfaceBuilder : public RuntimeInterfaceBuilder {
   InProcessRuntimeInterfaceBuilder(Interpreter &Interp, ASTContext &C, Sema &S)
       : Interp(Interp), Ctx(C), S(S) {}
 
-  ExprResult getCall(Expr *E, ArrayRef<Expr *> FixedArgs) override {
-    // Get rid of ExprWithCleanups.
-    if (auto *EWC = llvm::dyn_cast_if_present<ExprWithCleanups>(E))
-      E = EWC->getSubExpr();
+  AddCallToRuntimeInterfaceFunction *getCallCallback() override {
+    return [](RuntimeInterfaceBuilder *Builder, Expr *E, ArrayRef<Expr *> FixedArgs) -> ExprResult {
+      auto *B = static_cast<InProcessRuntimeInterfaceBuilder *>(Builder);
 
-    InterfaceKindVisitor Visitor(Ctx, S, E);
+      // Get rid of ExprWithCleanups.
+      if (auto *EWC = llvm::dyn_cast_if_present<ExprWithCleanups>(E))
+        E = EWC->getSubExpr();
 
-    // The Interpreter* parameter and the out parameter `OutVal`.
-    for (Expr *E : FixedArgs)
-      Visitor.Args.push_back(E);
+      InterfaceKindVisitor Visitor(B->Ctx, B->S, E);
 
-    QualType Ty = E->getType();
-    QualType DesugaredTy = Ty.getDesugaredType(Ctx);
+      // The Interpreter* parameter and the out parameter `OutVal`.
+      for (Expr *E : FixedArgs)
+        Visitor.Args.push_back(E);
 
-    // For lvalue struct, we treat it as a reference.
-    if (DesugaredTy->isRecordType() && E->isLValue()) {
-      DesugaredTy = Ctx.getLValueReferenceType(DesugaredTy);
-      Ty = Ctx.getLValueReferenceType(Ty);
-    }
+      QualType Ty = E->getType();
+      QualType DesugaredTy = Ty.getDesugaredType(B->Ctx);
 
-    Expr *TypeArg =
-        CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)Ty.getAsOpaquePtr());
-    // The QualType parameter `OpaqueType`, represented as `void*`.
-    Visitor.Args.push_back(TypeArg);
-
-    // We push the last parameter based on the type of the Expr. Note we need
-    // special care for rvalue struct.
-    Interpreter::InterfaceKind Kind = Visitor.Visit(&*DesugaredTy);
-    switch (Kind) {
-    case Interpreter::InterfaceKind::WithAlloc:
-    case Interpreter::InterfaceKind::CopyArray: {
-      // __clang_Interpreter_SetValueWithAlloc.
-      ExprResult AllocCall = S.ActOnCallExpr(
-          /*Scope=*/nullptr,
-          Interp.getValuePrintingInfo()[Interpreter::InterfaceKind::WithAlloc],
-          E->getBeginLoc(), Visitor.Args, E->getEndLoc());
-      assert(!AllocCall.isInvalid() && "Can't create runtime interface call!");
-
-      TypeSourceInfo *TSI = Ctx.getTrivialTypeSourceInfo(Ty, SourceLocation());
-
-      // Force CodeGen to emit destructor.
-      if (auto *RD = Ty->getAsCXXRecordDecl()) {
-        auto *Dtor = S.LookupDestructor(RD);
-        Dtor->addAttr(UsedAttr::CreateImplicit(Ctx));
-        Interp.getCompilerInstance()->getASTConsumer().HandleTopLevelDecl(
-            DeclGroupRef(Dtor));
+      // For lvalue struct, we treat it as a reference.
+      if (DesugaredTy->isRecordType() && E->isLValue()) {
+        DesugaredTy = B->Ctx.getLValueReferenceType(DesugaredTy);
+        Ty = B->Ctx.getLValueReferenceType(Ty);
       }
 
-      // __clang_Interpreter_SetValueCopyArr.
-      if (Kind == Interpreter::InterfaceKind::CopyArray) {
-        const auto *ConstantArrTy =
-            cast<ConstantArrayType>(DesugaredTy.getTypePtr());
-        size_t ArrSize = Ctx.getConstantArrayElementCount(ConstantArrTy);
-        Expr *ArrSizeExpr = IntegerLiteralExpr(Ctx, ArrSize);
-        Expr *Args[] = {E, AllocCall.get(), ArrSizeExpr};
-        return S.ActOnCallExpr(
-            /*Scope *=*/nullptr,
-            Interp
-                .getValuePrintingInfo()[Interpreter::InterfaceKind::CopyArray],
-            SourceLocation(), Args, SourceLocation());
+      Expr *TypeArg =
+          CStyleCastPtrExpr(B->S, B->Ctx.VoidPtrTy, (uintptr_t)Ty.getAsOpaquePtr());
+      // The QualType parameter `OpaqueType`, represented as `void*`.
+      Visitor.Args.push_back(TypeArg);
+
+      // We push the last parameter based on the type of the Expr. Note we need
+      // special care for rvalue struct.
+      Interpreter::InterfaceKind Kind = Visitor.Visit(&*DesugaredTy);
+      switch (Kind) {
+      case Interpreter::InterfaceKind::WithAlloc:
+      case Interpreter::InterfaceKind::CopyArray: {
+        // __clang_Interpreter_SetValueWithAlloc.
+        ExprResult AllocCall = B->S.ActOnCallExpr(
+            /*Scope=*/nullptr,
+            B->Interp.getValuePrintingInfo()[Interpreter::InterfaceKind::WithAlloc],
+            E->getBeginLoc(), Visitor.Args, E->getEndLoc());
+        assert(!AllocCall.isInvalid() && "Can't create runtime interface call!");
+
+        TypeSourceInfo *TSI = B->Ctx.getTrivialTypeSourceInfo(Ty, SourceLocation());
+
+        // Force CodeGen to emit destructor.
+        if (auto *RD = Ty->getAsCXXRecordDecl()) {
+          auto *Dtor = B->S.LookupDestructor(RD);
+          Dtor->addAttr(UsedAttr::CreateImplicit(B->Ctx));
+          B->Interp.getCompilerInstance()->getASTConsumer().HandleTopLevelDecl(
+              DeclGroupRef(Dtor));
+        }
+
+        // __clang_Interpreter_SetValueCopyArr.
+        if (Kind == Interpreter::InterfaceKind::CopyArray) {
+          const auto *ConstantArrTy =
+              cast<ConstantArrayType>(DesugaredTy.getTypePtr());
+          size_t ArrSize = B->Ctx.getConstantArrayElementCount(ConstantArrTy);
+          Expr *ArrSizeExpr = IntegerLiteralExpr(B->Ctx, ArrSize);
+          Expr *Args[] = {E, AllocCall.get(), ArrSizeExpr};
+          return B->S.ActOnCallExpr(
+              /*Scope *=*/nullptr,
+              B->Interp
+                  .getValuePrintingInfo()[Interpreter::InterfaceKind::CopyArray],
+              SourceLocation(), Args, SourceLocation());
+        }
+        Expr *Args[] = {
+            AllocCall.get(),
+            B->Interp.getValuePrintingInfo()[Interpreter::InterfaceKind::NewTag]};
+        ExprResult CXXNewCall = B->S.BuildCXXNew(
+            E->getSourceRange(),
+            /*UseGlobal=*/true, /*PlacementLParen=*/SourceLocation(), Args,
+            /*PlacementRParen=*/SourceLocation(),
+            /*TypeIdParens=*/SourceRange(), TSI->getType(), TSI, std::nullopt,
+            E->getSourceRange(), E);
+
+        assert(!CXXNewCall.isInvalid() &&
+              "Can't create runtime placement new call!");
+
+        return B->S.ActOnFinishFullExpr(CXXNewCall.get(),
+                                    /*DiscardedValue=*/false);
       }
-      Expr *Args[] = {
-          AllocCall.get(),
-          Interp.getValuePrintingInfo()[Interpreter::InterfaceKind::NewTag]};
-      ExprResult CXXNewCall = S.BuildCXXNew(
-          E->getSourceRange(),
-          /*UseGlobal=*/true, /*PlacementLParen=*/SourceLocation(), Args,
-          /*PlacementRParen=*/SourceLocation(),
-          /*TypeIdParens=*/SourceRange(), TSI->getType(), TSI, std::nullopt,
-          E->getSourceRange(), E);
-
-      assert(!CXXNewCall.isInvalid() &&
-             "Can't create runtime placement new call!");
-
-      return S.ActOnFinishFullExpr(CXXNewCall.get(),
-                                   /*DiscardedValue=*/false);
-    }
-      // __clang_Interpreter_SetValueNoAlloc.
-    case Interpreter::InterfaceKind::NoAlloc: {
-      return S.ActOnCallExpr(
-          /*Scope=*/nullptr,
-          Interp.getValuePrintingInfo()[Interpreter::InterfaceKind::NoAlloc],
-          E->getBeginLoc(), Visitor.Args, E->getEndLoc());
-    }
-    default:
-      llvm_unreachable("Unhandled Interpreter::InterfaceKind");
-    }
+        // __clang_Interpreter_SetValueNoAlloc.
+      case Interpreter::InterfaceKind::NoAlloc: {
+        return B->S.ActOnCallExpr(
+            /*Scope=*/nullptr,
+            B->Interp.getValuePrintingInfo()[Interpreter::InterfaceKind::NoAlloc],
+            E->getBeginLoc(), Visitor.Args, E->getEndLoc());
+      }
+      default:
+        llvm_unreachable("Unhandled Interpreter::InterfaceKind");
+      }
+    };
   }
 };
 } // namespace
@@ -765,8 +765,12 @@ Expr *Interpreter::SynthesizeExpr(Expr *E) {
   Sema &S = getCompilerInstance()->getSema();
   ASTContext &Ctx = S.getASTContext();
 
-  RuntimeInterfaceBuilder *Builder = FindRuntimeInterface();
-  assert(Builder && "We can't find the runtime interface for pretty print!");
+  if (!RuntimeIB) {
+    RuntimeIB = FindRuntimeInterface();
+    AddCallToRuntimeInterface = RuntimeIB->getCallCallback();
+  }
+
+  assert(AddCallToRuntimeInterface && "We don't have a runtime interface for pretty print!");
 
   // Create parameter `ThisInterp`.
   auto *ThisInterp = CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)this);
@@ -775,7 +779,7 @@ Expr *Interpreter::SynthesizeExpr(Expr *E) {
   auto *OutValue = CStyleCastPtrExpr(S, Ctx.VoidPtrTy, (uintptr_t)&LastValue);
 
   // Build `__clang_Interpreter_SetValue*` call.
-  ExprResult Result = Builder->getCall(E, {ThisInterp, OutValue});
+  ExprResult Result = AddCallToRuntimeInterface(RuntimeIB.get(), E, {ThisInterp, OutValue});
 
   // It could fail, like printing an array type in C. (not supported)
   if (Result.isInvalid())



More information about the cfe-commits mailing list