[clang][Interp] Implement This pointer passing to methods
authorTimm Bäder <tbaeder@redhat.com>
Sun, 18 Sep 2022 18:40:27 +0000 (20:40 +0200)
committerTimm Bäder <tbaeder@redhat.com>
Fri, 14 Oct 2022 08:21:53 +0000 (10:21 +0200)
Implement passing the this pointer to member functions and constructors.
The this pointer is passed via the stack. This changes the functions to
explicitly track whether they have a RVO pointer and a this pointer.

Differential Revision: https://reviews.llvm.org/D134699

12 files changed:
clang/lib/AST/Interp/ByteCodeEmitter.cpp
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeExprGen.h
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Disasm.cpp
clang/lib/AST/Interp/EvalEmitter.cpp
clang/lib/AST/Interp/Function.cpp
clang/lib/AST/Interp/Function.h
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/InterpFrame.cpp
clang/lib/AST/Interp/InterpFrame.h
clang/test/AST/Interp/records.cpp

index 20e054a..7fa189a 100644 (file)
@@ -31,8 +31,21 @@ Expected<Function *> ByteCodeEmitter::compileFunc(const FunctionDecl *F) {
 
   // If the return is not a primitive, a pointer to the storage where the value
   // is initialized in is passed as the first argument.
+  // See 'RVO' elsewhere in the code.
   QualType Ty = F->getReturnType();
+  bool HasRVO = false;
   if (!Ty->isVoidType() && !Ctx.classify(Ty)) {
+    HasRVO = true;
+    ParamTypes.push_back(PT_Ptr);
+    ParamOffset += align(primSize(PT_Ptr));
+  }
+
+  // If the function decl is a member decl, the next parameter is
+  // the 'this' pointer. This parameter is pop()ed from the
+  // InterStack when calling the function.
+  bool HasThisPointer = false;
+  if (const auto *MD = dyn_cast<CXXMethodDecl>(F); MD && MD->isInstance()) {
+    HasThisPointer = true;
     ParamTypes.push_back(PT_Ptr);
     ParamOffset += align(primSize(PT_Ptr));
   }
@@ -55,8 +68,9 @@ Expected<Function *> ByteCodeEmitter::compileFunc(const FunctionDecl *F) {
   }
 
   // Create a handle over the emitted code.
-  Function *Func = P.createFunction(F, ParamOffset, std::move(ParamTypes),
-                                    std::move(ParamDescriptors));
+  Function *Func =
+      P.createFunction(F, ParamOffset, std::move(ParamTypes),
+                       std::move(ParamDescriptors), HasThisPointer, HasRVO);
   // Compile the function body.
   if (!F->isConstexpr() || !visitFunc(F)) {
     // Return a dummy function if compilation failed.
index 2e83435..d743e3a 100644 (file)
@@ -637,34 +637,16 @@ bool ByteCodeExprGen<Emitter>::visitRecordInitializer(const Expr *Initializer) {
   assert(Initializer->getType()->isRecordType());
 
   if (const auto CtorExpr = dyn_cast<CXXConstructExpr>(Initializer)) {
-    const CXXConstructorDecl *Ctor = CtorExpr->getConstructor();
-    const RecordDecl *RD = Ctor->getParent();
-    const Record *R = getRecord(RD);
-
-    for (const auto *Init : Ctor->inits()) {
-      const FieldDecl *Member = Init->getMember();
-      const Expr *InitExpr = Init->getInit();
-
-      if (Optional<PrimType> T = classify(InitExpr->getType())) {
-        const Record::Field *F = R->getField(Member);
-
-        if (!this->emitDupPtr(Initializer))
-          return false;
+    const Function *Func = getFunction(CtorExpr->getConstructor());
 
-        if (!this->visit(InitExpr))
-          return false;
-
-        if (!this->emitInitField(*T, F->Offset, Initializer))
-          return false;
-      } else {
-        assert(false && "Handle initializer for non-primitive values");
-      }
-    }
+    if (!Func)
+      return false;
 
-    // FIXME: Actually visit() the constructor Body
-    const Stmt *Body = Ctor->getBody();
-    (void)Body;
-    return true;
+    // The This pointer is already on the stack because this is an initializer,
+    // but we need to dup() so the call() below has its own copy.
+    if (!this->emitDupPtr(Initializer))
+      return false;
+    return this->emitCallVoid(Func, Initializer);
   } else if (const auto *InitList = dyn_cast<InitListExpr>(Initializer)) {
     const Record *R = getRecord(InitList->getType());
 
@@ -688,7 +670,10 @@ bool ByteCodeExprGen<Emitter>::visitRecordInitializer(const Expr *Initializer) {
     return true;
   } else if (const CallExpr *CE = dyn_cast<CallExpr>(Initializer)) {
     const Decl *Callee = CE->getCalleeDecl();
-    const Function *Func = P.getFunction(dyn_cast<FunctionDecl>(Callee));
+    const Function *Func = getFunction(dyn_cast<FunctionDecl>(Callee));
+
+    if (!Func)
+      return false;
 
     if (Func->hasRVO()) {
       // RVO functions expect a pointer to initialize on the stack.
@@ -699,7 +684,6 @@ bool ByteCodeExprGen<Emitter>::visitRecordInitializer(const Expr *Initializer) {
       return this->visit(CE);
     }
   }
-
   return false;
 }
 
@@ -766,6 +750,23 @@ Record *ByteCodeExprGen<Emitter>::getRecord(const RecordDecl *RD) {
 }
 
 template <class Emitter>
+const Function *ByteCodeExprGen<Emitter>::getFunction(const FunctionDecl *FD) {
+  assert(FD);
+  const Function *Func = P.getFunction(FD);
+
+  if (!Func) {
+    if (auto R = ByteCodeStmtGen<ByteCodeEmitter>(Ctx, P).compileFunc(FD))
+      Func = *R;
+    else {
+      llvm::consumeError(R.takeError());
+      return nullptr;
+    }
+  }
+
+  return Func;
+}
+
+template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *Exp) {
   ExprScope<Emitter> RootScope(this);
   if (!visit(Exp))
@@ -820,16 +821,9 @@ bool ByteCodeExprGen<Emitter>::VisitCallExpr(const CallExpr *E) {
 
   const Decl *Callee = E->getCalleeDecl();
   if (const auto *FuncDecl = dyn_cast_or_null<FunctionDecl>(Callee)) {
-    const Function *Func = P.getFunction(FuncDecl);
-
-    // Templated functions might not have been compiled yet, so do it now.
-    if (!Func) {
-      if (auto R =
-              ByteCodeStmtGen<ByteCodeEmitter>(Ctx, P).compileFunc(FuncDecl))
-        Func = *R;
-    }
-    assert(Func);
-
+    const Function *Func = getFunction(FuncDecl);
+    if (!Func)
+      return false;
     // If the function is being compiled right now, this is a recursive call.
     // In that case, the function can't be valid yet, even though it will be
     // later.
@@ -840,23 +834,24 @@ bool ByteCodeExprGen<Emitter>::VisitCallExpr(const CallExpr *E) {
 
     QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
     Optional<PrimType> T = classify(ReturnType);
+    // Put arguments on the stack.
+    for (const auto *Arg : E->arguments()) {
+      if (!this->visit(Arg))
+        return false;
+    }
 
-    if (T || ReturnType->isVoidType()) {
-      // Put arguments on the stack.
-      for (const auto *Arg : E->arguments()) {
-        if (!this->visit(Arg))
-          return false;
-      }
+    // Primitive return value, just call it.
+    if (T)
+      return this->emitCall(*T, Func, E);
 
-      if (T)
-        return this->emitCall(*T, Func, E);
+    // Void Return value, easy.
+    if (ReturnType->isVoidType())
       return this->emitCallVoid(Func, E);
-    } else {
-      if (Func->hasRVO())
-        return this->emitCallVoid(Func, E);
-      assert(false && "Can't classify function return type");
-    }
 
+    // Non-primitive return value with Return Value Optimization,
+    // we already have a pointer on the stack to write the result into.
+    if (Func->hasRVO())
+      return this->emitCallVoid(Func, E);
   } else {
     assert(false && "We don't support non-FunctionDecl callees right now.");
   }
@@ -865,6 +860,16 @@ bool ByteCodeExprGen<Emitter>::VisitCallExpr(const CallExpr *E) {
 }
 
 template <class Emitter>
+bool ByteCodeExprGen<Emitter>::VisitCXXMemberCallExpr(
+    const CXXMemberCallExpr *E) {
+  // Get a This pointer on the stack.
+  if (!this->visit(E->getImplicitObjectArgument()))
+    return false;
+
+  return VisitCallExpr(E);
+}
+
+template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitCXXDefaultInitExpr(
     const CXXDefaultInitExpr *E) {
   return this->visit(E->getExpr());
@@ -895,6 +900,11 @@ bool ByteCodeExprGen<Emitter>::VisitCXXNullPtrLiteralExpr(
 }
 
 template <class Emitter>
+bool ByteCodeExprGen<Emitter>::VisitCXXThisExpr(const CXXThisExpr *E) {
+  return this->emitThis(E);
+}
+
+template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitUnaryOperator(const UnaryOperator *E) {
   const Expr *SubExpr = E->getSubExpr();
 
index 669c805..14356ec 100644 (file)
@@ -71,9 +71,11 @@ public:
   bool VisitBinaryOperator(const BinaryOperator *E);
   bool VisitCXXDefaultArgExpr(const CXXDefaultArgExpr *E);
   bool VisitCallExpr(const CallExpr *E);
+  bool VisitCXXMemberCallExpr(const CXXMemberCallExpr *E);
   bool VisitCXXDefaultInitExpr(const CXXDefaultInitExpr *E);
   bool VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E);
   bool VisitCXXNullPtrLiteralExpr(const CXXNullPtrLiteralExpr *E);
+  bool VisitCXXThisExpr(const CXXThisExpr *E);
   bool VisitUnaryOperator(const UnaryOperator *E);
   bool VisitDeclRefExpr(const DeclRefExpr *E);
   bool VisitImplicitValueInitExpr(const ImplicitValueInitExpr *E);
@@ -99,6 +101,10 @@ protected:
   Record *getRecord(QualType Ty);
   Record *getRecord(const RecordDecl *RD);
 
+  // Returns a function for the given FunctionDecl.
+  // If the function does not exist yet, it is compiled.
+  const Function *getFunction(const FunctionDecl *FD);
+
   /// Returns the size int bits of an integer.
   unsigned getIntWidth(QualType Ty) {
     auto &ASTContext = Ctx.getASTContext();
index 47c5527..54a0f50 100644 (file)
@@ -94,7 +94,33 @@ bool ByteCodeStmtGen<Emitter>::visitFunc(const FunctionDecl *F) {
   // Classify the return type.
   ReturnType = this->classify(F->getReturnType());
 
-  if (auto *Body = F->getBody())
+  // Constructor. Set up field initializers.
+  if (const auto Ctor = dyn_cast<CXXConstructorDecl>(F)) {
+    const RecordDecl *RD = Ctor->getParent();
+    const Record *R = this->getRecord(RD);
+
+    for (const auto *Init : Ctor->inits()) {
+      const FieldDecl *Member = Init->getMember();
+      const Expr *InitExpr = Init->getInit();
+
+      if (Optional<PrimType> T = this->classify(InitExpr->getType())) {
+        const Record::Field *F = R->getField(Member);
+
+        if (!this->emitDupPtr(InitExpr))
+          return false;
+
+        if (!this->visit(InitExpr))
+          return false;
+
+        if (!this->emitInitField(*T, F->Offset, InitExpr))
+          return false;
+      } else {
+        assert(false && "Handle initializer for non-primitive values");
+      }
+    }
+  }
+
+  if (const auto *Body = F->getBody())
     if (!visitStmt(Body))
       return false;
 
index 67cc248..9fc22f2 100644 (file)
@@ -48,6 +48,7 @@ LLVM_DUMP_METHOD void Function::dump(llvm::raw_ostream &OS) const {
   OS << "frame size: " << getFrameSize() << "\n";
   OS << "arg size:   " << getArgSize() << "\n";
   OS << "rvo:        " << hasRVO() << "\n";
+  OS << "this arg:   " << hasThisPointer() << "\n";
 
   auto PrintName = [&OS](const char *Name) {
     OS << Name;
index 3cc7ab0..94cd1ba 100644 (file)
@@ -105,18 +105,18 @@ template <PrimType OpType> bool EvalEmitter::emitRet(const SourceInfo &Info) {
 template <PrimType OpType>
 bool EvalEmitter::emitCall(const Function *Func, const SourceInfo &Info) {
 
-  S.Current =
-      new InterpFrame(S, const_cast<Function *>(Func), S.Current, {}, {});
+  S.Current = new InterpFrame(S, const_cast<Function *>(Func), {});
   // Result of call will be on the stack and needs to be handled by the caller.
   return Interpret(S, Result);
 }
 
 bool EvalEmitter::emitCallVoid(const Function *Func, const SourceInfo &Info) {
   APValue VoidResult;
-  S.Current =
-      new InterpFrame(S, const_cast<Function *>(Func), S.Current, {}, {});
+  InterpFrame *before = S.Current;
+  S.Current = new InterpFrame(S, const_cast<Function *>(Func), {});
   bool Success = Interpret(S, VoidResult);
   assert(VoidResult.isAbsent());
+  assert(S.Current == before);
   return Success;
 }
 
index 57d020e..a6dfd50 100644 (file)
@@ -17,9 +17,11 @@ using namespace clang::interp;
 
 Function::Function(Program &P, const FunctionDecl *F, unsigned ArgSize,
                    llvm::SmallVector<PrimType, 8> &&ParamTypes,
-                   llvm::DenseMap<unsigned, ParamDescriptor> &&Params)
+                   llvm::DenseMap<unsigned, ParamDescriptor> &&Params,
+                   bool HasThisPointer, bool HasRVO)
     : P(P), Loc(F->getBeginLoc()), F(F), ArgSize(ArgSize),
-      ParamTypes(std::move(ParamTypes)), Params(std::move(Params)) {}
+      ParamTypes(std::move(ParamTypes)), Params(std::move(Params)),
+      HasThisPointer(HasThisPointer), HasRVO(HasRVO) {}
 
 Function::ParamDescriptor Function::getParamDescriptor(unsigned Offset) const {
   auto It = Params.find(Offset);
index cd8b6d8..1fc27ee 100644 (file)
@@ -56,6 +56,21 @@ private:
 ///
 /// Contains links to the bytecode of the function, as well as metadata
 /// describing all arguments and stack-local variables.
+///
+/// # Calling Convention
+///
+/// When calling a function, all argument values must be on the stack.
+///
+/// If the function has a This pointer (i.e. hasThisPointer() returns true,
+/// the argument values need to be preceeded by a Pointer for the This object.
+///
+/// If the function uses Return Value Optimization, the arguments (and
+/// potentially the This pointer) need to be proceeded by a Pointer pointing
+/// to the location to construct the returned value.
+///
+/// After the function has been called, it will remove all arguments,
+/// including RVO and This pointer, from the stack.
+///
 class Function final {
 public:
   using ParamDescriptor = std::pair<PrimType, Descriptor *>;
@@ -84,7 +99,7 @@ public:
   ParamDescriptor getParamDescriptor(unsigned Offset) const;
 
   /// Checks if the first argument is a RVO pointer.
-  bool hasRVO() const { return ParamTypes.size() != Params.size(); }
+  bool hasRVO() const { return HasRVO; }
 
   /// Range over the scope blocks.
   llvm::iterator_range<llvm::SmallVector<Scope, 2>::iterator> scopes() {
@@ -115,11 +130,16 @@ public:
   /// Checks if the function is fully done compiling.
   bool isFullyCompiled() const { return IsFullyCompiled; }
 
+  bool hasThisPointer() const { return HasThisPointer; }
+
+  unsigned getNumParams() const { return ParamTypes.size(); }
+
 private:
   /// Construct a function representing an actual function.
   Function(Program &P, const FunctionDecl *F, unsigned ArgSize,
            llvm::SmallVector<PrimType, 8> &&ParamTypes,
-           llvm::DenseMap<unsigned, ParamDescriptor> &&Params);
+           llvm::DenseMap<unsigned, ParamDescriptor> &&Params,
+           bool HasThisPointer, bool HasRVO);
 
   /// Sets the code of a function.
   void setCode(unsigned NewFrameSize, std::vector<char> &&NewCode, SourceMap &&NewSrcMap,
@@ -162,6 +182,13 @@ private:
   /// Flag to indicate if the function is done being
   /// compiled to bytecode.
   bool IsFullyCompiled = false;
+  /// Flag indicating if this function takes the this pointer
+  /// as the first implicit argument
+  bool HasThisPointer = false;
+  /// Whether this function has Return Value Optimization, i.e.
+  /// the return value is constructed in the caller's stack frame.
+  /// This is done for functions that return non-primive values.
+  bool HasRVO = false;
 
 public:
   /// Dumps the disassembled bytecode to \c llvm::errs().
index 1a1f052..1bc2baf 100644 (file)
@@ -55,8 +55,7 @@ static bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
 
 template <PrimType Name, class T = typename PrimConv<Name>::T>
 static bool Call(InterpState &S, CodePtr &PC, const Function *Func) {
-  S.Current =
-      new InterpFrame(S, const_cast<Function *>(Func), S.Current, PC, {});
+  S.Current = new InterpFrame(S, const_cast<Function *>(Func), PC);
   APValue CallResult;
   // Note that we cannot assert(CallResult.hasValue()) here since
   // Ret() above only sets the APValue if the curent frame doesn't
@@ -66,8 +65,7 @@ static bool Call(InterpState &S, CodePtr &PC, const Function *Func) {
 
 static bool CallVoid(InterpState &S, CodePtr &PC, const Function *Func) {
   APValue VoidResult;
-  S.Current =
-      new InterpFrame(S, const_cast<Function *>(Func), S.Current, PC, {});
+  S.Current = new InterpFrame(S, const_cast<Function *>(Func), PC);
   bool Success = Interpret(S, VoidResult);
   assert(VoidResult.isAbsent());
 
index 60217ee..fb8e5cf 100644 (file)
@@ -36,6 +36,36 @@ InterpFrame::InterpFrame(InterpState &S, Function *Func, InterpFrame *Caller,
   }
 }
 
+InterpFrame::InterpFrame(InterpState &S, Function *Func, CodePtr RetPC)
+    : Caller(S.Current), S(S), Func(Func), RetPC(RetPC),
+      ArgSize(Func ? Func->getArgSize() : 0),
+      Args(static_cast<char *>(S.Stk.top())), FrameOffset(S.Stk.size()) {
+  assert(Func);
+
+  // As per our calling convention, the this pointer is
+  // part of the ArgSize.
+  // If the function has RVO, the RVO pointer is first.
+  // If the fuction has a This pointer, that one is next.
+  // Then follow the actual arguments (but those are handled
+  // in getParamPointer()).
+  if (Func->hasThisPointer()) {
+    if (Func->hasRVO())
+      This = stackRef<Pointer>(sizeof(Pointer));
+    else
+      This = stackRef<Pointer>(0);
+  }
+
+  if (unsigned FrameSize = Func->getFrameSize()) {
+    Locals = std::make_unique<char[]>(FrameSize);
+    for (auto &Scope : Func->scopes()) {
+      for (auto &Local : Scope.locals()) {
+        Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
+        B->invokeCtor();
+      }
+    }
+  }
+}
+
 InterpFrame::~InterpFrame() {
   if (Func && Func->isConstructor() && This.isBaseClass())
     This.initialize();
index fbd4207..3cc894c 100644 (file)
@@ -35,6 +35,11 @@ public:
   InterpFrame(InterpState &S, Function *Func, InterpFrame *Caller,
               CodePtr RetPC, Pointer &&This);
 
+  /// Creates a new frame with the values that make sense.
+  /// I.e., the caller is the current frame of S,
+  /// and the This() pointer is the current Pointer on the top of S's stack,
+  InterpFrame(InterpState &S, Function *Func, CodePtr RetPC);
+
   /// Destroys the frame, killing all live pointers to stack slots.
   ~InterpFrame();
 
index 0237640..ab363ff 100644 (file)
@@ -1,7 +1,6 @@
 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -verify %s
 // RUN: %clang_cc1 -verify=ref %s
 
-// ref-no-diagnostics
 // expected-no-diagnostics
 
 struct BoolPair {
@@ -38,7 +37,6 @@ static_assert(ints2.a == -20, "");
 static_assert(ints2.b == -30, "");
 static_assert(!ints2.c, "");
 
-#if 0
 constexpr Ints getInts() {
   return {64, 128, true};
 }
@@ -46,7 +44,6 @@ constexpr Ints ints3 = getInts();
 static_assert(ints3.a == 64, "");
 static_assert(ints3.b == 128, "");
 static_assert(ints3.c, "");
-#endif
 
 constexpr Ints ints4 = {
   .a = 40 * 50,
@@ -103,3 +100,38 @@ constexpr const C* getPointer() {
   return &c;
 }
 static_assert(getPointer()->a == 100, "");
+
+constexpr C RVOAndParams(const C *c) {
+  return C();
+}
+constexpr C RVOAndParamsResult = RVOAndParams(&c);
+
+constexpr int locals() {
+  C c;
+  c.a = 10;
+
+  // Assignment, not an initializer.
+  // c = C(); FIXME
+  c.a = 10;
+
+
+  // Assignment, not an initializer.
+  //c = RVOAndParams(&c); FIXME
+
+  return c.a;
+}
+static_assert(locals() == 10, "");
+
+namespace thisPointer {
+  struct S {
+    constexpr int get12() { return 12; }
+  };
+
+  constexpr int foo() { // ref-error {{never produces a constant expression}}
+    S *s = nullptr;
+    return s->get12(); // ref-note 2{{member call on dereferenced null pointer}}
+  }
+  // FIXME: The new interpreter doesn't reject this currently.
+  static_assert(foo() == 12, ""); // ref-error {{not an integral constant expression}} \
+                                  // ref-note {{in call to 'foo()'}}
+};