[PATCH] D50291: [C++] Delay checking of constexpr-ness for special members.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:6045
           // Inform the class that we've finished declaring this member.
           Record->finishedDefaultedOrDeletedMember(M);
           M->setTrivialForCall(
----------------
Your new handling should go up here, before we mark the method as finished (rather than marking it as finished twice).


================
Comment at: lib/Sema/SemaDeclCXX.cpp:6557-6558
   //   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++1y), this is checked elsewhere.
-  bool Constexpr = defaultedSpecialMemberIsConstexpr(*this, RD, CSM,
-                                                     HasConstParam);
-  if ((getLangOpts().CPlusPlus14 ? !isa<CXXDestructorDecl>(MD)
-                                 : isa<CXXConstructorDecl>(MD)) &&
-      MD->isConstexpr() && !Constexpr &&
-      MD->getTemplatedKind() == FunctionDecl::TK_NonTemplate) {
-    Diag(MD->getLocStart(), diag::err_incorrect_defaulted_constexpr) << CSM;
-    // FIXME: Explain why the special member can't be constexpr.
-    HadError = true;
-  }
+  // We need to delay this, because at this point we may not have enough
+  // information to determine if MD is really constexpr or not.
 
----------------
We should only delay this for the first declaration, just as we only delay the corresponding check for exception specifications for the first declaration. (Maybe move the call to `CheckExplicitlyDefaultedSpecialMemberConstexpr(MD);` from the out-of-class default case to here, inside an `if (!First)`.)


================
Comment at: lib/Sema/SemaDeclCXX.cpp:6560-6575
   //   and may have an explicit exception-specification only if it is compatible
   //   with the exception-specification on the implicit declaration.
   if (Type->hasExceptionSpec()) {
     // Delay the check if this is the first declaration of the special member,
     // since we may not have parsed some necessary in-class initializers yet.
     if (First) {
       // If the exception specification needs to be instantiated, do so now,
----------------
We should really implement the exception specification part and the constexpr part the same way. Either we should have a single list of special members we need to finish, or we should figure them out at the end of the class (like you're doing for `constexpr` now). But implementing exception specs one way and `constexpr` a different way invites bugs.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:6623
+
+  // Do not apply this rule to members of class templates, since core issue 1358
+  // makes such functions always instantiate to constexpr functions. For
----------------
... what rule? (This comment has lost the prior comment that it's commenting on.)


Repository:
  rC Clang

https://reviews.llvm.org/D50291