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][C++23] Implement P2448R2: Relaxing some constexpr restrictions #77753

Merged
merged 20 commits into from
Mar 7, 2024

Conversation

Fznamznon
Copy link
Contributor

Per https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2448r2.html function/constructor/destructor can be marked constexpr even though it never produces a constant expression.
Non-literal types as return types and parameter types of functions marked constexpr are also allowed.
Since this is not a DR, the diagnostic messages are still preserved for C++ standards older than C++23.

Per https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2448r2.html
function/constructor/destructor can be marked `constexpr` even though it never
produces a constant expression.
Non-literal types as return types and parameter types of functions
marked `constexpr` are also allowed.
Since this is not a DR, the diagnostic messages are still preserved for
C++ standards older than C++23.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 11, 2024

@llvm/pr-subscribers-clang

Author: Mariya Podchishchaeva (Fznamznon)

Changes

Per https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2448r2.html function/constructor/destructor can be marked constexpr even though it never produces a constant expression.
Non-literal types as return types and parameter types of functions marked constexpr are also allowed.
Since this is not a DR, the diagnostic messages are still preserved for C++ standards older than C++23.


Patch is 88.09 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/77753.diff

36 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+2)
  • (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+26-16)
  • (modified) clang/lib/Sema/SemaDeclCXX.cpp (+39-15)
  • (modified) clang/test/AST/Interp/cxx23.cpp (+6-18)
  • (modified) clang/test/AST/Interp/literals.cpp (+1-1)
  • (modified) clang/test/AST/Interp/shifts.cpp (+4-4)
  • (modified) clang/test/CXX/basic/basic.types/p10.cpp (+13-13)
  • (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/dtor.cpp (+4-4)
  • (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3-2b.cpp (+2-2)
  • (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp (+13-14)
  • (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp (+10-10)
  • (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p5.cpp (+3-3)
  • (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p6.cpp (+1-1)
  • (modified) clang/test/CXX/dcl.decl/dcl.fct.def/dcl.fct.def.default/p2.cpp (+3-3)
  • (modified) clang/test/CXX/drs/dr13xx.cpp (+11-11)
  • (modified) clang/test/CXX/drs/dr14xx.cpp (+3-3)
  • (modified) clang/test/CXX/drs/dr16xx.cpp (+6-6)
  • (modified) clang/test/CXX/drs/dr6xx.cpp (+12-12)
  • (modified) clang/test/CXX/expr/expr.const/p2-0x.cpp (+1-1)
  • (modified) clang/test/CXX/expr/expr.const/p5-26.cpp (+2-2)
  • (modified) clang/test/CXX/special/class.copy/p13-0x.cpp (+1-1)
  • (modified) clang/test/PCH/cxx11-constexpr.cpp (+1-1)
  • (modified) clang/test/SemaCXX/builtin_vectorelements.cpp (+1-1)
  • (modified) clang/test/SemaCXX/constant-expression-cxx11.cpp (+15-14)
  • (modified) clang/test/SemaCXX/constant-expression-cxx14.cpp (+17-16)
  • (modified) clang/test/SemaCXX/constant-expression-cxx2b.cpp (+12-6)
  • (modified) clang/test/SemaCXX/constexpr-function-recovery-crash.cpp (+2-2)
  • (modified) clang/test/SemaCXX/cxx1z-constexpr-lambdas.cpp (+2-2)
  • (added) clang/test/SemaCXX/cxx23-invalid-constexpr.cpp (+152)
  • (modified) clang/test/SemaCXX/cxx2a-consteval.cpp (+2-2)
  • (modified) clang/test/SemaCXX/deduced-return-type-cxx14.cpp (+4-4)
  • (modified) clang/test/SemaCXX/ms-constexpr-invalid.cpp (+3-3)
  • (modified) clang/test/SemaCXX/ms-constexpr.cpp (+1-1)
  • (modified) clang/test/SemaCXX/sizeless-1.cpp (+1-1)
  • (modified) clang/test/SemaOpenCLCXX/addrspace-constructors.clcpp (+1-1)
  • (modified) clang/www/cxx_status.html (+1-8)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a18d36a16b1a9c..23342a6a7256d8 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -165,6 +165,8 @@ C++23 Feature Support
 - Added a separate warning to warn the use of attributes on lambdas as a C++23 extension
   in previous language versions: ``-Wc++23-lambda-attributes``.
 
+- Implemented `P2448R2: Relaxing some constexpr restrictions <https://wg21.link/P2448R2>`_.
+
 C++2c Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 1a79892e40030a..67409374f26dfa 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2765,10 +2765,14 @@ def err_constexpr_tag : Error<
   "cannot be marked %sub{select_constexpr_spec_kind}1">;
 def err_constexpr_dtor : Error<
   "destructor cannot be declared %sub{select_constexpr_spec_kind}0">;
-def err_constexpr_dtor_subobject : Error<
-  "destructor cannot be declared %sub{select_constexpr_spec_kind}0 because "
+def ext_constexpr_dtor_subobject : ExtWarn<
+  "destructor cannot be declared %sub{select_constexpr_spec_kind}0 before C++23 because "
   "%select{data member %2|base class %3}1 does not have a "
-  "constexpr destructor">;
+  "constexpr destructor">, InGroup<CXX23>, DefaultError;
+def warn_cxx23_compat_constexpr_dtor_subobject : ExtWarn<
+  "%sub{select_constexpr_spec_kind}0 destructor is incompatible with C++ standards before C++23 because "
+  "%select{data member %2|base class %3}1 does not have a "
+  "constexpr destructor">, InGroup<CXXPre23Compat>, DefaultIgnore;
 def note_constexpr_dtor_subobject : Note<
   "%select{data member %1|base class %2}0 declared here">;
 def err_constexpr_wrong_decl_kind : Error<
@@ -2800,11 +2804,14 @@ def note_non_literal_incomplete : Note<
 def note_non_literal_virtual_base : Note<"%select{struct|interface|class}0 "
   "with virtual base %plural{1:class|:classes}1 is not a literal type">;
 def note_constexpr_virtual_base_here : Note<"virtual base class declared here">;
-def err_constexpr_non_literal_return : Error<
-  "%select{constexpr|consteval}0 function's return type %1 is not a literal type">;
-def err_constexpr_non_literal_param : Error<
-  "%select{constexpr|consteval}2 %select{function|constructor}1's %ordinal0 parameter type %3 is "
-  "not a literal type">;
+def ext_constexpr_non_literal_return : ExtWarn<
+  "%select{constexpr|consteval}0 function with non-literal return type %1 is a C++23 extension">, InGroup<CXX23>, DefaultError;
+def warn_cxx23_compat_constexpr_non_literal_return : Warning<
+  "%select{constexpr|consteval}0 function with non-literal return type %1 is incompatible with C++ standards before C++23">, InGroup<CXXPre23Compat>, DefaultIgnore;
+def ext_constexpr_non_literal_param : ExtWarn<
+  "%select{constexpr|consteval}2 %select{function|constructor}1 with %ordinal0 non-literal parameter type %3 is a C++23 extension">, InGroup<CXX23>, DefaultError;
+def warn_cxx23_compat_constexpr_non_literal_param : Warning<
+  "%select{constexpr|consteval}2 %select{function|constructor}1 with %ordinal0 non-literal parameter type %3 is not compatible with C++ standards before C++23">, InGroup<CXXPre23Compat>, DefaultIgnore;
 def err_constexpr_body_invalid_stmt : Error<
   "statement not allowed in %select{constexpr|consteval}1 %select{function|constructor}0">;
 def ext_constexpr_body_invalid_stmt : ExtWarn<
@@ -2865,8 +2872,11 @@ def warn_cxx17_compat_constexpr_local_var_no_init : Warning<
   "is incompatible with C++ standards before C++20">,
   InGroup<CXXPre20Compat>, DefaultIgnore;
 def ext_constexpr_function_never_constant_expr : ExtWarn<
-  "%select{constexpr|consteval}1 %select{function|constructor}0 never produces a "
-  "constant expression">, InGroup<DiagGroup<"invalid-constexpr">>, DefaultError;
+  "%select{constexpr|consteval}1 %select{function|constructor}0 that never produces a "
+  "constant expression is a C++23 extension">, InGroup<DiagGroup<"invalid-constexpr">>, DefaultError;
+def warn_cxx23_compat_constexpr_function_never_constant_expr : Warning<
+  "%select{constexpr|consteval}1 %select{function|constructor}0 that never produces a "
+  "constant expression is incompatible with C++ standards before C++23">, InGroup<CXXPre23Compat>, DefaultIgnore;
 def err_attr_cond_never_constant_expr : Error<
   "%0 attribute expression never produces a constant expression">;
 def err_diagnose_if_invalid_diagnostic_type : Error<
@@ -9539,14 +9549,14 @@ def err_defaulted_special_member_copy_const_param : Error<
 def err_defaulted_copy_assign_not_ref : Error<
   "the parameter for an explicitly-defaulted copy assignment operator must be an "
   "lvalue reference type">;
-def err_incorrect_defaulted_constexpr : Error<
-  "defaulted definition of %sub{select_special_member_kind}0 "
-  "is not constexpr">;
+def ext_incorrect_defaulted_constexpr : ExtWarn<
+  "defaulted definition of %sub{select_special_member_kind}0 that marked %select{constexpr|consteval}1 "
+  "but never produces a constant expression is a C++23 extension">, InGroup<CXX23>, DefaultError;
+def warn_cxx23_compat_incorrect_defaulted_constexpr : Warning<
+  "defaulted definition of %sub{select_special_member_kind}0 that marked %select{constexpr|consteval}1 "
+  "but never produces a constant expression is incompatible with C++ standards before C++23">, InGroup<CXXPre23Compat>, DefaultIgnore;
 def err_incorrect_defaulted_constexpr_with_vb: Error<
   "%sub{select_special_member_kind}0 cannot be 'constexpr' in a class with virtual base class">;
-def err_incorrect_defaulted_consteval : Error<
-  "defaulted declaration of %sub{select_special_member_kind}0 "
-  "cannot be consteval because implicit definition is not constexpr">;
 def warn_defaulted_method_deleted : Warning<
   "explicitly defaulted %sub{select_special_member_kind}0 is implicitly "
   "deleted">, InGroup<DefaultedFunctionDeleted>;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 36e53c684ac4dc..aa49dc73e157bf 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1722,12 +1722,19 @@ static bool CheckConstexprDestructorSubobjects(Sema &SemaRef,
       return true;
 
     if (Kind == Sema::CheckConstexprKind::Diagnose) {
-      SemaRef.Diag(DD->getLocation(), diag::err_constexpr_dtor_subobject)
+      SemaRef.Diag(DD->getLocation(),
+                   SemaRef.getLangOpts().CPlusPlus23
+                       ? diag::warn_cxx23_compat_constexpr_dtor_subobject
+                       : diag::ext_constexpr_dtor_subobject)
           << static_cast<int>(DD->getConstexprKind()) << !FD
           << (FD ? FD->getDeclName() : DeclarationName()) << T;
       SemaRef.Diag(Loc, diag::note_constexpr_dtor_subobject)
           << !FD << (FD ? FD->getDeclName() : DeclarationName()) << T;
     }
+
+    if (SemaRef.getLangOpts().CPlusPlus23)
+      return true;
+
     return false;
   };
 
@@ -1754,11 +1761,17 @@ static bool CheckConstexprParameterTypes(Sema &SemaRef,
     const ParmVarDecl *PD = FD->getParamDecl(ArgIndex);
     assert(PD && "null in a parameter list");
     SourceLocation ParamLoc = PD->getLocation();
-    if (CheckLiteralType(SemaRef, Kind, ParamLoc, *i,
-                         diag::err_constexpr_non_literal_param, ArgIndex + 1,
-                         PD->getSourceRange(), isa<CXXConstructorDecl>(FD),
-                         FD->isConsteval()))
+    if (CheckLiteralType(
+            SemaRef, Kind, ParamLoc, *i,
+            SemaRef.getLangOpts().CPlusPlus23
+                ? diag::warn_cxx23_compat_constexpr_non_literal_param
+                : diag::ext_constexpr_non_literal_param,
+            ArgIndex + 1, PD->getSourceRange(), isa<CXXConstructorDecl>(FD),
+            FD->isConsteval())) {
+      if (SemaRef.getLangOpts().CPlusPlus23)
+        return true;
       return false;
+    }
   }
   return true;
 }
@@ -1767,10 +1780,16 @@ static bool CheckConstexprParameterTypes(Sema &SemaRef,
 /// true. If not, produce a suitable diagnostic and return false.
 static bool CheckConstexprReturnType(Sema &SemaRef, const FunctionDecl *FD,
                                      Sema::CheckConstexprKind Kind) {
-  if (CheckLiteralType(SemaRef, Kind, FD->getLocation(), FD->getReturnType(),
-                       diag::err_constexpr_non_literal_return,
-                       FD->isConsteval()))
+  if (CheckLiteralType(
+          SemaRef, Kind, FD->getLocation(), FD->getReturnType(),
+          SemaRef.getLangOpts().CPlusPlus23
+              ? diag::warn_cxx23_compat_constexpr_non_literal_return
+              : diag::ext_constexpr_non_literal_return,
+          FD->isConsteval())) {
+    if (SemaRef.getLangOpts().CPlusPlus23)
+      return true;
     return false;
+  }
   return true;
 }
 
@@ -2458,8 +2477,11 @@ static bool CheckConstexprFunctionBody(Sema &SemaRef, const FunctionDecl *Dcl,
   SmallVector<PartialDiagnosticAt, 8> Diags;
   if (Kind == Sema::CheckConstexprKind::Diagnose &&
       !Expr::isPotentialConstantExpr(Dcl, Diags)) {
-    SemaRef.Diag(Dcl->getLocation(),
-                 diag::ext_constexpr_function_never_constant_expr)
+    SemaRef.Diag(
+        Dcl->getLocation(),
+        SemaRef.getLangOpts().CPlusPlus23
+            ? diag::warn_cxx23_compat_constexpr_function_never_constant_expr
+            : diag::ext_constexpr_function_never_constant_expr)
         << isa<CXXConstructorDecl>(Dcl) << Dcl->isConsteval()
         << Dcl->getNameInfo().getSourceRange();
     for (size_t I = 0, N = Diags.size(); I != N; ++I)
@@ -7852,13 +7874,15 @@ bool Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
           for (const auto &I : RD->vbases())
             Diag(I.getBeginLoc(), diag::note_constexpr_virtual_base_here);
         } else {
-          Diag(MD->getBeginLoc(), MD->isConsteval()
-                                      ? diag::err_incorrect_defaulted_consteval
-                                      : diag::err_incorrect_defaulted_constexpr)
-              << CSM;
+          Diag(MD->getBeginLoc(),
+               getLangOpts().CPlusPlus23
+                   ? diag::warn_cxx23_compat_incorrect_defaulted_constexpr
+                   : diag::ext_incorrect_defaulted_constexpr)
+              << CSM << MD->isConsteval();
         }
     // FIXME: Explain why the special member can't be constexpr.
-    HadError = true;
+    if (!getLangOpts().CPlusPlus23)
+      HadError = true;
   }
 
   if (First) {
diff --git a/clang/test/AST/Interp/cxx23.cpp b/clang/test/AST/Interp/cxx23.cpp
index bd1cf186d519c5..6ef7df9662e6a4 100644
--- a/clang/test/AST/Interp/cxx23.cpp
+++ b/clang/test/AST/Interp/cxx23.cpp
@@ -6,57 +6,45 @@
 
 /// FIXME: The new interpreter is missing all the 'control flows through...' diagnostics.
 
-constexpr int f(int n) {  // ref20-error {{constexpr function never produces a constant expression}} \
-                          // ref23-error {{constexpr function never produces a constant expression}}
+constexpr int f(int n) {  // ref20-error {{constexpr function that never produces a constant expression}}
   static const int m = n; // ref20-note {{control flows through the definition of a static variable}} \
                           // ref20-warning {{is a C++23 extension}} \
-                          // ref23-note {{control flows through the definition of a static variable}} \
                           // expected20-warning {{is a C++23 extension}}
 
   return m;
 }
-constexpr int g(int n) {        // ref20-error {{constexpr function never produces a constant expression}} \
-                                // ref23-error {{constexpr function never produces a constant expression}}
+constexpr int g(int n) {        // ref20-error {{constexpr function that never produces a constant expression}}
   thread_local const int m = n; // ref20-note {{control flows through the definition of a thread_local variable}} \
                                 // ref20-warning {{is a C++23 extension}} \
-                                // ref23-note {{control flows through the definition of a thread_local variable}} \
                                 // expected20-warning {{is a C++23 extension}}
   return m;
 }
 
-constexpr int c_thread_local(int n) { // ref20-error {{constexpr function never produces a constant expression}} \
-                                      // ref23-error {{constexpr function never produces a constant expression}}
+constexpr int c_thread_local(int n) { // ref20-error {{constexpr function that never produces a constant expression}}
   static _Thread_local int m = 0;     // ref20-note {{control flows through the definition of a thread_local variable}} \
                                       // ref20-warning {{is a C++23 extension}} \
-                                      // ref23-note {{control flows through the definition of a thread_local variable}} \
                                       // expected20-warning {{is a C++23 extension}}
   return m;
 }
 
 
-constexpr int gnu_thread_local(int n) { // ref20-error {{constexpr function never produces a constant expression}} \
-                                        // ref23-error {{constexpr function never produces a constant expression}}
+constexpr int gnu_thread_local(int n) { // ref20-error {{constexpr function that never produces a constant expression}}
   static __thread int m = 0;            // ref20-note {{control flows through the definition of a thread_local variable}} \
                                         // ref20-warning {{is a C++23 extension}} \
-                                        // ref23-note {{control flows through the definition of a thread_local variable}} \
                                         // expected20-warning {{is a C++23 extension}}
   return m;
 }
 
-constexpr int h(int n) {  // ref20-error {{constexpr function never produces a constant expression}} \
-                          // ref23-error {{constexpr function never produces a constant expression}}
+constexpr int h(int n) {  // ref20-error {{constexpr function that never produces a constant expression}}
   static const int m = n; // ref20-note {{control flows through the definition of a static variable}} \
                           // ref20-warning {{is a C++23 extension}} \
-                          // ref23-note {{control flows through the definition of a static variable}} \
                           // expected20-warning {{is a C++23 extension}}
   return &m - &m;
 }
 
-constexpr int i(int n) {        // ref20-error {{constexpr function never produces a constant expression}} \
-                                // ref23-error {{constexpr function never produces a constant expression}}
+constexpr int i(int n) {        // ref20-error {{constexpr function that never produces a constant expression}}
   thread_local const int m = n; // ref20-note {{control flows through the definition of a thread_local variable}} \
                                 // ref20-warning {{is a C++23 extension}} \
-                                // ref23-note {{control flows through the definition of a thread_local variable}} \
                                 // expected20-warning {{is a C++23 extension}}
   return &m - &m;
 }
diff --git a/clang/test/AST/Interp/literals.cpp b/clang/test/AST/Interp/literals.cpp
index 61825bc11438f6..ab09280e883fba 100644
--- a/clang/test/AST/Interp/literals.cpp
+++ b/clang/test/AST/Interp/literals.cpp
@@ -275,7 +275,7 @@ namespace SizeOf {
 
 #if __cplusplus >= 202002L
   /// FIXME: The following code should be accepted.
-  consteval int foo(int n) { // ref-error {{consteval function never produces a constant expression}}
+  consteval int foo(int n) { // ref-error {{consteval function that never produces a constant expression}}
     return sizeof(int[n]); // ref-note 3{{not valid in a constant expression}}
   }
   constinit int var = foo(5); // ref-error {{not a constant expression}} \
diff --git a/clang/test/AST/Interp/shifts.cpp b/clang/test/AST/Interp/shifts.cpp
index cf71e7145c2742..f8fa1b5d095da0 100644
--- a/clang/test/AST/Interp/shifts.cpp
+++ b/clang/test/AST/Interp/shifts.cpp
@@ -7,10 +7,10 @@
 
 
 namespace shifts {
-  constexpr void test() { // ref-error {{constexpr function never produces a constant expression}} \
-                          // ref-cxx17-error {{constexpr function never produces a constant expression}} \
-                          // expected-error {{constexpr function never produces a constant expression}} \
-                          // cxx17-error {{constexpr function never produces a constant expression}} \
+  constexpr void test() { // ref-error {{constexpr function that never produces a constant expression}} \
+                          // ref-cxx17-error {{constexpr function that never produces a constant expression}} \
+                          // expected-error {{constexpr function that never produces a constant expression}} \
+                          // cxx17-error {{constexpr function that never produces a constant expression}} \
 
     char c; // cxx17-warning {{uninitialized variable}} \
             // ref-cxx17-warning {{uninitialized variable}}
diff --git a/clang/test/CXX/basic/basic.types/p10.cpp b/clang/test/CXX/basic/basic.types/p10.cpp
index a543f248e53711..19e099d5077de7 100644
--- a/clang/test/CXX/basic/basic.types/p10.cpp
+++ b/clang/test/CXX/basic/basic.types/p10.cpp
@@ -8,7 +8,7 @@ struct NonLiteral { NonLiteral(); };
 // [C++1y] - void
 constexpr void f() {}
 #ifndef CXX1Y
-// expected-error@-2 {{'void' is not a literal type}}
+// expected-error@-2 {{constexpr function with non-literal return type 'void' is a C++23 extension}}
 #endif
 
 // - a scalar type
@@ -40,12 +40,12 @@ constexpr ClassTemp<int> classtemplate2[] = {};
 struct UserProvDtor {
   ~UserProvDtor(); // expected-note {{has a user-provided destructor}}
 };
-constexpr int f(UserProvDtor) { return 0; } // expected-error {{'UserProvDtor' is not a literal type}}
+constexpr int f(UserProvDtor) { return 0; } // expected-error {{non-literal parameter type 'UserProvDtor'}}
 struct NonTrivDtor {
   constexpr NonTrivDtor();
   virtual ~NonTrivDtor() = default; // expected-note {{has a non-trivial destructor}} expected-note {{because it is virtual}}
 };
-constexpr int f(NonTrivDtor) { return 0; } // expected-error {{'NonTrivDtor' is not a literal type}}
+constexpr int f(NonTrivDtor) { return 0; } // expected-error {{non-literal parameter type 'NonTrivDtor'}}
 struct NonTrivDtorBase {
   ~NonTrivDtorBase();
 };
@@ -53,7 +53,7 @@ template<typename T>
 struct DerivedFromNonTrivDtor : T { // expected-note {{'DerivedFromNonTrivDtor<NonTrivDtorBase>' is not literal because it has base class 'NonTrivDtorBase' of non-literal type}}
   constexpr DerivedFromNonTrivDtor();
 };
-constexpr int f(DerivedFromNonTrivDtor<NonTrivDtorBase>) { return 0; } // expected-error {{constexpr function's 1st parameter type 'DerivedFromNonTrivDtor<NonTrivDtorBase>' is not a literal type}}
+constexpr int f(DerivedFromNonTrivDtor<NonTrivDtorBase>) { return 0; } // expected-error {{constexpr function with 1st non-literal parameter type 'DerivedFromNonTrivDtor<NonTrivDtorBase>' is a C++23 extension}}
 struct TrivDtor {
   constexpr TrivDtor();
 };
@@ -77,11 +77,11 @@ struct CtorTemplate {
 struct CopyCtorOnly { // expected-note {{'CopyCtorOnly' is not literal because it is not an aggregate and has no constexpr constructors other than copy or move constructors}}
   constexpr CopyCtorOnly(CopyCtorOnly&);
 };
-constexpr int f(CopyCtorOnly) { return 0; } // expected-error {{'CopyCtorOnly' is not a literal type}}
+constexpr int f(CopyCtorOnly) { return 0; } // expected-error {{non-literal parameter type 'CopyCtorOnly'}}
 struct MoveCtorOnly { // expected-note {{no constexpr constructors other than copy or move constructors}}
   constexpr MoveCtorOnly(MoveCtorOnly&&);
 };
-constexpr int f(MoveCtorOnly) { return 0; } // expected-error {{'MoveCtorOnly' is not a literal type}}
+constexpr int f(MoveCtorOnly) { return 0; } // expected-error {{non-literal parameter type 'MoveCtorOnly'}}
 template<typename T>
 struct CtorArg {
   constexpr CtorArg(T);
@@ -97,7 +97,7 @@ struct Derived : HasVBase {
 template<typename T> struct DerivedFromVBase : T { // expected-note {{struct with virtual base class is not a literal type}}
   constexpr DerivedFromVBase();
 };
-constexpr int f(De...
[truncated]

Copy link

github-actions bot commented Jan 11, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@Endilll Endilll left a comment

Choose a reason for hiding this comment

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

I checked changes to DR tests, as that's the only part of this PR I'm competent enough for.

clang/test/CXX/drs/dr13xx.cpp Outdated Show resolved Hide resolved
clang/test/CXX/drs/dr13xx.cpp Outdated Show resolved Hide resolved
clang/test/CXX/drs/dr13xx.cpp Outdated Show resolved Hide resolved
clang/test/CXX/drs/dr14xx.cpp Outdated Show resolved Hide resolved
clang/test/CXX/drs/dr16xx.cpp Outdated Show resolved Hide resolved
clang/test/CXX/drs/dr6xx.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

Overall, I think this looks good.
Afaict the whole paper is implemented.

clang/lib/Sema/SemaDeclCXX.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaDeclCXX.cpp Outdated Show resolved Hide resolved
clang/lib/Sema/SemaDeclCXX.cpp Outdated Show resolved Hide resolved
clang/docs/ReleaseNotes.rst Show resolved Hide resolved
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

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

Looks good to me modulo the tiniest of nitpicks

clang/lib/Sema/SemaDeclCXX.cpp Outdated Show resolved Hide resolved
@Fznamznon
Copy link
Contributor Author

@Endilll , are you ok with the changes?
BTW, do we want this for 18, or it is better to wait and merge after cutoff?

@cor3ntin
Copy link
Contributor

@Fznamznon I think that unless bots find something, this is good for 18. I don't see anything that concerns me

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

Other than the one diagnostic concern, the changes LGTM and I think are fine for Clang 18.

@cor3ntin
Copy link
Contributor

CheckConstexprFunctionDefinition will return false for a destructor if defaultedDestructorIsConstexpr returns false (unless we want to emit a diagnostic)`

as @groundswellaudio pointed out in #78195, this is because

data().DefaultedDestructorIsConstexpr = false;

will set DefaultedDestructorIsConstexpr regardless of language mode, which it should not.
(and CheckConstexprFunctionDefinition should not look at defaultedDestructorIsConstexprin c++23 mode because of
that change https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2448r2.html#pnum_38

We should also inspect uses and setting of DefaultedDefaultConstructorIsConstexpr

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

I found some possible changes to be made to the diagnostics.

clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
clang/include/clang/Basic/DiagnosticSemaKinds.td Outdated Show resolved Hide resolved
Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the implementation!

@Fznamznon Fznamznon merged commit 99500e8 into llvm:main Mar 7, 2024
5 checks passed
@stbergmann
Copy link
Collaborator

This change started to cause

$ cat test.cc
template<typename T> struct P {
    ~P() { p->f(); }
    T * p;
};
struct X;
struct B { virtual ~B(); };
struct S: B { P<X> x; };
$ clang++ -std=c++23 -fsyntax-only test.cc
test.cc:2:13: error: member access into incomplete type 'X'
    2 |     ~P() { p->f(); }
      |             ^
test.cc:7:8: note: in instantiation of member function 'P<X>::~P' requested here
    7 | struct S: B { P<X> x; };
      |        ^
test.cc:5:8: note: forward declaration of 'X'
    5 | struct X;
      |        ^
1 error generated.

to fail, which I think is not intentional?

@Fznamznon
Copy link
Contributor Author

to fail, which I think is not intentional?

No, it is not. Fails only for c++23 now. I'm looking.

@Fznamznon
Copy link
Contributor Author

Ok, now I looked at it a bit more, and I think this is expected due to the change and how clang behaved before this patch. After this patch implicit destructor of S, i.e. ~S becomes constexpr. It used to be not constexpr because neither P nor B had constexpr destructors. clang defines and emits errors for constexpr defaulted members due to this bit here:

DefineDefaultedFunction(*this, M, M->getLocation());

So, once I manually change this example to force ~S() to be constexpr, older clang produces the same error:

template<typename T> struct P {
    cosntexpr ~P() { p->f(); } // add constexpr
    T * p;
};
struct X;
struct B { virtual constexpr ~B(); }; // add constexpr
struct S: B { P<X> x; }; // now all bases and members have constexpr destructors, ~S() is constexpr

Demo https://godbolt.org/z/rT3bWMdqz (note I took 18.1.0 clang which doesn't have P2448R2 implementation).

I'm not quite sure the error itself is correct, I've got to educate myself about standard matters a bit more to understand this, but no other compiler errors on this https://godbolt.org/z/Ehhn5sno7.

cc @AaronBallman , @cor3ntin, @zygoloid in case you can weigh in here

@stbergmann
Copy link
Collaborator

I think the discourse thread Clang is overly aggressive when evaluating constexpr functions is relevant here.

@AaronBallman
Copy link
Collaborator

Yeah, I think this is related to https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0859r0.html and I think this comment from @zygoloid might be related as well: #12223 (comment)

It's possible this is valid for Clang to reject because of eager instantiation, but I'm not 100% convinced yet either. CC @erichkeane @hubert-reinterpretcast as well

@amykhuang
Copy link
Collaborator

Hi, this is causing failures for Chrome, building with c++20:

$ cat t.cc
template <typename Type> class StrongAlias {
  friend constexpr bool operator==(const StrongAlias &, const StrongAlias &) = default;
  Type value;
};

class Token {};
bool operator==(const Token&, const Token&);

StrongAlias<Token> a;

$ clang -std=c++20 t.cc
t.cc:2:3: error: defaulted definition of equality comparison operator cannot be declared constexpr because it invokes a non-constexpr comparison function 
    2 |   friend constexpr bool operator==(const StrongAlias &, const StrongAlias &) = default;
      |   ^
t.cc:9:20: note: in instantiation of template class 'StrongAlias<Token>' requested here
    9 | StrongAlias<Token> a;
      |                    ^
t.cc:3:8: note: non-constexpr comparison function would be used to compare member 'value'
    3 |   Type value;
      |        ^
t.cc:7:6: note: non-constexpr comparison function declared here
    7 | bool operator==(const Token&, const Token&);
      |      ^
1 error generated.

@amykhuang
Copy link
Collaborator

I'm assuming this behavior is unintended? Sorry for the late revert

amykhuang added a commit that referenced this pull request Mar 13, 2024
Revert "[Clang][C++23] Implement P2448R2: Relaxing some constexpr
restrictions (#77753)"

This reverts commit 99500e8 because it
causes a behavior change for std=c++20. See
#77753.
amykhuang added a commit to amykhuang/llvm-project that referenced this pull request Mar 13, 2024
@amykhuang
Copy link
Collaborator

Nevermind, unreverted since there appear to be newer tests that use this patch.

@dyung
Copy link
Collaborator

dyung commented Mar 13, 2024

Nevermind, unreverted since there appear to be newer tests that use this patch.

@amykhuang your unrevert (9bfa506) appears to be an empty commit? Was that intended?

@amykhuang
Copy link
Collaborator

Whoops, not very familiar with git pull requests, I guess. actually reapplying the commit now

@amykhuang
Copy link
Collaborator

I looked at this a bit and the change that increases the constexpr restrictions is that ext_defaulted_comparison_constexpr_mismatch (which was added in https://reviews.llvm.org/D146090) became err_incorrect_defaulted_comparison_constexpr?

@Fznamznon
Copy link
Contributor Author

Fznamznon commented Mar 14, 2024

I looked at this a bit and the change that increases the constexpr restrictions is that ext_defaulted_comparison_constexpr_mismatch (which was added in https://reviews.llvm.org/D146090) became err_incorrect_defaulted_comparison_constexpr?

Yes, it was intentional change. And the diagnostic you're seeing is valid for C++20. Prior C++23 defaulted function that invokes non-constexpr function under the hood cannot be marked constexpr. Since relaxing that was made in https://reviews.llvm.org/D146090 is a part of P2448R2, but other parts implemented by this patch are C++23-only due to the problems I described in #77753 (comment) , I've made all parts of P2448R2 consistent.
I would suggest opening an issue here on GitHub to discuss whether we would like to make the feature inconsistent. Since some parts of the feature can be backported to older standards easily enough.

qihangkong pushed a commit to rvgpu/llvm that referenced this pull request Apr 18, 2024
…trictions (#77753)"

This reverts commit 99500e8 because it
causes a behavior change for std=c++20. See
llvm/llvm-project#77753.
BertalanD added a commit to BertalanD/ladybird that referenced this pull request Jun 4, 2024
BertalanD added a commit to BertalanD/ladybird that referenced this pull request Jun 4, 2024
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.

None yet

8 participants