[clang][Interp] Implement This pointer passing to methods

[tbaeder]

Summary

Seems like I completely forgot to post this to phab!

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.

[tbaeder]

Ping

[tbaeder]

@aaron.ballman This one has working precommit CI \o/ (array filler patch is not needed for it though)

[aaron.ballman]

clang/lib/AST/Interp/ByteCodeEmitter.cpp

47

clang/lib/AST/Interp/ByteCodeExprGen.cpp

637–638

What happens if Func is null?

668

Any reason not to use cast here instead, given that getFunction() expects a nonnull pointer anyway?

clang/test/AST/Interp/records.cpp

106

We're missing a fair amount of test coverage here in terms of calling member functions. Can you add some tests for that, as well as a test where the this pointer is invalid and causes UB? e.g.,

struct S {
  constexpr void member() {}
};

constexpr void func(S *s) {
  s->member(); // Should not be a constant expression
}

int main() {
  func(nullptr);
}

[tbaeder]

clang/lib/AST/Interp/ByteCodeExprGen.cpp

637–638

We need to bail out!

668

Not particularly, I guess the generated assertion output is a little nicer if it reaches the one in getFunction().

[tbaeder]

clang/test/AST/Interp/records.cpp

106

I just checked and we indeed don't reject that example. There are already functions to detect such scenarios in Interp.h/.cpp. But actually using them seems to require a bigger surgery since Call/CallVoid are currenly in both EvalEmitter.cpp and Interp.cpp.

I've written this down but I think for this patch it makes sense to add the failing test case and ignore the failure for now.

[aaron.ballman]

clang/test/AST/Interp/records.cpp

106

I'm okay with adding a failure test case with a fixme comment on it for now.

[aaron.ballman]

clang/test/AST/Interp/records.cpp

139

The other thing I think we need some tests for are constructor and destructor calls where the this pointer may be a bit surprising because it needs adjustments. For example, with multiple inheritance where the this pointer may need to be adjusted to get to the fields of the object, and ensuring the correct constructors are called in the correct order.

Another case is with virtual functions (I'm assuming there's no vtable support yet and so that's less interesting, but it will become interesting once we get there so you may want to keep it in mind).

[tbaeder]

clang/test/AST/Interp/records.cpp

139

For constructors, see https://reviews.llvm.org/D135025. Although it doesn't test any sort of order I believe and destructors aren't part of that either.

[aaron.ballman]

LGTM

clang/lib/AST/Interp/EvalEmitter.cpp

108

Not related to this review: it'd be nice to fix the interface so we get const correct behavior rather than needing to use const_cast like this.

[tbaeder]

clang/lib/AST/Interp/EvalEmitter.cpp

108

Already have a local patch for this!

clang/lib/AST/Interp/ByteCodeEmitter.cpp

Expected<Function *> ByteCodeEmitter::compileFunc(const FunctionDecl *F) {
 
   // Set up argument indices.
   unsigned ParamOffset = 0;
   SmallVector<PrimType, 8> ParamTypes;
   llvm::DenseMap<unsigned, Function::ParamDescriptor> ParamDescriptors;
 
   // 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->isStatic()) {
+    HasThisPointer = true;
     ParamTypes.push_back(PT_Ptr);
     ParamOffset += align(primSize(PT_Ptr));
   }
 
   // Assign descriptors to all parameters.
   // Composite objects are lowered to pointers.
   for (const ParmVarDecl *PD : F->parameters()) {
     PrimType Ty;
     if (llvm::Optional<PrimType> T = Ctx.classify(PD->getType())) {
       Ty = *T;
     } else {
       Ty = PT_Ptr;
     }
 
     Descriptor *Desc = P.createDescriptor(PD, Ty);
     ParamDescriptors.insert({ParamOffset, {Ty, Desc}});
     Params.insert({PD, ParamOffset});
     ParamOffset += align(primSize(Ty));
     ParamTypes.push_back(Ty);
   }
 
   // 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.
     if (BailLocation)
       return llvm::make_error<ByteCodeGenError>(*BailLocation);
     else {
       Func->setIsFullyCompiled(true);
       return Func;

clang/lib/AST/Interp/ByteCodeExprGen.h

public:
 
   // Expression visitors - result returned on interp stack.
   bool VisitCastExpr(const CastExpr *E);
   bool VisitIntegerLiteral(const IntegerLiteral *E);
   bool VisitParenExpr(const ParenExpr *E);
   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);
   bool VisitSubstNonTypeTemplateParmExpr(const SubstNonTypeTemplateParmExpr *E);
   bool VisitArraySubscriptExpr(const ArraySubscriptExpr *E);
   bool VisitInitListExpr(const InitListExpr *E);
   bool VisitConstantExpr(const ConstantExpr *E);
   bool VisitUnaryExprOrTypeTraitExpr(const UnaryExprOrTypeTraitExpr *E);
protected:
 
   /// Returns a record type from a record or pointer type.
   const RecordType *getRecordTy(QualType Ty);
 
   /// Returns a record from a record or pointer type.
   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.
+  Function *getFunction(const FunctionDecl *FD);
+
   /// Returns the size int bits of an integer.
   unsigned getIntWidth(QualType Ty) {
     auto &ASTContext = Ctx.getASTContext();
     return ASTContext.getIntWidth(Ty);
   }
 
   /// Returns the value of CHAR_BIT.
   unsigned getCharBit() const {

clang/lib/AST/Interp/ByteCodeExprGen.cpp

bool ByteCodeExprGen<Emitter>::visitArrayInitializer(const Expr *Initializer) {
   return true;
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitRecordInitializer(const Expr *Initializer) {
   Initializer = Initializer->IgnoreParenImpCasts();
   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);
+    const Function *Func = getFunction(CtorExpr->getConstructor());
 
-        if (!this->emitDupPtr(Initializer))
-          return false;
-
-        if (!this->visit(InitExpr))
-          return false;
-
-        if (!this->emitInitField(*T, F->Offset, Initializer))
-          return false;
-      } else {
-        assert(false && "Handle initializer for non-primitive values");
-      }
-    }
-
-    // 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.
+    return this->emitCallVoid(Func, Initializer);
   } else if (const auto *InitList = dyn_cast<InitListExpr>(Initializer)) {
     const Record *R = getRecord(InitList->getType());
 
     unsigned InitIndex = 0;
     for (const Expr *Init : InitList->inits()) {
       const Record::Field *FieldToInit = R->getField(InitIndex);
 
       if (Optional<PrimType> T = classify(Init->getType())) {
         if (!this->emitDupPtr(Initializer))
           return false;
 
         if (!this->visit(Init))
           return false;
 
         if (!this->emitInitField(*T, FieldToInit->Offset, Initializer))
           return false;
       }
       ++InitIndex;
     }
 
     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));
 
+    // TODO: I'm pretty sure the RVO business here isn't general enough.
     if (Func->hasRVO()) {
       // RVO functions expect a pointer to initialize on the stack.
       // Dup our existing pointer so it has its own copy to use.
       // TODO: Instead of doing this, we could just skip removing
       //   the RVO pointer from the stack.
       if (!this->emitDupPtr(Initializer))
         return false;
 
       return this->visit(CE);
     }
   }
-
   return false;
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitInitializer(const Expr *Initializer) {
   QualType InitializerType = Initializer->getType();
 
   if (InitializerType->isArrayType())

 }
 
 template <class Emitter>
 Record *ByteCodeExprGen<Emitter>::getRecord(const RecordDecl *RD) {
   return P.getOrCreateRecord(RD);
 }
 
 template <class Emitter>
+Function *ByteCodeExprGen<Emitter>::getFunction(const FunctionDecl *FD) {
+  assert(FD);
+  Function *Func = P.getFunction(FD);
+
+  if (!Func) {
+    if (auto R = ByteCodeStmtGen<ByteCodeEmitter>(Ctx, P).compileFunc(FD))
+      Func = *R;
+  }
+
+  return Func;
+}
+
+template <class Emitter>
 bool ByteCodeExprGen<Emitter>::visitExpr(const Expr *Exp) {
   ExprScope<Emitter> RootScope(this);
   if (!visit(Exp))
     return false;
 
   if (Optional<PrimType> T = classify(Exp))
     return this->emitRet(*T, Exp);
   else

 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitCallExpr(const CallExpr *E) {
   assert(!E->getBuiltinCallee() && "Builtin functions aren't supported yet");
 
   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 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.
     // If the function is already fully compiled but not constexpr, it was
     // found to be faulty earlier on, so bail out.
     if (Func->isFullyCompiled() && !Func->isConstexpr())
       return false;
 
     QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
     Optional<PrimType> T = classify(ReturnType);
-
-    if (T || ReturnType->isVoidType()) {
-      // Put arguments on the stack.
-      for (const auto *Arg : E->arguments()) {
-        if (!this->visit(Arg))
-          return false;
-      }
 
-      if (T)
-        return this->emitCall(*T, Func, E);
+    // Primitive return value, just call it.
+
+    // 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.
+    // TODO: Is this always the case?
   } else {
     assert(false && "We don't support non-FunctionDecl callees right now.");
   }
 
   return false;
 }
 
 template <class Emitter>
+bool ByteCodeExprGen<Emitter>::VisitCXXMemberCallExpr(
+    const CXXMemberCallExpr *E) {
+  // Get 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());
 }
 
 template <class Emitter>
 bool ByteCodeExprGen<Emitter>::VisitCXXDefaultArgExpr(
     const CXXDefaultArgExpr *E) {
bool ByteCodeExprGen<Emitter>::VisitCXXNullPtrLiteralExpr(
     const CXXNullPtrLiteralExpr *E) {
   if (DiscardResult)
     return true;
 
   return this->emitNullPtr(E);
 }
 
 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();
 
   switch (E->getOpcode()) {
   case UO_PostInc: // x++
   case UO_PostDec: // x--
   case UO_PreInc:  // --x
   case UO_PreDec:  // ++x

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

 } // namespace interp
 } // namespace clang
 
 template <class Emitter>
 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;
 
   // Emit a guard return to protect against a code path missing one.
   if (F->getReturnType()->isVoidType())
     return this->emitRetVoid(SourceInfo{});
   else
     return this->emitNoRet(SourceInfo{});

clang/lib/AST/Interp/Disasm.cpp

if (auto *Cons = dyn_cast<CXXConstructorDecl>(F)) {
       OS << F->getDeclName();
     }
     OS << " " << (void*)this << ":\n";
   } else {
     OS << "<<expr>>\n";
   }
 
   OS << "frame size: " << getFrameSize() << "\n";
-  OS << "arg size:   " << getArgSize() << "\n";
+  OS << "arg size:   " << getArgSize() << " (" << getNumParams() << ")\n";
   OS << "rvo:        " << hasRVO() << "\n";
+  OS << "this arg:   " << hasThisPointer() << "\n";
 
   auto PrintName = [&OS](const char *Name) {
     OS << Name;
     for (long I = 0, N = strlen(Name); I < 30 - N; ++I) {
       OS << ' ';
     }
   };
 

clang/lib/AST/Interp/EvalEmitter.cpp

if (!isActive())
     return true;
   using T = typename PrimConv<OpType>::T;
   return ReturnValue<T>(S.Stk.pop<T>(), Result);
 }
 
 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;
 }
 
 bool EvalEmitter::emitRetVoid(const SourceInfo &Info) { return true; }
 
 bool EvalEmitter::emitRetValue(const SourceInfo &Info) {
   // Method to recursively traverse composites.
   std::function<bool(QualType, const Pointer &, APValue &)> Composite;

clang/lib/AST/Interp/Function.h

private:
   /// Object descriptors in this block.
   LocalVectorTy Descriptors;
 };
 
 /// Bytecode function.
 ///
 /// 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 *>;
 
   /// Returns the size of the function's local stack.
   unsigned getFrameSize() const { return FrameSize; }
   /// Returns the size of the argument stack.
   unsigned getArgSize() const { return ArgSize; }
public:
 
   /// Returns the location.
   SourceLocation getLoc() const { return Loc; }
 
   /// Returns a parameter descriptor.
   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() {
     return llvm::make_range(Scopes.begin(), Scopes.end());
   }
 
   /// Range over argument types.
   using arg_reverse_iterator = SmallVectorImpl<PrimType>::reverse_iterator;
public:
   bool isVirtual() const;
 
   /// Checks if the function is a constructor.
   bool isConstructor() const { return isa<CXXConstructorDecl>(F); }
 
   /// 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,
                llvm::SmallVector<Scope, 2> &&NewScopes) {
     FrameSize = NewFrameSize;
     Code = std::move(NewCode);
     SrcMap = std::move(NewSrcMap);
     Scopes = std::move(NewScopes);
private:
   llvm::SmallVector<PrimType, 8> ParamTypes;
   /// Map from byte offset to parameter descriptor.
   llvm::DenseMap<unsigned, ParamDescriptor> Params;
   /// Flag to indicate if the function is valid.
   bool IsValid = false;
   /// 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().
   void dump() const;
   void dump(llvm::raw_ostream &OS) const;
 };
 
 } // namespace interp
 } // namespace clang
 
 #endif

clang/lib/AST/Interp/Function.cpp

 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 
 using namespace clang;
 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);
   assert(It != Params.end() && "Invalid parameter offset");
   return It->second;
 }
 
 SourceInfo Function::getSource(CodePtr PC) const {

clang/lib/AST/Interp/Interp.cpp

if (InterpFrame *Caller = S.Current->Caller) {
     if (!ReturnValue<T>(Ret, Result))
       return false;
   }
   return true;
 }
 
 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
   // have a caller set.
   return Interpret(S, CallResult);
 }
 
 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());
 
   return Success;
 }
 
 static bool RetVoid(InterpState &S, CodePtr &PC, APValue &Result) {
   S.CallStackDepth--;

clang/lib/AST/Interp/InterpFrame.h

 public:
   /// The frame of the previous function.
   InterpFrame *Caller;
 
   /// Creates a new frame for a method call.
   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();
 
   /// Invokes the destructors for a scope.
   void destroy(unsigned Idx);
 
   /// Pops the arguments off the stack.
   void popArgs();

clang/lib/AST/Interp/InterpFrame.cpp

if (unsigned FrameSize = Func->getFrameSize()) {
           Block *B = new (localBlock(Local.Offset)) Block(Local.Desc);
           B->invokeCtor();
         }
       }
     }
   }
 }
 
+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();
   for (auto &Param : Params)
     S.deallocate(reinterpret_cast<Block *>(Param.second.get()));
 }
 
 void InterpFrame::destroy(unsigned Idx) {

clang/lib/AST/Interp/Pointer.h

public:
 
   /// Checks if two pointers are comparable.
   static bool hasSameBase(const Pointer &A, const Pointer &B);
   /// Checks if two pointers can be subtracted.
   static bool hasSameArray(const Pointer &A, const Pointer &B);
 
   /// Prints the pointer.
   void print(llvm::raw_ostream &OS) const {
-    OS << "{" << Base << ", " << Offset << ", ";
+    OS << (void *)Pointee << "{" << Base << ", " << Offset << ", ";
     if (Pointee)
       OS << Pointee->getSize();
     else
       OS << "nullptr";
     OS << "}";
   }
 
 private:

clang/test/AST/Interp/records.cpp

constexpr int getA(const C &c) {
   return c.a;
 }
 static_assert(getA(c) == 100, "");
 
 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, "");