Addressing feedback. Redesigning proposed solution.

katzdm · katzdm · commit ec17c7668650 · 2024-04-26T13:27:21.000-04:00
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
@@ -593,8 +593,10 @@ Bug Fixes to C++ Support
 - Fixed a use-after-free bug in parsing of type constraints with default arguments that involve lambdas. (#GH67235)
 - Fixed bug in which the body of a consteval lambda within a template was not parsed as within an
   immediate function context.
-- Fix a bug in which valid constexpr variable initializers containing immediate function calls were
-  sometimes diagnosed as not constant expressions.
+- Fix a C++23 bug in implementation of P2564R3 which evaluates immediate invocations in place
+  within initializers for variables that are usable in constant expressions or are constant
+  initialized, rather than evaluating them as a part of the larger manifestly constant evaluated
+  expression.
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
@@ -10201,7 +10201,9 @@ class Sema final : public SemaBase {
         S.ExprEvalContexts.back().InImmediateFunctionContext =
             FD->isImmediateFunction() ||
             S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
-                .isConstantEvaluated();
+                .isConstantEvaluated() ||
+            S.ExprEvalContexts[S.ExprEvalContexts.size() - 2]
+                .isImmediateFunctionContext();
         S.ExprEvalContexts.back().InImmediateEscalatingFunctionContext =
             S.getLangOpts().CPlusPlus20 && FD->isImmediateEscalating();
       } else
diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
@@ -2552,43 +2552,37 @@ Decl *Parser::ParseDeclarationAfterDeclarator(
   return ParseDeclarationAfterDeclaratorAndAttributes(D, TemplateInfo);
 }
 
-static bool isConstexprVariable(const Decl *D) {
-  if (const VarDecl *Var = dyn_cast_if_present<VarDecl>(D))
-    return Var->isConstexpr();
-
-  return false;
-}
-
 Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
     Declarator &D, const ParsedTemplateInfo &TemplateInfo, ForRangeInit *FRI) {
   // RAII type used to track whether we're inside an initializer.
   struct InitializerScopeRAII {
     Parser &P;
     Declarator &D;
     Decl *ThisDecl;
-    EnterExpressionEvaluationContext EC;
+    bool Entered;
 
     InitializerScopeRAII(Parser &P, Declarator &D, Decl *ThisDecl)
-        : P(P), D(D), ThisDecl(ThisDecl),
-          EC(P.Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated,
-             ThisDecl, Sema::ExpressionEvaluationContextRecord::EK_Other,
-             isConstexprVariable(ThisDecl)) {
+        : P(P), D(D), ThisDecl(ThisDecl), Entered(false) {
       if (ThisDecl && P.getLangOpts().CPlusPlus) {
         Scope *S = nullptr;
         if (D.getCXXScopeSpec().isSet()) {
           P.EnterScope(0);
           S = P.getCurScope();
         }
-        P.Actions.ActOnCXXEnterDeclInitializer(S, ThisDecl);
+        if (ThisDecl && !ThisDecl->isInvalidDecl()) {
+          P.Actions.ActOnCXXEnterDeclInitializer(S, ThisDecl);
+          Entered = true;
+        }
       }
     }
-    ~InitializerScopeRAII() { pop(); }
-    void pop() {
+    ~InitializerScopeRAII() {
       if (ThisDecl && P.getLangOpts().CPlusPlus) {
         Scope *S = nullptr;
         if (D.getCXXScopeSpec().isSet())
           S = P.getCurScope();
-        P.Actions.ActOnCXXExitDeclInitializer(S, ThisDecl);
+
+        if (Entered)
+          P.Actions.ActOnCXXExitDeclInitializer(S, ThisDecl);
         if (S)
           P.ExitScope();
       }
@@ -2719,8 +2713,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
         FRI->RangeExpr = Init;
       }
 
-      InitScope.pop();
-
       if (Init.isInvalid()) {
         SmallVector<tok::TokenKind, 2> StopTokens;
         StopTokens.push_back(tok::comma);
@@ -2768,8 +2760,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
 
     bool SawError = ParseExpressionList(Exprs, ExpressionStarts);
 
-    InitScope.pop();
-
     if (SawError) {
       if (ThisVarDecl && PP.isCodeCompletionReached() && !CalledSignatureHelp) {
         Actions.ProduceConstructorSignatureHelp(
@@ -2801,8 +2791,6 @@ Decl *Parser::ParseDeclarationAfterDeclaratorAndAttributes(
     PreferredType.enterVariableInit(Tok.getLocation(), ThisDecl);
     ExprResult Init(ParseBraceInitializer());
 
-    InitScope.pop();
-
     if (Init.isInvalid()) {
       Actions.ActOnInitializerError(ThisDecl);
     } else
diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
@@ -16548,8 +16548,8 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
         std::string PrettySourceValue = toString(Value, 10);
         std::string PrettyTargetValue = PrettyPrintInRange(Value, TargetRange);
 
-        S.DiagRuntimeBehavior(
-            E->getExprLoc(), E,
+        S.Diag(
+            E->getExprLoc(),
             S.PDiag(diag::warn_impcast_integer_precision_constant)
                 << PrettySourceValue << PrettyTargetValue << E->getType() << T
                 << E->getSourceRange() << SourceRange(CC));
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18545,15 +18545,6 @@ void Sema::ActOnPureSpecifier(Decl *D, SourceLocation ZeroLoc) {
     Diag(D->getLocation(), diag::err_illegal_initializer);
 }
 
-/// Determine whether the given declaration is a global variable or
-/// static data member.
-static bool isNonlocalVariable(const Decl *D) {
-  if (const VarDecl *Var = dyn_cast_or_null<VarDecl>(D))
-    return Var->hasGlobalStorage();
-
-  return false;
-}
-
 /// Invoked when we are about to parse an initializer for the declaration
 /// 'Dcl'.
 ///
@@ -18562,9 +18553,7 @@ static bool isNonlocalVariable(const Decl *D) {
 /// class X. If the declaration had a scope specifier, a scope will have
 /// been created and passed in for this purpose. Otherwise, S will be null.
 void Sema::ActOnCXXEnterDeclInitializer(Scope *S, Decl *D) {
-  // If there is no declaration, there was an error parsing it.
-  if (!D || D->isInvalidDecl())
-    return;
+  assert(D && !D->isInvalidDecl());
 
   // We will always have a nested name specifier here, but this declaration
   // might not be out of line if the specifier names the current namespace:
@@ -18573,25 +18562,35 @@ void Sema::ActOnCXXEnterDeclInitializer(Scope *S, Decl *D) {
   if (S && D->isOutOfLine())
     EnterDeclaratorContext(S, D->getDeclContext());
 
-  // If we are parsing the initializer for a static data member, push a
-  // new expression evaluation context that is associated with this static
-  // data member.
-  if (isNonlocalVariable(D))
-    PushExpressionEvaluationContext(
-        ExpressionEvaluationContext::PotentiallyEvaluated, D);
+  PushExpressionEvaluationContext(
+      ExpressionEvaluationContext::PotentiallyEvaluated, D);
 }
 
 /// Invoked after we are finished parsing an initializer for the declaration D.
 void Sema::ActOnCXXExitDeclInitializer(Scope *S, Decl *D) {
-  // If there is no declaration, there was an error parsing it.
-  if (!D || D->isInvalidDecl())
-    return;
-
-  if (isNonlocalVariable(D))
-    PopExpressionEvaluationContext();
+  assert(D);
 
   if (S && D->isOutOfLine())
     ExitDeclaratorContext(S);
+
+  if (getLangOpts().CPlusPlus23) {
+    // An expression or conversion is 'manifestly constant-evaluated' if it is:
+    // [...]
+    // - the initializer of a variable that is usable in constant expressions or
+    //   has constant initialization.
+    if (auto *VD = dyn_cast<VarDecl>(D);
+        VD && (VD->isUsableInConstantExpressions(Context) ||
+               VD->hasConstantInitialization())) {
+      // An expression or conversion is in an 'immediate function context' if it
+      // is potentially evaluated and either:
+      // [...]
+      // - it is a subexpression of a manifestly constant-evaluated expression or
+      //   conversion.
+      ExprEvalContexts.back().InImmediateFunctionContext = true;
+    }
+  }
+
+  PopExpressionEvaluationContext();
 }
 
 /// ActOnCXXConditionDeclarationExpr - Parsed a condition declaration of a
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
@@ -18011,6 +18011,7 @@ HandleImmediateInvocations(Sema &SemaRef,
                            Sema::ExpressionEvaluationContextRecord &Rec) {
   if ((Rec.ImmediateInvocationCandidates.size() == 0 &&
        Rec.ReferenceToConsteval.size() == 0) ||
+      Rec.isImmediateFunctionContext() ||
       SemaRef.RebuildingImmediateInvocation)
     return;
 
diff --git a/clang/test/AST/Interp/intap.cpp b/clang/test/AST/Interp/intap.cpp
@@ -9,7 +9,8 @@ using MaxBitInt = _BitInt(128);
 
 constexpr _BitInt(2) A = 0;
 constexpr _BitInt(2) B = A + 1;
-constexpr _BitInt(2) C = B + 1;
+constexpr _BitInt(2) C = B + 1; // expected-warning {{from 2 to -2}} \
+                                // ref-warning {{from 2 to -2}}
 static_assert(C == -2, "");
 static_assert(C - B == A, ""); // expected-error {{not an integral constant expression}} \
                                // expected-note {{value -3 is outside the range of representable values}} \
diff --git a/clang/test/CXX/expr/expr.const/p2-0x.cpp b/clang/test/CXX/expr/expr.const/p2-0x.cpp
@@ -244,8 +244,8 @@ namespace UndefinedBehavior {
     constexpr int n13 = n5 + n5; // expected-error {{constant expression}} expected-note {{value -4294967296 is outside the range of }}
     constexpr int n14 = n3 - n5; // expected-error {{constant expression}} expected-note {{value 4294967295 is outside the range of }}
     constexpr int n15 = n5 * n5; // expected-error {{constant expression}} expected-note {{value 4611686018427387904 is outside the range of }}
-    constexpr signed char c1 = 100 * 2; // ok - no error from changing value because initializer is constexpr.
-    constexpr signed char c2 = '\x64' * '\2'; // also ok - no error from changing value because initializer is constexpr.
+    constexpr signed char c1 = 100 * 2; // ok expected-warning{{changes value}}
+    constexpr signed char c2 = '\x64' * '\2'; // also ok  expected-warning{{changes value}}
     constexpr long long ll1 = 0x7fffffffffffffff; // ok
     constexpr long long ll2 = ll1 + 1; // expected-error {{constant}} expected-note {{ 9223372036854775808 }}
     constexpr long long ll3 = -ll1 - 1; // ok
diff --git a/clang/test/CXX/expr/expr.const/p6-2a.cpp b/clang/test/CXX/expr/expr.const/p6-2a.cpp
@@ -43,11 +43,12 @@ struct Temporary {
 constexpr Temporary t = {3}; // expected-error {{must have constant destruction}} expected-note {{created here}} expected-note {{in call}}
 
 namespace P1073R3 {
-consteval int f() { return 42; } // expected-note {{declared here}}
+consteval int f() { return 42; } // expected-note 2 {{declared here}}
 consteval auto g() { return f; }
 consteval int h(int (*p)() = g()) { return p(); }
 constexpr int r = h();
-constexpr auto e = g();  // expected-error {{constexpr variable 'e' must be initialized by a constant expression}} \
-                            expected-note {{pointer to a consteval declaration is not a constant expression}}
+constexpr auto e = g();  // expected-error {{call to consteval function 'P1073R3::g' is not a constant expression}} \
+                            expected-error {{constexpr variable 'e' must be initialized by a constant expression}} \
+                            expected-note 2 {{pointer to a consteval declaration is not a constant expression}}
 static_assert(r == 42);
 } // namespace P1073R3
diff --git a/clang/test/Parser/pragma-fenv_access.c b/clang/test/Parser/pragma-fenv_access.c
@@ -32,6 +32,9 @@ int main(void) {
   CONST int not_too_big = 255;
   CONST float fnot_too_big = not_too_big;
   CONST int too_big = 0x7ffffff0;
+#if defined(CPP)
+//expected-warning@+2{{implicit conversion}}
+#endif
   CONST float fbig = too_big; // inexact
 #if !defined(CPP)
 #define static_assert _Static_assert
diff --git a/clang/test/SemaCXX/builtin_vectorelements.cpp b/clang/test/SemaCXX/builtin_vectorelements.cpp
@@ -42,11 +42,11 @@ void test_builtin_vectorelements() {
 #include <arm_sve.h>
 
 consteval int consteval_elements() { // expected-error {{consteval function never produces a constant expression}}
-  return __builtin_vectorelements(svuint64_t); // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}
+  return __builtin_vectorelements(svuint64_t); // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}  // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}
 }
 
 void test_bad_constexpr() {
-  constexpr int eval = consteval_elements(); // expected-error {{initialized by a constant expression}} // expected-note {{in call}}
+  constexpr int eval = consteval_elements(); // expected-error {{initialized by a constant expression}} // expected-error {{not a constant expression}} // expected-note {{in call}} // expected-note {{in call}}
   constexpr int i32 = __builtin_vectorelements(svuint32_t); // expected-error {{initialized by a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}
   constexpr int i16p8 = __builtin_vectorelements(svuint16_t) + 16; // expected-error {{initialized by a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}}
   constexpr int lambda = [] { return __builtin_vectorelements(svuint16_t); }(); // expected-error {{initialized by a constant expression}} // expected-note {{cannot determine number of elements for sizeless vectors in a constant expression}} // expected-note {{in call}}
diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -1961,7 +1961,7 @@ namespace ConstexprConstructorRecovery {
 
 namespace Lifetime {
   void f() {
-    constexpr int &n = n; // expected-error {{constant expression}} expected-note {{use of reference outside its lifetime}}
+    constexpr int &n = n; // expected-error {{constant expression}} expected-note {{use of reference outside its lifetime}} expected-warning {{not yet bound to a value}}
     constexpr int m = m; // expected-error {{constant expression}} expected-note {{read of object outside its lifetime}}
   }
 
diff --git a/clang/test/SemaCXX/cxx2a-consteval.cpp b/clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -733,7 +733,7 @@ constexpr derp d;
 struct test {
   consteval int operator[](int i) const { return {}; }
   consteval const derp * operator->() const { return &d; }
-  consteval int f() const { return 12; } // expected-note {{declared here}}
+  consteval int f() const { return 12; } // expected-note 2{{declared here}}
 };
 
 constexpr test a;
@@ -746,7 +746,8 @@ constexpr int s = a.operator[](1);
 constexpr int t = a[1];
 constexpr int u = a.operator->()->b;
 constexpr int v = a->b;
-constexpr int w = (a.*&test::f)();
+// FIXME: I believe this case should work, but we currently reject.
+constexpr int w = (a.*&test::f)(); // expected-error {{cannot take address of consteval function 'f' outside of an immediate invocation}}
 constexpr int x = a.f();
 
 // Show that we reject when not in an immediate context.
@@ -1052,22 +1053,30 @@ int f() {
 namespace GH57682 {
 void test() {
   constexpr auto l1 = []() consteval { // expected-error {{cannot take address of consteval call operator of '(lambda at}} \
-                                       // expected-note 2 {{declared here}}
+                                       // expected-note  2{{declared here}}
         return 3;
   };
-
-  int (*f1)(void) = l1;
-
-  constexpr int (*f2)(void) = l1; // expected-error {{constexpr variable 'f2' must be initialized by a constant expression}} \
+  constexpr int (*f1)(void) = l1; // expected-error {{constexpr variable 'f1' must be initialized by a constant expression}} \
                                   // expected-note  {{pointer to a consteval declaration is not a constant expression}}
 
-  constexpr auto lstatic = []() static consteval { // expected-note  {{declared here}} \
-                                                   // expected-warning {{extension}}
+
+  constexpr auto lstatic = []() static consteval { // expected-error {{cannot take address of consteval call operator of '(lambda at}} \
+                                       // expected-note  2{{declared here}} \
+                                       // expected-warning {{extension}}
         return 3;
   };
-  constexpr int (*f3)(void) = lstatic; // expected-error {{constexpr variable 'f3' must be initialized by a constant expression}} \
+  constexpr int (*f2)(void) = lstatic; // expected-error {{constexpr variable 'f2' must be initialized by a constant expression}} \
                                        // expected-note  {{pointer to a consteval declaration is not a constant expression}}
-  };
+
+  int (*f3)(void) = []() consteval { return 3; };  // expected-error {{cannot take address of consteval call operator of '(lambda at}} \
+                                                   // expected-note {{declared here}}
+}
+
+consteval void consteval_test() {
+  constexpr auto l1 = []() consteval { return 3; };
+
+  int (*f1)(void) = l1;  // ok
+}
 }
 
 namespace GH60286 {
@@ -1092,17 +1101,18 @@ struct tester {
 consteval const char* make_name(const char* name) { return name;}
 consteval const char* pad(int P) { return "thestring"; }
 
-int bad = 10; // expected-note 5{{declared here}}
+int bad = 10; // expected-note 6{{declared here}}
 
 tester glob1(make_name("glob1"));
 tester glob2(make_name("glob2"));
 constexpr tester cglob(make_name("cglob"));
-tester paddedglob(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
+tester paddedglob(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
                                         // expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
 
 constexpr tester glob3 = { make_name("glob3") };
-constexpr tester glob4 = { make_name(pad(bad)) }; // expected-error {{constexpr variable 'glob4' must be initialized by a constant expression}} \
-                                                  // expected-note 1{{read of non-const variable 'bad' is not allowed in a constant expression}}
+constexpr tester glob4 = { make_name(pad(bad)) }; // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
+                                                  // expected-error {{constexpr variable 'glob4' must be initialized by a constant expression}} \
+                                                  // expected-note 2{{read of non-const variable 'bad' is not allowed in a constant expression}}
 
 auto V = make_name(pad(3));
 auto V1 = make_name(pad(bad)); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
@@ -1112,12 +1122,12 @@ auto V1 = make_name(pad(bad)); // expected-error {{call to consteval function 'G
 void foo() {
   static tester loc1(make_name("loc1"));
   static constexpr tester loc2(make_name("loc2"));
-  static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
+  static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
                                                 // expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
 }
 
 void bar() {
-  static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::make_name' is not a constant expression}} \
+  static tester paddedloc(make_name(pad(bad))); // expected-error {{call to consteval function 'GH58207::tester::tester' is not a constant expression}} \
                                                 // expected-note {{read of non-const variable 'bad' is not allowed in a constant expression}}
 }
 }
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
diff --git a/clang/test/SemaCXX/enum-scoped.cpp b/clang/test/SemaCXX/enum-scoped.cpp
diff --git a/clang/test/SemaCXX/ext-int.cpp b/clang/test/SemaCXX/ext-int.cpp
diff --git a/clang/unittests/Support/TimeProfilerTest.cpp b/clang/unittests/Support/TimeProfilerTest.cpp
