Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Clang] Static member initializers are not immediate escalating context. #66021

Merged
merged 8 commits into from
Sep 19, 2023

Conversation

cor3ntin
Copy link
Contributor

Per CWG2760, default members initializers should be consider part the body of constructors, which mean they are evaluated in an immediate escalating context.

However, this does not apply to static members.

This patch produces some extraneous diagnostics, unfortunately we do not have a good way to report an error back to the initializer and this is a pre existing issue

Fixes #65985

@cor3ntin cor3ntin requested a review from a team as a code owner September 11, 2023 22:18
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Sep 11, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 11, 2023

@llvm/pr-subscribers-clang

Changes

Per CWG2760, default members initializers should be consider part the body of constructors, which mean they are evaluated in an immediate escalating context.

However, this does not apply to static members.

This patch produces some extraneous diagnostics, unfortunately we do not have a good way to report an error back to the initializer and this is a pre existing issue

Fixes #65985

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

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+4)
  • (modified) clang/lib/Parse/ParseDeclCXX.cpp (+10-2)
  • (modified) clang/test/SemaCXX/cxx2b-consteval-propagate.cpp (+23)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d3757c833ef83a2..8bc66ee44e3d289 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -161,6 +161,10 @@ Bug Fixes to C++ Support
   requires-expression. This fixes:
   (`#64138 _`).
 
+- Fix a crash when calling a non-constant immediate function
+  in the initializer of a static data member.
+  (`#65985 _`).
+
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp
index 42f268b0911b783..c2a2a561a3f0700 100644
--- a/clang/lib/Parse/ParseDeclCXX.cpp
+++ b/clang/lib/Parse/ParseDeclCXX.cpp
@@ -3229,13 +3229,21 @@ ExprResult Parser::ParseCXXMemberInitializer(Decl *D, bool IsFunction,
   assert(Tok.isOneOf(tok::equal, tok::l_brace) &&
          "Data member initializer not starting with '=' or '{'");
 
+  bool IsFieldInitialization = isa_and_present(D);
+
   EnterExpressionEvaluationContext Context(
       Actions,
-      isa_and_present(D)
+      IsFieldInitialization
           ? Sema::ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed
           : Sema::ExpressionEvaluationContext::PotentiallyEvaluated,
       D);
-  Actions.ExprEvalContexts.back().InImmediateEscalatingFunctionContext = true;
+
+  // CWG2760
+  // Default member initializers used to initialize a base or member subobject
+  // [...] are considered to be part of the function body
+  Actions.ExprEvalContexts.back().InImmediateEscalatingFunctionContext =
+      IsFieldInitialization;
+
   if (TryConsumeToken(tok::equal, EqualLoc)) {
     if (Tok.is(tok::kw_delete)) {
       // In principle, an initializer of '= delete p;' is legal, but it will
diff --git a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
index c5eb7f139327505..f967ce6554d777c 100644
--- a/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
+++ b/clang/test/SemaCXX/cxx2b-consteval-propagate.cpp
@@ -330,3 +330,26 @@ struct S {
 S s(0); // expected-note {{in the default initializer of 'j'}}
 
 }
+
+namespace GH65985 {
+
+int consteval operator""_foo(unsigned long long V) {
+    return 0;
+}
+int consteval operator""_bar(unsigned long long V); // expected-note 3{{here}}
+
+struct C {
+    static const int a = 1_foo;
+    static constexpr int b = 1_foo;
+    static const int c = 1_bar; // expected-error {{call to consteval function 'GH65985::operator""_bar' is not a constant expression}} \
+                                // expected-note {{undefined function 'operator""_bar' cannot be used in a constant expression}} \
+                                // expected-error {{in-class initializer for static data member is not a constant expression}}
+
+    // FIXME: remove duplicate diagnostics
+    static constexpr int d = 1_bar; // expected-error {{call to consteval function 'GH65985::operator""_bar' is not a constant expression}} \
+                                    // expected-note {{undefined function 'operator""_bar' cannot be used in a constant expression}} \
+                                    // expected-error {{constexpr variable 'd' must be initialized by a constant expression}}  \
+                                    // expected-note {{undefined function 'operator""_bar' cannot be used in a constant expression}}
+};
+
+}

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM I am wondering if we can have a test that fails to initialize a static member b/c we select an immediate function.

@shafik shafik requested a review from a team September 12, 2023 20:24
@cor3ntin
Copy link
Contributor Author

Instead of a udl you mean? Sure!

Per CWG2760, default members initializers should be consider
part the body of consstructors, which mean they are evaluated
in an immediate escalating context.

However, this does not apply to static members.

This patch produces some extraneous diagnostics, unfortunately
we do not have a good way to report an error back to the
initializer and this is a preexisting issue

Fixes llvm#65985
@cor3ntin cor3ntin force-pushed the consteval_static_member branch from e46c273 to 0ed8935 Compare September 16, 2023 10:28
@cor3ntin
Copy link
Contributor Author

@erichkeane @shafik You are happy with that ? (modulo rebase / spurious ws changes I need to revert)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assertion in clang::Sema::MarkExpressionAsImmediateEscalating when there are diagnostic errors
4 participants