[clang][Interp] Explicitly handle RVO Pointer

[tbaeder]

Summary

The calling convention is:

[RVO pointer]
[instance pointer]
[... args ...]

We handle the instance pointer ourselves, BUT for the RVO pointer, we
just assumed in visitReturnStmt() that it is on top of the stack. Which
isn't true if there are other args present (and a this pointer, maybe).

Fix this by recording the RVO pointer explicitly when creating an
InterpFrame, just like we do with the instance/This pointer.

There is already a "RVOAndParams()" test in test/AST/Inter/records.cpp,
that was supposed to test this, however, it didn't trigger any
problematic behavior because the parameter and the return value have the
same type.

[tbaeder]

Ping

[aaron.ballman]

LGTM!

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

if (const Expr *RE = RS->getRetValue()) {
     if (ReturnType) {
       // Primitive types are simply returned.
       if (!this->visit(RE))
         return false;
       this->emitCleanup();
       return this->emitRet(*ReturnType, RS);
     } else {
       // RVO - construct the value in the return location.
+      if (!this->emitRVOPtr(RE))
+        return false;
       if (!this->visitInitializer(RE))
         return false;
+      if (!this->emitPopPtr(RE))
+        return false;
+
       this->emitCleanup();
       return this->emitRetVoid(RS);
     }
   }
 
   // Void return.
   this->emitCleanup();
   return this->emitRetVoid(RS);

clang/lib/AST/Interp/Interp.h

inline bool This(InterpState &S, CodePtr OpPC) {
   const Pointer &This = S.Current->getThis();
   if (!CheckThis(S, OpPC, This))
     return false;
 
   S.Stk.push<Pointer>(This);
   return true;
 }
 
+inline bool RVOPtr(InterpState &S, CodePtr OpPC) {
+  assert(S.Current->getFunction()->hasRVO());
+  S.Stk.push<Pointer>(S.Current->getRVOPtr());
+  return true;
+}
+
 //===----------------------------------------------------------------------===//
 // Shr, Shl
 //===----------------------------------------------------------------------===//
 
 template <PrimType NameL, PrimType NameR>
 inline bool Shr(InterpState &S, CodePtr OpPC) {
   using LT = typename PrimConv<NameL>::T;
   using RT = typename PrimConv<NameR>::T;

clang/lib/AST/Interp/InterpFrame.h

public:
   InterpFrame *Caller;
 
   /// Creates a new frame for a method call.
   InterpFrame(InterpState &S, const 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,
+  /// the This() pointer is the current Pointer on the top of S's stack,
+  /// and the RVO pointer is before that.
   InterpFrame(InterpState &S, const 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);
 
public:
   }
 
   /// Returns a pointer to an argument - lazily creates a block.
   Pointer getParamPointer(unsigned Offset);
 
   /// Returns the 'this' pointer.
   const Pointer &getThis() const { return This; }
 
+  /// Returns the RVO pointer, if the Function has one.
+  const Pointer &getRVOPtr() const { return RVOPtr; }
+
   /// Checks if the frame is a root frame - return should quit the interpreter.
   bool isRoot() const { return !Func; }
 
   /// Returns the PC of the frame's code start.
   CodePtr getPC() const { return Func->getCodeBegin(); }
 
   /// Returns the return address of the frame.
   CodePtr getRetPC() const { return RetPC; }

 
 private:
   /// Reference to the interpreter state.
   InterpState &S;
   /// Reference to the function being executed.
   const Function *Func;
   /// Current object pointer for methods.
   Pointer This;
+  /// Pointer the non-primitive return value gets constructed in.
+  Pointer RVOPtr;
   /// Return address.
   CodePtr RetPC;
   /// The size of all the arguments.
   const unsigned ArgSize;
   /// Pointer to the arguments in the callee's frame.
   char *Args = nullptr;
   /// Fixed, initial storage for known local variables.
   std::unique_ptr<char[]> Locals;

clang/lib/AST/Interp/InterpFrame.cpp

InterpFrame::InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC)
   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->hasRVO())
+    RVOPtr = stackRef<Pointer>(0);
+
   if (Func->hasThisPointer()) {
     if (Func->hasRVO())
       This = stackRef<Pointer>(sizeof(Pointer));
     else
       This = stackRef<Pointer>(0);
   }
 
   if (unsigned FrameSize = Func->getFrameSize()) {

clang/lib/AST/Interp/Opcodes.td

 // [] -> [Pointer]
 def GetPtrThisVirtBase : Opcode {
   // RecordDecl of base class.
   let Args = [ArgRecordDecl];
 }
 // [] -> [Pointer]
 def This : Opcode;
 
+// [] -> [Pointer]
+def RVOPtr : Opcode;
+
 // [Pointer] -> [Pointer]
 def NarrowPtr : Opcode;
 // [Pointer] -> [Pointer]
 def ExpandPtr : Opcode;
 
 //===----------------------------------------------------------------------===//
 // Direct field accessors
 //===----------------------------------------------------------------------===//

clang/test/AST/Interp/records.cpp

 
 #if __cplusplus >= 201703L
 // FIXME: In c++14, this uses a MaterializeTemporaryExpr,
 //   which the new interpreter doesn't support yet.
 constexpr C RVOAndParams(const C *c) {
   return C();
 }
 constexpr C RVOAndParamsResult = RVOAndParams(&c);
+
+/// Parameter and return value have different types.
+constexpr C RVOAndParams(int a) {
+  return C();
+}
+constexpr C RVOAndParamsResult2 = RVOAndParams(12);
 #endif
 
 class Bar { // expected-note {{definition of 'Bar' is not complete}} \
             // ref-note {{definition of 'Bar' is not complete}}
 public:
   constexpr Bar(){}
   constexpr Bar b; // expected-error {{cannot be constexpr}} \
                    // expected-error {{has incomplete type 'const Bar'}} \