[Clang][Interp] Fix display of syntactically-invalid note for member function calls

[yronglin]

This PR fix display of syntactically-invalid note for member function calls to follow the behavior of current interpreter.

[llvmbot]

@llvm/pr-subscribers-clang

Author: None (yronglin)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/102170.diff

12 Files Affected:

• (modified) clang/lib/AST/Interp/Compiler.cpp (+10-9)
• (modified) clang/lib/AST/Interp/Context.cpp (+1-1)
• (modified) clang/lib/AST/Interp/EvalEmitter.cpp (+2-2)
• (modified) clang/lib/AST/Interp/Interp.cpp (+1-1)
• (modified) clang/lib/AST/Interp/Interp.h (+10-9)
• (modified) clang/lib/AST/Interp/InterpFrame.cpp (+22-9)
• (modified) clang/lib/AST/Interp/InterpFrame.h (+4-2)
• (modified) clang/lib/AST/Interp/Opcodes.td (+3-3)
• (modified) clang/test/AST/Interp/constexpr-nqueens.cpp (+1-1)
• (modified) clang/test/AST/Interp/lambda.cpp (+1-1)
• (modified) clang/test/AST/Interp/records.cpp (+2-2)
• (modified) clang/test/SemaCXX/constexpr-frame-describe.cpp (+5)

diff --git a/clang/lib/AST/Interp/Compiler.cpp b/clang/lib/AST/Interp/Compiler.cpp
index 02cbe38f5fb1fa..d0494a5065f5dc 100644
--- a/clang/lib/AST/Interp/Compiler.cpp
+++ b/clang/lib/AST/Interp/Compiler.cpp
@@ -2551,10 +2551,10 @@ bool Compiler<Emitter>::VisitCXXConstructExpr(const CXXConstructExpr *E) {
         VarArgSize +=
             align(primSize(classify(E->getArg(I)->getType()).value_or(PT_Ptr)));
       }
-      if (!this->emitCallVar(Func, VarArgSize, E))
+      if (!this->emitCallVar(Func, VarArgSize, E, E))
         return false;
     } else {
-      if (!this->emitCall(Func, 0, E))
+      if (!this->emitCall(Func, /*VarArgSize=*/0, E, E))
         return false;
     }
 
@@ -2588,7 +2588,7 @@ bool Compiler<Emitter>::VisitCXXConstructExpr(const CXXConstructExpr *E) {
           return false;
       }
 
-      if (!this->emitCall(Func, 0, E))
+      if (!this->emitCall(Func, /*VarArgSize=*/0, E, E))
         return false;
     }
     return true;
@@ -2799,7 +2799,7 @@ bool Compiler<Emitter>::VisitCXXInheritedCtorInitExpr(
     Offset += align(primSize(PT));
   }
 
-  return this->emitCall(F, 0, E);
+  return this->emitCall(F, /*VarArgSize=*/0, E, E);
 }
 
 template <class Emitter>
@@ -4087,7 +4087,7 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
       for (unsigned I = NumParams, N = E->getNumArgs(); I != N; ++I)
         VarArgSize += align(primSize(classify(E->getArg(I)).value_or(PT_Ptr)));
 
-      if (!this->emitCallVirt(Func, VarArgSize, E))
+      if (!this->emitCallVirt(Func, VarArgSize, E, E))
         return false;
     } else if (Func->isVariadic()) {
       uint32_t VarArgSize = 0;
@@ -4095,10 +4095,10 @@ bool Compiler<Emitter>::VisitCallExpr(const CallExpr *E) {
           Func->getNumWrittenParams() + isa<CXXOperatorCallExpr>(E);
       for (unsigned I = NumParams, N = E->getNumArgs(); I != N; ++I)
         VarArgSize += align(primSize(classify(E->getArg(I)).value_or(PT_Ptr)));
-      if (!this->emitCallVar(Func, VarArgSize, E))
+      if (!this->emitCallVar(Func, VarArgSize, E, E))
         return false;
     } else {
-      if (!this->emitCall(Func, 0, E))
+      if (!this->emitCall(Func, /*VarArgSize=*/0, E, E))
         return false;
     }
   } else {
@@ -4705,7 +4705,7 @@ bool Compiler<Emitter>::emitLambdaStaticInvokerBody(const CXXMethodDecl *MD) {
       return false;
   }
 
-  if (!this->emitCall(Func, 0, LambdaCallOp))
+  if (!this->emitCall(Func, /*VarArgSize=*/0, /*CE=*/nullptr, LambdaCallOp))
     return false;
 
   this->emitCleanup();
@@ -5567,7 +5567,8 @@ bool Compiler<Emitter>::emitRecordDestruction(const Record *R) {
     assert(DtorFunc->getNumParams() == 1);
     if (!this->emitDupPtr(SourceInfo{}))
       return false;
-    if (!this->emitCall(DtorFunc, 0, SourceInfo{}))
+    if (!this->emitCall(DtorFunc, /*VarArgSize=*/0, /*CE=*/nullptr,
+                        SourceInfo{}))
       return false;
   }
 
diff --git a/clang/lib/AST/Interp/Context.cpp b/clang/lib/AST/Interp/Context.cpp
index 92ac28137fdb45..6ea61bdc445539 100644
--- a/clang/lib/AST/Interp/Context.cpp
+++ b/clang/lib/AST/Interp/Context.cpp
@@ -206,7 +206,7 @@ bool Context::Run(State &Parent, const Function *Func, APValue &Result) {
   {
     InterpState State(Parent, *P, Stk, *this);
     State.Current = new InterpFrame(State, Func, /*Caller=*/nullptr, CodePtr(),
-                                    Func->getArgSize());
+                                    Func->getArgSize(), /*CE=*/nullptr);
     if (Interpret(State, Result)) {
       assert(Stk.empty());
       return true;
diff --git a/clang/lib/AST/Interp/EvalEmitter.cpp b/clang/lib/AST/Interp/EvalEmitter.cpp
index 08536536ac3c2e..d7e442f37e23f9 100644
--- a/clang/lib/AST/Interp/EvalEmitter.cpp
+++ b/clang/lib/AST/Interp/EvalEmitter.cpp
@@ -20,8 +20,8 @@ EvalEmitter::EvalEmitter(Context &Ctx, Program &P, State &Parent,
                          InterpStack &Stk)
     : Ctx(Ctx), P(P), S(Parent, P, Stk, Ctx, this), EvalResult(&Ctx) {
   // Create a dummy frame for the interpreter which does not have locals.
-  S.Current =
-      new InterpFrame(S, /*Func=*/nullptr, /*Caller=*/nullptr, CodePtr(), 0);
+  S.Current = new InterpFrame(S, /*Func=*/nullptr, /*Caller=*/nullptr,
+                              CodePtr(), /*ArgSize=*/0, /*CE=*/nullptr);
 }
 
 EvalEmitter::~EvalEmitter() {
diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 0f72b860ddad77..fe38cfb54fb34c 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -872,7 +872,7 @@ static bool runRecordDestructor(InterpState &S, CodePtr OpPC,
       return false;
 
     S.Stk.push<Pointer>(BasePtr);
-    if (!Call(S, OpPC, DtorFunc, 0))
+    if (!Call(S, OpPC, DtorFunc, /*VarArgSize=*/0, /*CE=*/nullptr))
       return false;
   }
 
diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 2eed0d3d1f16b3..bfe1c24288924a 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -2513,7 +2513,7 @@ inline bool ArrayDecay(InterpState &S, CodePtr OpPC) {
 }
 
 inline bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
-                    uint32_t VarArgSize) {
+                    uint32_t VarArgSize, const Expr *CE) {
   if (Func->hasThisPointer()) {
     size_t ArgSize = Func->getArgSize() + VarArgSize;
     size_t ThisOffset = ArgSize - (Func->hasRVO() ? primSize(PT_Ptr) : 0);
@@ -2540,7 +2540,7 @@ inline bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
   if (!CheckCallDepth(S, OpPC))
     return false;
 
-  auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC, VarArgSize);
+  auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC, VarArgSize, CE);
   InterpFrame *FrameBefore = S.Current;
   S.Current = NewFrame.get();
 
@@ -2563,7 +2563,7 @@ inline bool CallVar(InterpState &S, CodePtr OpPC, const Function *Func,
 }
 
 inline bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
-                 uint32_t VarArgSize) {
+                 uint32_t VarArgSize, const Expr *CE) {
   if (Func->hasThisPointer()) {
     size_t ArgSize = Func->getArgSize() + VarArgSize;
     size_t ThisOffset = ArgSize - (Func->hasRVO() ? primSize(PT_Ptr) : 0);
@@ -2591,7 +2591,7 @@ inline bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
   if (!CheckCallDepth(S, OpPC))
     return false;
 
-  auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC, VarArgSize);
+  auto NewFrame = std::make_unique<InterpFrame>(S, Func, OpPC, VarArgSize, CE);
   InterpFrame *FrameBefore = S.Current;
   S.Current = NewFrame.get();
 
@@ -2612,7 +2612,7 @@ inline bool Call(InterpState &S, CodePtr OpPC, const Function *Func,
 }
 
 inline bool CallVirt(InterpState &S, CodePtr OpPC, const Function *Func,
-                     uint32_t VarArgSize) {
+                     uint32_t VarArgSize, const Expr *CE) {
   assert(Func->hasThisPointer());
   assert(Func->isVirtual());
   size_t ArgSize = Func->getArgSize() + VarArgSize;
@@ -2659,7 +2659,7 @@ inline bool CallVirt(InterpState &S, CodePtr OpPC, const Function *Func,
     }
   }
 
-  if (!Call(S, OpPC, Func, VarArgSize))
+  if (!Call(S, OpPC, Func, VarArgSize, CE))
     return false;
 
   // Covariant return types. The return type of Overrider is a pointer
@@ -2686,7 +2686,8 @@ inline bool CallVirt(InterpState &S, CodePtr OpPC, const Function *Func,
 
 inline bool CallBI(InterpState &S, CodePtr &PC, const Function *Func,
                    const CallExpr *CE) {
-  auto NewFrame = std::make_unique<InterpFrame>(S, Func, PC);
+  auto NewFrame =
+      std::make_unique<InterpFrame>(S, Func, PC, /*VarArgSize=*/0, CE);
 
   InterpFrame *FrameBefore = S.Current;
   S.Current = NewFrame.get();
@@ -2737,9 +2738,9 @@ inline bool CallPtr(InterpState &S, CodePtr OpPC, uint32_t ArgSize,
     VarArgSize -= align(primSize(PT_Ptr));
 
   if (F->isVirtual())
-    return CallVirt(S, OpPC, F, VarArgSize);
+    return CallVirt(S, OpPC, F, VarArgSize, CE);
 
-  return Call(S, OpPC, F, VarArgSize);
+  return Call(S, OpPC, F, VarArgSize, CE);
 }
 
 inline bool GetFnPtr(InterpState &S, CodePtr OpPC, const Function *Func) {
diff --git a/clang/lib/AST/Interp/InterpFrame.cpp b/clang/lib/AST/Interp/InterpFrame.cpp
index 83784db91f4f3e..1d285dfb5f6210 100644
--- a/clang/lib/AST/Interp/InterpFrame.cpp
+++ b/clang/lib/AST/Interp/InterpFrame.cpp
@@ -18,15 +18,17 @@
 #include "Program.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
 
 using namespace clang;
 using namespace clang::interp;
 
 InterpFrame::InterpFrame(InterpState &S, const Function *Func,
-                         InterpFrame *Caller, CodePtr RetPC, unsigned ArgSize)
+                         InterpFrame *Caller, CodePtr RetPC, unsigned ArgSize,
+                         const clang::Expr *CE)
     : Caller(Caller), S(S), Depth(Caller ? Caller->Depth + 1 : 0), Func(Func),
-      RetPC(RetPC), ArgSize(ArgSize), Args(static_cast<char *>(S.Stk.top())),
-      FrameOffset(S.Stk.size()) {
+      CallExpr(CE), RetPC(RetPC), ArgSize(ArgSize),
+      Args(static_cast<char *>(S.Stk.top())), FrameOffset(S.Stk.size()) {
   if (!Func)
     return;
 
@@ -46,8 +48,9 @@ InterpFrame::InterpFrame(InterpState &S, const Function *Func,
 }
 
 InterpFrame::InterpFrame(InterpState &S, const Function *Func, CodePtr RetPC,
-                         unsigned VarArgSize)
-    : InterpFrame(S, Func, S.Current, RetPC, Func->getArgSize() + VarArgSize) {
+                         unsigned VarArgSize, const clang::Expr *CE)
+    : InterpFrame(S, Func, S.Current, RetPC, Func->getArgSize() + VarArgSize,
+                  CE) {
   // As per our calling convention, the this pointer is
   // part of the ArgSize.
   // If the function has RVO, the RVO pointer is first.
@@ -170,10 +173,20 @@ void InterpFrame::describe(llvm::raw_ostream &OS) const {
     return;
 
   const FunctionDecl *F = getCallee();
-  if (const auto *M = dyn_cast<CXXMethodDecl>(F);
-      M && M->isInstance() && !isa<CXXConstructorDecl>(F)) {
-    print(OS, This, S.getCtx(), S.getCtx().getRecordType(M->getParent()));
-    OS << "->";
+  if (const auto *MCE = dyn_cast_if_present<CXXMemberCallExpr>(CallExpr)) {
+    const Expr *Object = MCE->getImplicitObjectArgument();
+    Object->printPretty(OS, /*Helper=*/nullptr, S.getCtx().getPrintingPolicy(),
+                        /*Indentation=*/0);
+    if (Object->getType()->isPointerType())
+      OS << "->";
+    else
+      OS << ".";
+  } else if (const auto *OCE =
+                 dyn_cast_if_present<CXXOperatorCallExpr>(CallExpr)) {
+    OCE->getArg(0)->printPretty(OS, /*Helper=*/nullptr,
+                                S.getCtx().getPrintingPolicy(),
+                                /*Indentation=*/0);
+    OS << ".";
   }
 
   F->getNameForDiagnostic(OS, S.getCtx().getPrintingPolicy(),
diff --git a/clang/lib/AST/Interp/InterpFrame.h b/clang/lib/AST/Interp/InterpFrame.h
index 91b9b41b5d3343..6f68275faa3a97 100644
--- a/clang/lib/AST/Interp/InterpFrame.h
+++ b/clang/lib/AST/Interp/InterpFrame.h
@@ -30,14 +30,14 @@ class InterpFrame final : public Frame {
 
   /// Creates a new frame for a method call.
   InterpFrame(InterpState &S, const Function *Func, InterpFrame *Caller,
-              CodePtr RetPC, unsigned ArgSize);
+              CodePtr RetPC, unsigned ArgSize, const Expr *CE);
 
   /// Creates a new frame with the values that make sense.
   /// I.e., the caller is the current frame of S,
   /// 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,
-              unsigned VarArgSize = 0);
+              unsigned VarArgSize = 0, const Expr *CE = nullptr);
 
   /// Destroys the frame, killing all live pointers to stack slots.
   ~InterpFrame();
@@ -152,6 +152,8 @@ class InterpFrame final : public Frame {
   unsigned Depth;
   /// Reference to the function being executed.
   const Function *Func;
+  /// The syntactical structure of member function calls
+  const Expr *CallExpr;
   /// Current object pointer for methods.
   Pointer This;
   /// Pointer the non-primitive return value gets constructed in.
diff --git a/clang/lib/AST/Interp/Opcodes.td b/clang/lib/AST/Interp/Opcodes.td
index 220dff0c556b18..9aabd9198f129e 100644
--- a/clang/lib/AST/Interp/Opcodes.td
+++ b/clang/lib/AST/Interp/Opcodes.td
@@ -198,11 +198,11 @@ def NoRet : Opcode {}
 
 
 def Call : Opcode {
-  let Args = [ArgFunction, ArgUint32];
+  let Args = [ArgFunction, ArgUint32, ArgExpr];
 }
 
 def CallVirt : Opcode {
-  let Args = [ArgFunction, ArgUint32];
+  let Args = [ArgFunction, ArgUint32, ArgExpr];
 }
 
 def CallBI : Opcode {
@@ -214,7 +214,7 @@ def CallPtr : Opcode {
 }
 
 def CallVar : Opcode {
-  let Args = [ArgFunction, ArgUint32];
+  let Args = [ArgFunction, ArgUint32, ArgExpr];
 }
 
 def OffsetOf : Opcode {
diff --git a/clang/test/AST/Interp/constexpr-nqueens.cpp b/clang/test/AST/Interp/constexpr-nqueens.cpp
index 971f99a032b665..ed038dbc9b0779 100644
--- a/clang/test/AST/Interp/constexpr-nqueens.cpp
+++ b/clang/test/AST/Interp/constexpr-nqueens.cpp
@@ -49,7 +49,7 @@ constexpr Board buildBoardScan(int N, int Col, int Row, const Board &B) {
   return Row == N ? Board(0, true) :
          B.ok(Row, Col) ?
          tryBoard(buildBoardRecurse(N, Col + 1, B.addQueen(Row, Col)), // ref-note {{in call to 'B.addQueen(0, 0)}} \
-                                                                       // expected-note {{in call to '&Board()->addQueen(0, 0)}}
+                                                                       // expected-note {{in call to 'B.addQueen(0, 0)}}
                   N, Col, Row+1, B) :
          buildBoardScan(N, Col, Row + 1, B);
 }
diff --git a/clang/test/AST/Interp/lambda.cpp b/clang/test/AST/Interp/lambda.cpp
index d68fe995e8fa1c..27bbebddf44804 100644
--- a/clang/test/AST/Interp/lambda.cpp
+++ b/clang/test/AST/Interp/lambda.cpp
@@ -46,7 +46,7 @@ constexpr int div(int a, int b) {
     return a / b; // both-note {{division by zero}}
   };
 
-  return f(); // expected-note {{in call to '&f->operator()()'}} \
+  return f(); // expected-note {{in call to 'f.operator()()'}} \
               // ref-note {{in call to 'f.operator()()'}}
 }
 static_assert(div(8, 2) == 4);
diff --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index 479c0487fecae0..0735d4765a22aa 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -335,7 +335,7 @@ namespace InitializerTemporaries {
 
   constexpr int f() {
     S{}; // ref-note {{in call to 'S{}.~S()'}} \
-         // expected-note {{in call to '&S{}->~S()'}}
+         // expected-note {{in call to '~S()'}}
     return 12;
   }
   static_assert(f() == 12); // both-error {{not an integral constant expression}} \
@@ -599,7 +599,7 @@ namespace Destructors {
   };
   constexpr int testS() {
     S{}; // ref-note {{in call to 'S{}.~S()'}} \
-         // expected-note {{in call to '&S{}->~S()'}}
+         // expected-note {{in call to '~S()'}}
     return 1;
   }
   static_assert(testS() == 1); // both-error {{not an integral constant expression}} \
diff --git a/clang/test/SemaCXX/constexpr-frame-describe.cpp b/clang/test/SemaCXX/constexpr-frame-describe.cpp
index 7b832d9a4b4a18..b305c281f92e5d 100644
--- a/clang/test/SemaCXX/constexpr-frame-describe.cpp
+++ b/clang/test/SemaCXX/constexpr-frame-describe.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -DNEW_CONST_INTERP -std=c++20 -fexperimental-new-constant-interpreter -fsyntax-only -verify %s
 
 
 struct Foo {
@@ -67,9 +68,11 @@ struct Bar {
   template <typename U, int num>
   constexpr int fail2() const { return 1 / 0; } // expected-warning {{division by zero}} \
                                                 // expected-note {{division by zero}}
+#ifndef NEW_CONST_INTERP
   template <typename ...Args>
   constexpr int fail3(Args... args) const { return 1 / 0; } // expected-warning {{division by zero}} \
                                                 // expected-note {{division by zero}}
+#endif
 };
 
 constexpr Bar<int> bar;
@@ -77,5 +80,7 @@ static_assert(bar.fail1<int>()); // expected-error {{constant expression}} \
                                  // expected-note {{in call to 'bar.fail1<int>()'}}
 static_assert(bar.fail2<int*, 42>()); // expected-error {{constant expression}} \
                                       // expected-note {{in call to 'bar.fail2<int *, 42>()'}}
+#ifndef NEW_CONST_INTERP
 static_assert(bar.fail3(3, 4UL, bar, &bar)); // expected-error {{constant expression}} \
                                              // expected-note {{in call to 'bar.fail3<int, unsigned long, Bar<int>, const Bar<int> *>(3, 4, {}, &bar)'}}
+#endif

[tbaederr]

LGTM otherwise. The CI currently fails on Windows, but that looks like it's using an old version of the test?

[tbaederr]

And FYI, no additional regressions reported:

[yronglin]

LGTM otherwise. The CI currently fails on Windows, but that looks like it's using an old version of the test?

Thanks! Some local changes were missed last time and were not committed.