Handle explicitly defaulted consteval special members.

[usaxena95]

Summary

Followup patch for D128083

Previously, using a non-consteval constructor from an consteval constructor would code generates the consteval constructor.
Example

template <typename T>
struct S {
  T i;
  consteval S() = default;
};
struct Foo {
    Foo() {}
};
void func() {
  S<Foo> three; // incorrectly accepted by clang.
}

This happened because clang erroneously disregards consteval specifier for a consteval explicitly defaulted special member functions in a class template if it has dependent data members without a consteval default constructor.

According to

C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358):
If the instantiated template specialization of a constexpr function
template or member function of a class template would fail to satisfy
the requirements for a constexpr function or constexpr constructor, that
specialization is still a constexpr function or constexpr constructor,
even though a call to such a function cannot appear in a constant
expression.

Therefore the consteval defaulted constructor of a class template should be considered consteval even if the data members' default constructors are not consteval.
Keeping this constructor consteval allows complaining while processing the call to data member constructors.
(Same applies for other special member functions).

This works fine even when we have more than one default constructors since we process the constructors after the templates are instantiated.

This does not address initialization issues raised in
2602 and compiler divergence seen in https://godbolt.org/z/va9EMvvMe

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

[usaxena95]

Added tests for multiple constructors in a templated class.

[usaxena95]

Verify that non-consteval constructors (among others) cannot be used in a consteval context.

[ilya-biryukov]

Should we follow suggestion from CWG Issue 2602 to fix this instead?
I think the trick is to keep the constructor consteval, but complain when processing its call, e.g. somewhere around HandleConstructorCall in ExprConstant.cpp.
WDYT?

clang/lib/Sema/SemaDeclCXX.cpp

7207

The function attempts to check whether the implicitly defined member would be constexpr, which should not depend on what is written in the expliclitly defaulted declaration.

So the check should definitely be performed somewhere outside this function.

[usaxena95]

Addressed comment.

[usaxena95]

As discussed offline, we are already following the discussion 2602.
We are keeping the constructor consteval to allow complaining when we are actually processing the call to data member constructors (instead of the consteval constructor itself.)

[ilya-biryukov]

LGTM from my side. Thanks for explaining, I was confused because I looked at the code right after defaulted check and thought we were working around that particular error.
Turns out this is not the case and the change was actually marking the function as consteval, just as was expected.

@aaron.ballman, could you also take a quick look here?

clang/test/SemaCXX/cxx2a-consteval.cpp

857

NIT

[aaron.ballman]

This is generally looking good to me, thank you! Just a few minor points and I think this will be ready.

clang/lib/Sema/SemaDeclCXX.cpp

7564

On my earlier patch, @rsmith had suggested that we check if MD is being instantiated here rather than RD to more closely match the standards wording. I think that's a reasonable suggestion -- WDYT?

clang/test/SemaCXX/cxx2a-consteval.cpp

805

Why do we need to use the regex here (and elsewhere)?

[aaron.ballman]

clang/test/SemaCXX/cxx2a-consteval.cpp

812

This likely has nothing to do with your changes here, but what the heck is with that leading & in this already-pretty-surprisingly-weird note? Is that aiming for (&fail1)->operator=(good0) for some reason?

[shafik]

clang/test/SemaCXX/cxx2a-consteval.cpp

812

Note, we also see a similar diagnostic in SemaCXX/constant-expression-cxx11.cpp so this looks like it existed before.

[usaxena95]

Addressed comments.

[usaxena95]

clang/test/SemaCXX/cxx2a-consteval.cpp

805

I wanted to omit the namespace name in the error messages and make the namespace more descriptive. See the .* in the error messages.

[aaron.ballman]

clang/test/SemaCXX/cxx2a-consteval.cpp

805

I think it's better to skip the regex and spell out the diagnostic fully (even with the descriptive namespace name). Regular expression matching is going to be slower than regular matching (generally) and there's no real need for it here except brevity.

812

Good catch! I filed https://github.com/llvm/llvm-project/issues/57081 so that hopefully we go back to make that wording better some day.

[usaxena95]

Removed regex matching for diagnostics.

[aaron.ballman]

Thanks, this LGTM! Please be sure to add a release note for the fix before landing.

[usaxena95]

Added a release note.

[Mordante]

This change breaks the libc++ test libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp. (https://buildkite.com/llvm-project/libcxx-ci/builds/13149#0182d0d4-341a-4b41-b67f-12937f41e6d5)

Looking at the description of this patch it should only affect consteval functions, but it seems to affect constexpr functions too. Can you have a look?

[aaron.ballman]

In D131479#3753462, @Mordante wrote:

This change breaks the libc++ test libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp. (https://buildkite.com/llvm-project/libcxx-ci/builds/13149#0182d0d4-341a-4b41-b67f-12937f41e6d5)

Looking at the description of this patch it should only affect consteval functions, but it seems to affect constexpr functions too. Can you have a look?

Agreed that we should take a look, but it's interesting to note that libstdc++ seems to have a different behavior than libc++ here: https://godbolt.org/z/6cWhf6GYj (the last three compiler panes show Clang 14 libstd++, Clang 14 libc++, and Clang trunk libc++). How sure are you that the libc++ test is actually correct, because (without checking the standard, so take with a giant grain of salt) this seems like it might have fixed a bug in libc++? The release note says this is expected to impact constexpr and consteval default special member functions, so behavioral changes to constexpr aren't unexpected (though I agree that the commit message didn't mention constexpr so the changes may seem surprising).

[Mordante]

In D131479#3753533, @aaron.ballman wrote:

In D131479#3753462, @Mordante wrote:

This change breaks the libc++ test libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp. (https://buildkite.com/llvm-project/libcxx-ci/builds/13149#0182d0d4-341a-4b41-b67f-12937f41e6d5)

Looking at the description of this patch it should only affect consteval functions, but it seems to affect constexpr functions too. Can you have a look?

Agreed that we should take a look, but it's interesting to note that libstdc++ seems to have a different behavior than libc++ here: https://godbolt.org/z/6cWhf6GYj (the last three compiler panes show Clang 14 libstd++, Clang 14 libc++, and Clang trunk libc++). How sure are you that the libc++ test is actually correct, because (without checking the standard, so take with a giant grain of salt) this seems like it might have fixed a bug in libc++? The release note says this is expected to impact constexpr and consteval default special member functions, so behavioral changes to constexpr aren't unexpected (though I agree that the commit message didn't mention constexpr so the changes may seem surprising).

Thanks for confirming it's indeed intended to affect constexpr too! It was a bit confusing since it also referred to C++14, which didn't have consteval. I wasn't entirely sure about the status of libc++, but since the patch gave of a mixed message I wanted make sure that constexpr was intended.

I've done some further digging and according to cppreference the paper P0602R4 was a DR against C++20. This was proposed in the paper.
Looking at the changes in the addressing this paper I see no C++ version checks https://reviews.llvm.org/D32385
In a followup introduces tests using C++ version checks https://reviews.llvm.org/D54772
The failing test was introduced in 308624127fd6cc36558b6eee4d4ffa4e215a074e before the paper was voted in

So I think there's indeed a bug in libc++ which was hidden since Clang hadn't implemented some DRs. I will look at the libc++ side what needs to be done to fix the CI failures.

[aaron.ballman]

In D131479#3753586, @Mordante wrote:

In D131479#3753533, @aaron.ballman wrote:

In D131479#3753462, @Mordante wrote:

This change breaks the libc++ test libcxx/test/std/utilities/optional/optional.object/optional.object.ctor/move.fail.cpp. (https://buildkite.com/llvm-project/libcxx-ci/builds/13149#0182d0d4-341a-4b41-b67f-12937f41e6d5)

Looking at the description of this patch it should only affect consteval functions, but it seems to affect constexpr functions too. Can you have a look?

Agreed that we should take a look, but it's interesting to note that libstdc++ seems to have a different behavior than libc++ here: https://godbolt.org/z/6cWhf6GYj (the last three compiler panes show Clang 14 libstd++, Clang 14 libc++, and Clang trunk libc++). How sure are you that the libc++ test is actually correct, because (without checking the standard, so take with a giant grain of salt) this seems like it might have fixed a bug in libc++? The release note says this is expected to impact constexpr and consteval default special member functions, so behavioral changes to constexpr aren't unexpected (though I agree that the commit message didn't mention constexpr so the changes may seem surprising).

Thanks for confirming it's indeed intended to affect constexpr too! It was a bit confusing since it also referred to C++14, which didn't have consteval. I wasn't entirely sure about the status of libc++, but since the patch gave of a mixed message I wanted make sure that constexpr was intended.

It was a good question to bring up!

I've done some further digging and according to cppreference the paper P0602R4 was a DR against C++20. This was proposed in the paper.
Looking at the changes in the addressing this paper I see no C++ version checks https://reviews.llvm.org/D32385
In a followup introduces tests using C++ version checks https://reviews.llvm.org/D54772
The failing test was introduced in 308624127fd6cc36558b6eee4d4ffa4e215a074e before the paper was voted in

So I think there's indeed a bug in libc++ which was hidden since Clang hadn't implemented some DRs. I will look at the libc++ side what needs to be done to fix the CI failures.

Great sleuth work! Thank you for the investigation!

[Mordante]

In D131479#3755266, @aaron.ballman wrote:

In D131479#3753586, @Mordante wrote:

So I think there's indeed a bug in libc++ which was hidden since Clang hadn't implemented some DRs. I will look at the libc++ side what needs to be done to fix the CI failures.

Great sleuth work! Thank you for the investigation!

The issue was indeed in libc++, which has been fixed :-)

clang/lib/Sema/SemaDeclCXX.cpp

 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTLambda.h"
 #include "clang/AST/ASTMutationListener.h"
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/ComparisonCategories.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/AST/TypeOrdering.h"
 #include "clang/Basic/AttributeCommonInfo.h"
specialMemberIsConstexpr(Sema &S, CXXRecordDecl *ClassDecl,
   return SMOR.getMethod()->isConstexpr();
 }
 
 /// Determine whether the specified special member function would be constexpr
 /// if it were implicitly defined.
 static bool defaultedSpecialMemberIsConstexpr(
     Sema &S, CXXRecordDecl *ClassDecl, Sema::CXXSpecialMember CSM,
     bool ConstArg, CXXConstructorDecl *InheritedCtor = nullptr,
-    Sema::InheritedConstructorInfo *Inherited = nullptr) {
+    Sema::InheritedConstructorInfo *Inherited = nullptr,
+    CXXMethodDecl *SpecialMemberDecl = nullptr) {
   if (!S.getLangOpts().CPlusPlus11)
     return false;
 
+  // C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358):
+  //   If the instantiated template specialization of a constexpr function
+  //   template or member function of a class template would fail to satisfy
+  //   the requirements for a constexpr function or constexpr constructor, that
+  //   specialization is still a constexpr function or constexpr constructor,
+  //   even though a call to such a function cannot appear in a constant
+  //   expression.
+  if (SpecialMemberDecl &&
+      (isa<ClassTemplateSpecializationDecl>(ClassDecl) ||
+        SpecialMemberDecl->isFunctionTemplateSpecialization())
+        && SpecialMemberDecl->isConstexpr())
+    return true;
+
   // C++11 [dcl.constexpr]p4:
   // In the definition of a constexpr constructor [...]
   bool Ctor = true;
   switch (CSM) {
   case Sema::CXXDefaultConstructor:
     if (Inherited)
       break;
     // Since default constructor lookup is essentially trivial (and cannot
bool Sema::CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
   //   An explicitly-defaulted function may be declared constexpr only if it
   //   would have been implicitly declared as constexpr,
   // Do not apply this rule to members of class templates, since core issue 1358
   // makes such functions always instantiate to constexpr functions. For
   // functions which cannot be constexpr (for non-constructors in C++11 and for
   // destructors in C++14 and C++17), this is checked elsewhere.
   //
   // FIXME: This should not apply if the member is deleted.
-  bool Constexpr = defaultedSpecialMemberIsConstexpr(*this, RD, CSM,
-                                                     HasConstParam);
+  bool Constexpr = defaultedSpecialMemberIsConstexpr(
+      *this, RD, CSM, HasConstParam, nullptr, nullptr, MD);
   if ((getLangOpts().CPlusPlus20 ||
        (getLangOpts().CPlusPlus14 ? !isa<CXXDestructorDecl>(MD)
                                   : isa<CXXConstructorDecl>(MD))) &&
       MD->isConstexpr() && !Constexpr &&
       MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
     Diag(MD->getBeginLoc(), MD->isConsteval()
                                 ? diag::err_incorrect_defaulted_consteval
                                 : diag::err_incorrect_defaulted_constexpr)
         << CSM;
     // FIXME: Explain why the special member can't be constexpr.
     HadError = true;
   }
 
   if (First) {
     // C++2a [dcl.fct.def.default]p3:
     //   If a function is explicitly defaulted on its first declaration, it is

clang/test/SemaCXX/cxx0x-defaulted-functions.cpp

 void tester() {
   foo f, g(f);
   bar b, c(b);
   f = g;
   b = c;
 }
 
 template<typename T> struct S : T {
-  constexpr S() = default;
-  constexpr S(const S&) = default;
-  constexpr S(S&&) = default;
+  constexpr S() = default;         // expected-note {{previous declaration is here}}
+  constexpr S(const S&) = default; // expected-note {{previous declaration is here}}
+  constexpr S(S&&) = default;      // expected-note {{previous declaration is here}}
 };
 struct lit { constexpr lit() {} };
 S<lit> s_lit; // ok
 S<bar> s_bar; // ok
 
 struct Friends {
-  friend S<bar>::S();
-  friend S<bar>::S(const S&);
-  friend S<bar>::S(S&&);
+  // FIXME: these error may or may not be correct; there is an open question on
+  // the CWG reflectors about this.
+  friend S<bar>::S();          // expected-error {{non-constexpr declaration of 'S' follows constexpr declaration}}
+  friend S<bar>::S(const S&);  // expected-error {{non-constexpr declaration of 'S' follows constexpr declaration}}
+  friend S<bar>::S(S&&);       // expected-error {{non-constexpr declaration of 'S' follows constexpr declaration}}
 };
 
 namespace DefaultedFnExceptionSpec {
   // DR1330: The exception-specification of an implicitly-declared special
   // member function is evaluated as needed.
   template<typename T> T &&declval();
   template<typename T> struct pair {
     pair(const pair&) noexcept(noexcept(T(declval<T>())));

clang/test/SemaCXX/cxx2a-consteval.cpp

 
 void test() {
   static_assert(f1('a') == 4);
   static_assert(f2('a') == 4);
   constexpr int c = f1('a') + f2('a');
   static_assert(c == 8);
 }
 }
+
+namespace DefaultedTemp {
+template <typename T>
+struct default_ctor {
+  T data;
+  consteval default_ctor() = default; // expected-note {{non-constexpr constructor 'foo' cannot be used in a constant expression}}
+};
+
+template <typename T>
+struct copy {
+  T data;
+
+  consteval copy(const copy &) = default;            // expected-note {{non-constexpr constructor 'foo' cannot be used in a constant expression}}
+  consteval copy &operator=(const copy &) = default; // expected-note {{non-constexpr function 'operator=' cannot be used in a constant expression}}
+  copy() = default;
+};
+
+template <typename T>
+struct move {
+  T data;
+
+  consteval move(move &&) = default;            // expected-note {{non-constexpr constructor 'foo' cannot be used in a constant expression}}
+  consteval move &operator=(move &&) = default; // expected-note {{non-constexpr function 'operator=' cannot be used in a constant expression}}
+  move() = default;
+};
+
+struct foo {
+  foo() {}            // expected-note {{declared here}}
+  foo(const foo &) {} // expected-note {{declared here}}
+  foo(foo &&) {}      // expected-note {{declared here}}
+
+  foo& operator=(const foo &) { return *this; } // expected-note {{declared here}}
+  foo& operator=(foo &&) { return *this; }      // expected-note {{declared here}}
+};
+
+void func() {
+  default_ctor<foo> fail0; // expected-error {{call to consteval function 'DefaultedTemp::default_ctor<DefaultedTemp::foo>::default_ctor' is not a constant expression}} \
+                              expected-note {{in call to 'default_ctor()'}}
+
+  copy<foo> good0;
+  copy<foo> fail1{good0}; // expected-error {{call to consteval function 'DefaultedTemp::copy<DefaultedTemp::foo>::copy' is not a constant expression}} \
+                             expected-note {{in call to 'copy(good0)'}}
+  fail1 = good0;          // expected-error {{call to consteval function 'DefaultedTemp::copy<DefaultedTemp::foo>::operator=' is not a constant expression}} \
+                             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 'DefaultedTemp::move<DefaultedTemp::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 'DefaultedTemp::move<DefaultedTemp::foo>::operator=' is not a constant expression}} \
+                                                       expected-note {{in call to '&fail2->operator=(good1)'}}
+}
+} // namespace DefaultedTemp