Addressing feedback. Redesigning proposed solution.

Author: katzdm

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
 ^^^^^^^^^^^^^^^^^^^^^^^^^

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

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

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));

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

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;
 

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}} \

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

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

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