[clang] f6be96a - [clang][ExprConstant] Fix display of syntactically-invalid note for member function calls

Takuya Shimizu via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 27 08:20:24 PDT 2023


Author: Takuya Shimizu
Date: 2023-06-28T00:19:46+09:00
New Revision: f6be96aa4e5e282ea52041ad6fe5fff13a3df103

URL: https://github.com/llvm/llvm-project/commit/f6be96aa4e5e282ea52041ad6fe5fff13a3df103
DIFF: https://github.com/llvm/llvm-project/commit/f6be96aa4e5e282ea52041ad6fe5fff13a3df103.diff

LOG: [clang][ExprConstant] Fix display of syntactically-invalid note for member function calls

This patch makes the display of member function calls more true to the user-written code by making use of the syntactical structure of the function calls.
This patch also changes the display of conventional value-based printing from arrow operator to dot operator.
This avoids the syntactical invalidness in notes previously caused by the display of & operator
(lack of parentheses and reference of rvalue)

Fixes https://github.com/llvm/llvm-project/issues/57081

Reviewed By: cjdb
Differential Revision: https://reviews.llvm.org/D151720

Added: 
    clang/test/SemaCXX/constexpr-frame-describe.cpp

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/AST/ExprConstant.cpp
    clang/test/AST/Interp/constexpr-nqueens.cpp
    clang/test/AST/Interp/lambda.cpp
    clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
    clang/test/CXX/temp/temp.param/p8-cxx20.cpp
    clang/test/SemaCXX/constant-expression-cxx11.cpp
    clang/test/SemaCXX/cxx2a-consteval.cpp
    clang/test/SemaCXX/deduced-return-type-cxx14.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1ecf620f2a768..f00082fad24dd 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -371,6 +371,10 @@ Improvements to Clang's diagnostics
   ``#pragma clang|GCC diagnostic push|pop`` directive.
   (`#13920: <https://github.com/llvm/llvm-project/issues/13920>`_)
 - Clang now does not try to analyze cast validity on variables with dependent alignment (`#63007: <https://github.com/llvm/llvm-project/issues/63007>`_).
+- Clang constexpr evaluator now displays member function calls more precisely
+  by making use of the syntactical structure of function calls. This avoids display
+  of syntactically invalid codes in diagnostics.
+  (`#57081: <https://github.com/llvm/llvm-project/issues/57081>`_)
 
 Bug Fixes in This Version
 -------------------------

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 19d8e24fef63c..777245f86bdff 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -531,6 +531,9 @@ namespace {
     /// This - The binding for the this pointer in this call, if any.
     const LValue *This;
 
+    /// CallExpr - The syntactical structure of member function calls
+    const Expr *CallExpr;
+
     /// Information on how to find the arguments to this call. Our arguments
     /// are stored in our parent's CallStackFrame, using the ParmVarDecl* as a
     /// key and this value as the version.
@@ -584,7 +587,7 @@ namespace {
 
     CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
                    const FunctionDecl *Callee, const LValue *This,
-                   CallRef Arguments);
+                   const Expr *CallExpr, CallRef Arguments);
     ~CallStackFrame();
 
     // Return the temporary for Key whose version number is Version.
@@ -978,7 +981,9 @@ namespace {
           CallStackDepth(0), NextCallIndex(1),
           StepsLeft(C.getLangOpts().ConstexprStepLimit),
           EnableNewConstInterp(C.getLangOpts().EnableNewConstInterp),
-          BottomFrame(*this, SourceLocation(), nullptr, nullptr, CallRef()),
+          BottomFrame(*this, SourceLocation(), /*Callee=*/nullptr,
+                      /*This=*/nullptr,
+                      /*CallExpr=*/nullptr, CallRef()),
           EvaluatingDecl((const ValueDecl *)nullptr),
           EvaluatingDeclValue(nullptr), HasActiveDiagnostic(false),
           HasFoldFailureDiagnostic(false), EvalMode(Mode) {}
@@ -1436,9 +1441,10 @@ void SubobjectDesignator::diagnosePointerArithmetic(EvalInfo &Info,
 
 CallStackFrame::CallStackFrame(EvalInfo &Info, SourceLocation CallLoc,
                                const FunctionDecl *Callee, const LValue *This,
-                               CallRef Call)
+                               const Expr *CallExpr, CallRef Call)
     : Info(Info), Caller(Info.CurrentCall), Callee(Callee), This(This),
-      Arguments(Call), CallLoc(CallLoc), Index(Info.NextCallIndex++) {
+      CallExpr(CallExpr), Arguments(Call), CallLoc(CallLoc),
+      Index(Info.NextCallIndex++) {
   Info.CurrentCall = this;
   ++Info.CallStackDepth;
 }
@@ -1918,12 +1924,29 @@ void CallStackFrame::describe(raw_ostream &Out) {
     Out << *Callee << '(';
 
   if (This && IsMemberCall) {
-    APValue Val;
-    This->moveInto(Val);
-    Val.printPretty(Out, Info.Ctx,
-                    This->Designator.MostDerivedType);
-    // FIXME: Add parens around Val if needed.
-    Out << "->" << *Callee << '(';
+    if (const auto *MCE = dyn_cast_if_present<CXXMemberCallExpr>(CallExpr)) {
+      const Expr *Object = MCE->getImplicitObjectArgument();
+      Object->printPretty(Out, /*Helper=*/nullptr, Info.Ctx.getPrintingPolicy(),
+                          /*Indentation=*/0);
+      if (Object->getType()->isPointerType())
+          Out << "->";
+      else
+          Out << ".";
+    } else if (const auto *OCE =
+                   dyn_cast_if_present<CXXOperatorCallExpr>(CallExpr)) {
+      OCE->getArg(0)->printPretty(Out, /*Helper=*/nullptr,
+                                  Info.Ctx.getPrintingPolicy(),
+                                  /*Indentation=*/0);
+      Out << ".";
+    } else {
+      APValue Val;
+      This->moveInto(Val);
+      Val.printPretty(
+          Out, Info.Ctx,
+          Info.Ctx.getLValueReferenceType(This->Designator.MostDerivedType));
+      Out << ".";
+    }
+    Out << *Callee << '(';
     IsMemberCall = false;
   }
 
@@ -6161,13 +6184,13 @@ static bool handleTrivialCopy(EvalInfo &Info, const ParmVarDecl *Param,
 /// Evaluate a function call.
 static bool HandleFunctionCall(SourceLocation CallLoc,
                                const FunctionDecl *Callee, const LValue *This,
-                               ArrayRef<const Expr *> Args, CallRef Call,
-                               const Stmt *Body, EvalInfo &Info,
+                               const Expr *E, ArrayRef<const Expr *> Args,
+                               CallRef Call, const Stmt *Body, EvalInfo &Info,
                                APValue &Result, const LValue *ResultSlot) {
   if (!Info.CheckCallLimit(CallLoc))
     return false;
 
-  CallStackFrame Frame(Info, CallLoc, Callee, This, Call);
+  CallStackFrame Frame(Info, CallLoc, Callee, This, E, Call);
 
   // For a trivial copy or move assignment, perform an APValue copy. This is
   // essential for unions, where the operations performed by the assignment
@@ -6232,7 +6255,7 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This,
       Info,
       ObjectUnderConstruction{This.getLValueBase(), This.Designator.Entries},
       RD->getNumBases());
-  CallStackFrame Frame(Info, CallLoc, Definition, &This, Call);
+  CallStackFrame Frame(Info, CallLoc, Definition, &This, E, Call);
 
   // FIXME: Creating an APValue just to hold a nonexistent return value is
   // wasteful.
@@ -6533,7 +6556,8 @@ static bool HandleDestructionImpl(EvalInfo &Info, SourceLocation CallLoc,
   if (!CheckConstexprFunction(Info, CallLoc, DD, Definition, Body))
     return false;
 
-  CallStackFrame Frame(Info, CallLoc, Definition, &This, CallRef());
+  CallStackFrame Frame(Info, CallLoc, Definition, &This, /*CallExpr=*/nullptr,
+                       CallRef());
 
   // We're now in the period of destruction of this object.
   unsigned BasesLeft = RD->getNumBases();
@@ -7802,7 +7826,7 @@ class ExprEvaluatorBase
     Stmt *Body = FD->getBody(Definition);
 
     if (!CheckConstexprFunction(Info, E->getExprLoc(), FD, Definition, Body) ||
-        !HandleFunctionCall(E->getExprLoc(), Definition, This, Args, Call,
+        !HandleFunctionCall(E->getExprLoc(), Definition, This, E, Args, Call,
                             Body, Info, Result, ResultSlot))
       return false;
 
@@ -16204,7 +16228,8 @@ bool Expr::EvaluateWithSubstitution(APValue &Value, ASTContext &Ctx,
   Info.EvalStatus.HasSideEffects = false;
 
   // Build fake call to Callee.
-  CallStackFrame Frame(Info, Callee->getLocation(), Callee, ThisPtr, Call);
+  CallStackFrame Frame(Info, Callee->getLocation(), Callee, ThisPtr, This,
+                       Call);
   // FIXME: Missing ExprWithCleanups in enable_if conditions?
   FullExpressionRAII Scope(Info);
   return Evaluate(Value, Info, this) && Scope.destroy() &&
@@ -16261,7 +16286,8 @@ bool Expr::isPotentialConstantExpr(const FunctionDecl *FD,
   } else {
     SourceLocation Loc = FD->getLocation();
     HandleFunctionCall(Loc, FD, (MD && MD->isInstance()) ? &This : nullptr,
-                       Args, CallRef(), FD->getBody(), Info, Scratch, nullptr);
+                       &VIE, Args, CallRef(), FD->getBody(), Info, Scratch,
+                       /*ResultSlot=*/nullptr);
   }
 
   return Diags.empty();
@@ -16283,7 +16309,8 @@ bool Expr::isPotentialConstantExprUnevaluated(Expr *E,
   Info.CheckingPotentialConstantExpression = true;
 
   // Fabricate a call stack frame to give the arguments a plausible cover story.
-  CallStackFrame Frame(Info, SourceLocation(), FD, /*This*/ nullptr, CallRef());
+  CallStackFrame Frame(Info, SourceLocation(), FD, /*This=*/nullptr,
+                       /*CallExpr=*/nullptr, CallRef());
 
   APValue ResultScratch;
   Evaluate(ResultScratch, Info, E);

diff  --git a/clang/test/AST/Interp/constexpr-nqueens.cpp b/clang/test/AST/Interp/constexpr-nqueens.cpp
index c71092af069bb..8b8b608fc8c57 100644
--- a/clang/test/AST/Interp/constexpr-nqueens.cpp
+++ b/clang/test/AST/Interp/constexpr-nqueens.cpp
@@ -48,7 +48,7 @@ constexpr Board tryBoard(const Board &Try,
 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 '&Board()->addQueen(0, 0)}} \
+         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)}}
                   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 49e2290743285..c588da530e4d0 100644
--- a/clang/test/AST/Interp/lambda.cpp
+++ b/clang/test/AST/Interp/lambda.cpp
@@ -48,7 +48,7 @@ constexpr int div(int a, int b) {
   };
 
   return f(); // expected-note {{in call to '&f->operator()()'}} \
-              // ref-note {{in call to '&f->operator()()'}}
+              // ref-note {{in call to 'f.operator()()'}}
 }
 static_assert(div(8, 2) == 4);
 static_assert(div(8, 0) == 4); // expected-error {{not an integral constant expression}} \

diff  --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
index b1d80e3c428d3..270bc3cb48be9 100644
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
@@ -42,12 +42,30 @@ struct A {
   bool ok;
   constexpr A(bool ok) : ok(ok) {}
   constexpr ~A() noexcept(false) {
-    void oops(); // expected-note 2{{declared here}}
-    if (!ok) oops(); // expected-note 2{{non-constexpr function}}
+    void oops(); // expected-note 6{{declared here}}
+    if (!ok) oops(); // expected-note 6{{non-constexpr function}}
   }
 };
 
+struct B {
+  A a[2];
+  constexpr B(bool ok) : a{A(!ok), A(ok)}{}
+};
+
+struct Cons {
+  bool val[2];
+  constexpr Cons() : val{true, false} {}
+};
+
 constexpr A const_dtor(true);
+static_assert(B(false).a[1].ok); // expected-error {{static assertion expression is not an integral constant expression}} \
+                                    expected-note {{in call to 'B(false).a[1].~A()'}} expected-note {{in call to 'B(false).~B()'}}
+static_assert(B(true).a[1].ok); // expected-error {{static assertion expression is not an integral constant expression}} \
+                                   expected-note {{in call to 'B(true).a[0].~A()'}} expected-note {{in call to 'B(true).~B()'}}
+static_assert(B(Cons().val[1]).a[1].ok); // expected-error {{static assertion expression is not an integral constant expression}} \
+                                            expected-note {{in call to 'B(Cons().val[1]).a[1].~A()'}} expected-note {{in call to 'B(Cons().val[1]).~B()'}}
+static_assert(B((new Cons)->val[0]).a[1].ok); // expected-error {{static assertion expression is not an integral constant expression}} \
+                                                 expected-note {{in call to 'B((new Cons)->val[0]).a[0].~A()'}} expected-note {{in call to 'B((new Cons)->val[0]).~B()'}}
 constexpr A non_const_dtor(false); // expected-error {{must have constant destruction}} expected-note {{in call}}
-constexpr A arr_dtor[5] = {true, true, true, false, true}; // expected-error {{must have constant destruction}} expected-note {{in call to '&arr_dtor[3]->~A()'}}
+constexpr A arr_dtor[5] = {true, true, true, false, true}; // expected-error {{must have constant destruction}} expected-note {{in call to 'arr_dtor[3].~A()'}}
 #endif

diff  --git a/clang/test/CXX/temp/temp.param/p8-cxx20.cpp b/clang/test/CXX/temp/temp.param/p8-cxx20.cpp
index eebf1a4b6fcca..a3478c0669661 100644
--- a/clang/test/CXX/temp/temp.param/p8-cxx20.cpp
+++ b/clang/test/CXX/temp/temp.param/p8-cxx20.cpp
@@ -61,5 +61,5 @@ namespace ConstDestruction {
 
   template<D d> struct Z {};
   Z<D{2, true}> z1;
-  Z<D{2, false}> z2; // expected-error {{non-type template argument is not a constant expression}} expected-note-re {{in call to '{{.*}}->~D()'}}
+  Z<D{2, false}> z2; // expected-error {{non-type template argument is not a constant expression}} expected-note-re {{in call to '{{.*}}.~D()'}}
 }

diff  --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index f454bba5afd8c..2a92eb6a3af4e 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -975,7 +975,7 @@ struct T : S {
   int n;
 };
 constexpr int S::f() const {
-  return static_cast<const T*>(this)->n; // expected-note {{cannot cast}}
+  return static_cast<const T*>(this)->n; // expected-note 5{{cannot cast}}
 }
 constexpr int S::g() const {
   // FIXME: Better diagnostic for this.
@@ -984,8 +984,20 @@ constexpr int S::g() const {
 // The T temporary is implicitly cast to an S subobject, but we can recover the
 // T full-object via a base-to-derived cast, or a derived-to-base-casted member
 // pointer.
-static_assert(S().f(), ""); // expected-error {{constant expression}} expected-note {{in call to '&S()->f()'}}
-static_assert(S().g(), ""); // expected-error {{constant expression}} expected-note {{in call to '&S()->g()'}}
+static_assert(S().f(), ""); // expected-error {{constant expression}} expected-note {{in call to 'S().f()'}}
+static_assert(S().g(), ""); // expected-error {{constant expression}} expected-note {{in call to 'S().g()'}}
+constexpr S sobj;
+constexpr const S& slref = sobj;
+constexpr const S&& srref = S();
+constexpr const S *sptr = &sobj;
+static_assert(sobj.f(), ""); // expected-error {{constant expression}} \
+                                expected-note {{in call to 'sobj.f()'}}
+static_assert(sptr->f(), ""); // expected-error {{constant expression}} \
+                                 expected-note {{in call to 'sptr->f()'}}
+static_assert(slref.f(), ""); // expected-error {{constant expression}} \
+                                 expected-note {{in call to 'slref.f()'}}
+static_assert(srref.f(), ""); // expected-error {{constant expression}} \
+                                 expected-note {{in call to 'srref.f()'}}
 static_assert(T(3).f() == 3, "");
 static_assert(T(4).g() == 4, "");
 

diff  --git a/clang/test/SemaCXX/constexpr-frame-describe.cpp b/clang/test/SemaCXX/constexpr-frame-describe.cpp
new file mode 100644
index 0000000000000..3a87232ebeabe
--- /dev/null
+++ b/clang/test/SemaCXX/constexpr-frame-describe.cpp
@@ -0,0 +1,60 @@
+// RUN: %clang_cc1 -std=c++20 -fsyntax-only -verify %s
+
+
+struct Foo {
+    constexpr void zomg() const { (void)(1 / 0); } // expected-error {{constant expression}} \
+                                                      expected-warning {{division by zero}} \
+                                                      expected-note 2{{division by zero}}
+};
+
+struct S {
+    constexpr S() {}
+    constexpr bool operator==(const S&) const { // expected-error {{never produces a constant expression}}
+      return 1 / 0; // expected-warning {{division by zero}} \
+                       expected-note 3{{division by zero}}
+    }
+
+    constexpr bool heh() const {
+        auto F = new Foo();
+        F->zomg(); // expected-note {{in call to 'F->zomg()'}}
+        delete F;
+        return false;
+    }
+};
+
+constexpr S s;
+
+static_assert(s.heh()); // expected-error {{constant expression}} \
+                           expected-note {{in call to 's.heh()'}}
+
+constexpr S s2;
+constexpr const S *sptr = &s;
+constexpr const S *sptr2 = &s2;
+static_assert(s == s2); // expected-error {{constant expression}} \
+                           expected-note {{in call to 's.operator==(s2)'}}
+static_assert(*sptr == *sptr2); // expected-error {{constant expression}} \
+                                   expected-note {{in call to '*sptr.operator==(s2)'}}
+
+struct A {
+  constexpr int foo() { (void)(1/0); return 1;} // expected-error {{never produces a constant expression}} \
+                                                   expected-warning {{division by zero}} \
+                                                   expected-note 2{{division by zero}}
+};
+
+struct B {
+  A aa;
+  A *a = &aa;
+};
+
+struct C {
+  B b;
+};
+
+struct D {
+  C cc;
+  C *c = &cc;
+};
+
+constexpr D d{};
+static_assert(d.c->b.a->foo() == 1); // expected-error {{constant expression}} \
+                                        expected-note {{in call to 'd.c->b.a->foo()'}}

diff  --git a/clang/test/SemaCXX/cxx2a-consteval.cpp b/clang/test/SemaCXX/cxx2a-consteval.cpp
index 1a949a286a00b..0c24fc7419698 100644
--- a/clang/test/SemaCXX/cxx2a-consteval.cpp
+++ b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -840,13 +840,13 @@ void func() {
   copy<foo> fail1{good0}; // expected-error {{call to consteval function 'defaulted_special_member_template::copy<defaulted_special_member_template::foo>::copy' is not a constant expression}} \
                              expected-note {{in call to 'copy(good0)'}}
   fail1 = good0;          // expected-error {{call to consteval function 'defaulted_special_member_template::copy<defaulted_special_member_template::foo>::operator=' is not a constant expression}} \
-                             expected-note {{in call to '&fail1->operator=(good0)'}}
+                             expected-note {{in call to 'fail1.operator=(good0)'}}
 
   move<foo> good1;
   move<foo> fail2{static_cast<move<foo>&&>(good1)}; // expected-error {{call to consteval function 'defaulted_special_member_template::move<defaulted_special_member_template::foo>::move' is not a constant expression}} \
                                                        expected-note {{in call to 'move(good1)'}}
   fail2 = static_cast<move<foo>&&>(good1);          // expected-error {{call to consteval function 'defaulted_special_member_template::move<defaulted_special_member_template::foo>::operator=' is not a constant expression}} \
-                                                       expected-note {{in call to '&fail2->operator=(good1)'}}
+                                                       expected-note {{in call to 'fail2.operator=(good1)'}}
 }
 } // namespace defaulted_special_member_template
 

diff  --git a/clang/test/SemaCXX/deduced-return-type-cxx14.cpp b/clang/test/SemaCXX/deduced-return-type-cxx14.cpp
index aa67f5368e77a..6344d1df3fbae 100644
--- a/clang/test/SemaCXX/deduced-return-type-cxx14.cpp
+++ b/clang/test/SemaCXX/deduced-return-type-cxx14.cpp
@@ -296,7 +296,7 @@ namespace Constexpr {
   void f() {
     X<int>().f();
     Y<void>().f();
-    constexpr int q = Y<int>().f(); // expected-error {{must be initialized by a constant expression}} expected-note {{in call to '&Y<int>()->f()'}}
+    constexpr int q = Y<int>().f(); // expected-error {{must be initialized by a constant expression}} expected-note {{in call to 'Y<int>().f()'}}
   }
   struct NonLiteral { ~NonLiteral(); } nl; // cxx14-note {{user-provided destructor}}
   // cxx20_23-note at -1 {{'NonLiteral' is not literal because its destructor is not constexpr}}


        


More information about the cfe-commits mailing list