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

[Rakete1111]

Summary

Specific class layouts meant that the constexpr-ness of a special member could only be decided after every special member was seen. However, this was at odds with the implementation, which checked the constexpr-ness for each special member in the order in which they were declared (hence that the bug only occurs when special members are ordered in a specific order).

This patch moves the checking of constexpr-ness after evaluating each special member.
This fixes https://llvm.org/pr38143

[Rakete1111]

Add missing test cases. :)

[rsmith]

lib/Sema/SemaDeclCXX.cpp

6045

Your new handling should go up here, before we mark the method as finished (rather than marking it as finished twice).

6557–6558

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

6560–6575

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.

6623

... what rule? (This comment has lost the prior comment that it's commenting on.)

include/clang/Sema/Sema.h

   void CheckConversionDeclarator(Declarator &D, QualType &R,
                                  StorageClass& SC);
   Decl *ActOnConversionDeclarator(CXXConversionDecl *Conversion);
   void CheckDeductionGuideDeclarator(Declarator &D, QualType &R,
                                      StorageClass &SC);
   void CheckDeductionGuideTemplate(FunctionTemplateDecl *TD);
 
   void CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD);
+  void CheckExplicitlyDefaultedSpecialMemberConstexpr(CXXMethodDecl *MD);
   void CheckExplicitlyDefaultedMemberExceptionSpec(CXXMethodDecl *MD,
                                                    const FunctionProtoType *T);
   void CheckDelayedMemberExceptionSpecs();
 
   //===--------------------------------------------------------------------===//
   // C++ Derived Classes
   //
 

lib/Sema/SemaDeclCXX.cpp

void Sema::CheckCompletedCXXClass(CXXRecordDecl *Record) {
   // "trivial_abi".
   bool HasTrivialABI = Record->hasAttr<TrivialABIAttr>();
 
   if (HasTrivialABI)
     Record->setHasTrivialSpecialMemberForCall();
 
   bool HasMethodWithOverrideControl = false,
        HasOverridingMethodWithoutOverrideControl = false;
+  CXXMethodDecl *CopyAssignment = nullptr, *MoveAssignment = nullptr;
   if (!Record->isDependentType()) {
     for (auto *M : Record->methods()) {
       // See if a method overloads virtual methods in a base
       // class without overriding any.
       if (!M->isStatic())
         DiagnoseHiddenVirtualMethods(M);
       if (M->hasAttr<OverrideAttr>())
         HasMethodWithOverrideControl = true;
       else if (M->size_overridden_methods() > 0)
         HasOverridingMethodWithoutOverrideControl = true;
       // Check whether the explicitly-defaulted special members are valid.
       if (!M->isInvalidDecl() && M->isExplicitlyDefaulted())
         CheckExplicitlyDefaultedSpecialMember(M);
 
       // For an explicitly defaulted or deleted special member, we defer
       // determining triviality until the class is complete. That time is now!
       CXXSpecialMember CSM = getSpecialMember(M);
       if (!M->isImplicit() && !M->isUserProvided()) {
         if (CSM != CXXInvalid) {
           M->setTrivial(SpecialMemberIsTrivial(M, CSM));
           // Inform the class that we've finished declaring this member.
           Record->finishedDefaultedOrDeletedMember(M);
           M->setTrivialForCall(
               HasTrivialABI ||
               SpecialMemberIsTrivial(M, CSM, TAH_ConsiderTrivialABI));
           Record->setTrivialForCallFlags(M);
         }
       }
 
+      if (CSM == CXXCopyAssignment)
+        CopyAssignment = M;
+      else if (CSM == CXXMoveAssignment)
+        MoveAssignment = M;
+
       // Set triviality for the purpose of calls if this is a user-provided
       // copy/move constructor or destructor.
       if ((CSM == CXXCopyConstructor || CSM == CXXMoveConstructor ||
            CSM == CXXDestructor) && M->isUserProvided()) {
         M->setTrivialForCall(HasTrivialABI);
         Record->setTrivialForCallFlags(M);
       }
 
       if (!M->isInvalidDecl() && M->isExplicitlyDefaulted() &&
           M->hasAttr<DLLExportAttr>()) {
         if (getLangOpts().isCompatibleWithMSVC(LangOptions::MSVC2015) &&
             M->isTrivial() &&
             (CSM == CXXDefaultConstructor || CSM == CXXCopyConstructor ||
              CSM == CXXDestructor))
           M->dropAttr<DLLExportAttr>();
 
         if (M->hasAttr<DLLExportAttr>()) {
           DefineImplicitSpecialMember(*this, M, M->getLocation());
           ActOnFinishInlineFunctionDef(M);
         }
       }
     }
+
+    // Now we can decide whether the special members are constexpr.
+    auto ActOnSpecialMember = [this, &Record](CXXMethodDecl *MD) {
+      if (!MD->isInvalidDecl() && MD->isExplicitlyDefaulted()) {
+        CheckExplicitlyDefaultedSpecialMemberConstexpr(MD);
+        Record->finishedDefaultedOrDeletedMember(MD);
+      }
+    };
+    for (auto *M : Record->ctors())
+      ActOnSpecialMember(M);
+    if (CopyAssignment)
+      ActOnSpecialMember(CopyAssignment);
+    if (MoveAssignment)
+      ActOnSpecialMember(MoveAssignment);
+    if (CXXDestructorDecl *CD = Record->getDestructor())
+      ActOnSpecialMember(CD);
   }
 
   if (HasMethodWithOverrideControl &&
       HasOverridingMethodWithoutOverrideControl) {
     // At least one method has the 'override' control declared.
     // Diagnose all other overridden methods which do not have 'override' specified on them.
     for (auto *M : Record->methods())
       DiagnoseAbsenceOfOverrideControl(M);
if (ExpectedParams && ArgType->isReferenceType()) {
     assert(CSM == CXXCopyAssignment && "unexpected non-ref argument");
     Diag(MD->getLocation(), diag::err_defaulted_copy_assign_not_ref);
     HadError = true;
   }
 
   // C++11 [dcl.fct.def.default]p2:
   //   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++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.
 
   //   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,
       // before we clobber it with an EST_Unevaluated specification below.
       if (Type->getExceptionSpecType() == EST_Uninstantiated) {
         InstantiateExceptionSpec(MD->getLocStart(), MD);
         Type = MD->getType()->getAs<FunctionProtoType>();
       }
       DelayedDefaultedMemberExceptionSpecs.push_back(std::make_pair(MD, Type));
     } else
       CheckExplicitlyDefaultedMemberExceptionSpec(MD, Type);
   }
 
   //   If a function is explicitly defaulted on its first declaration,
   if (First) {
     //  -- it is implicitly considered to be constexpr if the implicit
     //     definition would be,
-    MD->setConstexpr(Constexpr);
-
     //  -- it is implicitly considered to have the same exception-specification
     //     as if it had been implicitly declared,
     FunctionProtoType::ExtProtoInfo EPI = Type->getExtProtoInfo();
     EPI.ExceptionSpec.Type = EST_Unevaluated;
     EPI.ExceptionSpec.SourceDecl = MD;
     MD->setType(Context.getFunctionType(ReturnType,
                                         llvm::makeArrayRef(&ArgType,
                                                            ExpectedParams),
if (First) {
       HadError = true;
     }
   }
 
   if (HadError)
     MD->setInvalidDecl();
 }
 
+void Sema::CheckExplicitlyDefaultedSpecialMemberConstexpr(CXXMethodDecl *MD) {
+  CXXSpecialMember CSM = getSpecialMember(MD);
+  assert(MD && MD->isExplicitlyDefaulted() && CSM != CXXInvalid &&
+         "not an explicitly-defaulted special member");
+
+  const FunctionProtoType *Type = MD->getType()->getAs<FunctionProtoType>();
+
+  unsigned ExpectedParams =
+      CSM != CXXDefaultConstructor && CSM != CXXDestructor;
+  QualType ArgType = ExpectedParams ? Type->getParamType(0) : QualType();
+
+  bool HasConstParam = ExpectedParams && ArgType->isReferenceType() &&
+                       ArgType->getPointeeType().isConstQualified();
+
+  // 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, MD->getParent(),
+                                                     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.
+    MD->setInvalidDecl();
+  }
+
+  if (MD == MD->getCanonicalDecl())
+    MD->setConstexpr(Constexpr);
+}
+
 /// Check whether the exception specification provided for an
 /// explicitly-defaulted special member matches the exception specification
 /// that would have been generated for an implicit special member, per
 /// C++11 [dcl.fct.def.default]p2.
 void Sema::CheckExplicitlyDefaultedMemberExceptionSpec(
     CXXMethodDecl *MD, const FunctionProtoType *SpecifiedType) {
   // If the exception specification was explicitly specified but hadn't been
   // parsed when the method was defaulted, grab it now.
if (MD) {
 
     // If the method was defaulted on its first declaration, we will have
     // already performed the checking in CheckCompletedCXXClass. Such a
     // declaration doesn't trigger an implicit definition.
     if (Primary->getCanonicalDecl()->isDefaulted())
       return;
 
     CheckExplicitlyDefaultedSpecialMember(MD);
+    CheckExplicitlyDefaultedSpecialMemberConstexpr(MD);
 
     if (!MD->isInvalidDecl())
       DefineImplicitSpecialMember(*this, MD, DefaultLoc);
   } else {
     Diag(DefaultLoc, diag::err_default_special_members);
   }
 }
 

test/CXX/special/class.copy/p10.cpp

+// RUN: %clang_cc1 -std=c++14 -fsyntax-only -verify %s
+// expected-no-diagnostics
+
+// The implicitly-defined copy/move assignment operator is constexpr if
+// - X is a literal type, and
+// - [...]
+//
+// PR38143: Order of defaulted special members matters.
+struct X {
+  constexpr X() = default;
+  constexpr X(const X &) = default;
+  constexpr X &operator=(const X &) = default;
+  ~X() = default;
+  constexpr X(X &&) = default;
+  constexpr X &operator=(X &&) = default;
+};
+
+struct Y {
+  constexpr Y &operator=(const Y &) = default;
+  constexpr Y &operator=(Y &&) = default;
+  constexpr Y() = default;
+  constexpr Y(const Y &) = default;
+  ~Y() = default;
+  constexpr Y(Y &&) = default;
+};