Revert "[Clang] Implement P0848 (Conditionally Trivial Special Member Functions)"

See bug report here: llvm#57351
This reverts commit 7171615.

Author: royjacobson

clang/docs/ReleaseNotes.rst

@@ -167,10 +167,6 @@ C++20 Feature Support
   (C++14 [dcl.constexpr]p6 (CWG DR647/CWG DR1358))
 - Correctly defer dependent immediate function invocations until template instantiation.
   This fixes `GH55601 <https://github.com/llvm/llvm-project/issues/55601>`_.
-- Implemented "Conditionally Trivial Special Member Functions" (`P0848 <https://wg21.link/p0848r3>`_).
-  Note: The handling of deleted functions is not yet compliant, as Clang
-  does not implement `DR1496 <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1496>`_
-  and `DR1734 <https://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1734>`_.
 
 
 

clang/lib/AST/DeclCXX.cpp

@@ -894,11 +894,9 @@ void CXXRecordDecl::addedMember(Decl *D) {
         // This is an extension in C++03.
         data().PlainOldData = false;
       }
-      // When instantiating a class, we delay updating the destructor and
-      // triviality properties of the class until selecting a destructor and
-      // computing the eligibility of its special member functions. This is
-      // because there might be function constraints that we need to evaluate
-      // and compare later in the instantiation.
+      // We delay updating destructor relevant properties until
+      // addedSelectedDestructor.
+      // FIXME: Defer this for the other special member functions as well.
       if (!Method->isIneligibleOrNotSelected()) {
         addedEligibleSpecialMemberFunction(Method, SMKind);
       }
@@ -1439,12 +1437,10 @@ void CXXRecordDecl::finishedDefaultedOrDeletedMember(CXXMethodDecl *D) {
 
   // Update which trivial / non-trivial special members we have.
   // addedMember will have skipped this step for this member.
-  if (!D->isIneligibleOrNotSelected()) {
-    if (D->isTrivial())
-      data().HasTrivialSpecialMembers |= SMKind;
-    else
-      data().DeclaredNonTrivialSpecialMembers |= SMKind;
-  }
+  if (D->isTrivial())
+    data().HasTrivialSpecialMembers |= SMKind;
+  else
+    data().DeclaredNonTrivialSpecialMembers |= SMKind;
 }
 
 void CXXRecordDecl::setCaptures(ASTContext &Context,

clang/lib/Frontend/InitPreprocessor.cpp

@@ -674,9 +674,6 @@ static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
   // C++20 features.
   if (LangOpts.CPlusPlus20) {
     //Builder.defineMacro("__cpp_aggregate_paren_init", "201902L");
-    // P0848 is implemented, but we're still waiting for other concepts
-    // issues to be addressed before bumping __cpp_concepts up to 202002L.
-    // Refer to the discussion of this at https://reviews.llvm.org/D128619.
     Builder.defineMacro("__cpp_concepts", "201907L");
     Builder.defineMacro("__cpp_conditional_explicit", "201806L");
     //Builder.defineMacro("__cpp_consteval", "201811L");

clang/lib/Sema/SemaDecl.cpp

@@ -17968,6 +17968,7 @@ void Sema::ActOnLastBitfield(SourceLocation DeclLoc,
   AllIvarDecls.push_back(Ivar);
 }
 
+namespace {
 /// [class.dtor]p4:
 ///   At the end of the definition of a class, overload resolution is
 ///   performed among the prospective destructors declared in that class with
@@ -17976,7 +17977,7 @@ void Sema::ActOnLastBitfield(SourceLocation DeclLoc,
 ///
 /// We do the overload resolution here, then mark the selected constructor in the AST.
 /// Later CXXRecordDecl::getDestructor() will return the selected constructor.
-static void ComputeSelectedDestructor(Sema &S, CXXRecordDecl *Record) {
+void ComputeSelectedDestructor(Sema &S, CXXRecordDecl *Record) {
   if (!Record->hasUserDeclaredDestructor()) {
     return;
   }
@@ -18034,130 +18035,7 @@ static void ComputeSelectedDestructor(Sema &S, CXXRecordDecl *Record) {
     Record->addedSelectedDestructor(dyn_cast<CXXDestructorDecl>(OCS.begin()->Function));
   }
 }
-
-/// [class.mem.special]p5
-/// Two special member functions are of the same kind if:
-/// - they are both default constructors,
-/// - they are both copy or move constructors with the same first parameter
-///   type, or
-/// - they are both copy or move assignment operators with the same first
-///   parameter type and the same cv-qualifiers and ref-qualifier, if any.
-static bool AreSpecialMemberFunctionsSameKind(ASTContext &Context,
-                                              CXXMethodDecl *M1,
-                                              CXXMethodDecl *M2,
-                                              Sema::CXXSpecialMember CSM) {
-  if (CSM == Sema::CXXDefaultConstructor)
-    return true;
-  if (!Context.hasSameType(M1->getParamDecl(0)->getType(),
-                           M2->getParamDecl(0)->getType()))
-    return false;
-  if (!Context.hasSameType(M1->getThisType(), M2->getThisType()))
-    return false;
-
-  return true;
-}
-
-/// [class.mem.special]p6:
-/// An eligible special member function is a special member function for which:
-/// - the function is not deleted,
-/// - the associated constraints, if any, are satisfied, and
-/// - no special member function of the same kind whose associated constraints
-///   [CWG2595], if any, are satisfied is more constrained.
-static void SetEligibleMethods(Sema &S, CXXRecordDecl *Record,
-                               ArrayRef<CXXMethodDecl *> Methods,
-                               Sema::CXXSpecialMember CSM) {
-  SmallVector<bool, 4> SatisfactionStatus;
-
-  for (CXXMethodDecl *Method : Methods) {
-    const Expr *Constraints = Method->getTrailingRequiresClause();
-    if (!Constraints)
-      SatisfactionStatus.push_back(true);
-    else {
-      ConstraintSatisfaction Satisfaction;
-      if (S.CheckFunctionConstraints(Method, Satisfaction))
-        SatisfactionStatus.push_back(false);
-      else
-        SatisfactionStatus.push_back(Satisfaction.IsSatisfied);
-    }
-  }
-
-  for (size_t i = 0; i < Methods.size(); i++) {
-    if (!SatisfactionStatus[i])
-      continue;
-    CXXMethodDecl *Method = Methods[i];
-    const Expr *Constraints = Method->getTrailingRequiresClause();
-    bool AnotherMethodIsMoreConstrained = false;
-    for (size_t j = 0; j < Methods.size(); j++) {
-      if (i == j || !SatisfactionStatus[j])
-        continue;
-      CXXMethodDecl *OtherMethod = Methods[j];
-      if (!AreSpecialMemberFunctionsSameKind(S.Context, Method, OtherMethod,
-                                             CSM))
-        continue;
-
-      const Expr *OtherConstraints = OtherMethod->getTrailingRequiresClause();
-      if (!OtherConstraints)
-        continue;
-      if (!Constraints) {
-        AnotherMethodIsMoreConstrained = true;
-        break;
-      }
-      if (S.IsAtLeastAsConstrained(OtherMethod, {OtherConstraints}, Method,
-                                   {Constraints},
-                                   AnotherMethodIsMoreConstrained)) {
-        // There was an error with the constraints comparison. Exit the loop
-        // and don't consider this function eligible.
-        AnotherMethodIsMoreConstrained = true;
-      }
-      if (AnotherMethodIsMoreConstrained)
-        break;
-    }
-    // FIXME: Do not consider deleted methods as eligible after implementing
-    // DR1734 and DR1496.
-    if (!AnotherMethodIsMoreConstrained) {
-      Method->setIneligibleOrNotSelected(false);
-      Record->addedEligibleSpecialMemberFunction(Method, 1 << CSM);
-    }
-  }
-}
-
-static void ComputeSpecialMemberFunctionsEligiblity(Sema &S,
-                                                    CXXRecordDecl *Record) {
-  SmallVector<CXXMethodDecl *, 4> DefaultConstructors;
-  SmallVector<CXXMethodDecl *, 4> CopyConstructors;
-  SmallVector<CXXMethodDecl *, 4> MoveConstructors;
-  SmallVector<CXXMethodDecl *, 4> CopyAssignmentOperators;
-  SmallVector<CXXMethodDecl *, 4> MoveAssignmentOperators;
-
-  for (auto *Decl : Record->decls()) {
-    auto *MD = dyn_cast<CXXMethodDecl>(Decl);
-    if (!MD)
-      continue;
-    if (auto *CD = dyn_cast<CXXConstructorDecl>(MD)) {
-      if (CD->isInvalidDecl())
-        continue;
-      if (CD->isDefaultConstructor())
-        DefaultConstructors.push_back(MD);
-      else if (CD->isCopyConstructor())
-        CopyConstructors.push_back(MD);
-      else if (CD->isMoveConstructor())
-        MoveConstructors.push_back(MD);
-    } else if (MD->isCopyAssignmentOperator()) {
-      CopyAssignmentOperators.push_back(MD);
-    } else if (MD->isMoveAssignmentOperator()) {
-      MoveAssignmentOperators.push_back(MD);
-    }
-  }
-
-  SetEligibleMethods(S, Record, DefaultConstructors,
-                     Sema::CXXDefaultConstructor);
-  SetEligibleMethods(S, Record, CopyConstructors, Sema::CXXCopyConstructor);
-  SetEligibleMethods(S, Record, MoveConstructors, Sema::CXXMoveConstructor);
-  SetEligibleMethods(S, Record, CopyAssignmentOperators,
-                     Sema::CXXCopyAssignment);
-  SetEligibleMethods(S, Record, MoveAssignmentOperators,
-                     Sema::CXXMoveAssignment);
-}
+} // namespace
 
 void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
                        ArrayRef<Decl *> Fields, SourceLocation LBrac,
@@ -18185,6 +18063,9 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
   RecordDecl *Record = dyn_cast<RecordDecl>(EnclosingDecl);
   CXXRecordDecl *CXXRecord = dyn_cast<CXXRecordDecl>(EnclosingDecl);
 
+  if (CXXRecord && !CXXRecord->isDependentType())
+    ComputeSelectedDestructor(*this, CXXRecord);
+
   // Start counting up the number of named members; make sure to include
   // members of anonymous structs and unions in the total.
   unsigned NumNamedMembers = 0;
@@ -18470,8 +18351,6 @@ void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
             Completed = true;
           }
         }
-        ComputeSelectedDestructor(*this, CXXRecord);
-        ComputeSpecialMemberFunctionsEligiblity(*this, CXXRecord);
       }
     }
 

clang/lib/Sema/SemaDeclCXX.cpp

@@ -6618,7 +6618,7 @@ static bool canPassInRegisters(Sema &S, CXXRecordDecl *D,
     bool CopyCtorIsTrivial = false, CopyCtorIsTrivialForCall = false;
     bool DtorIsTrivialForCall = false;
 
-    // If a class has at least one eligible, trivial copy constructor, it
+    // If a class has at least one non-deleted, trivial copy constructor, it
     // is passed according to the C ABI. Otherwise, it is passed indirectly.
     //
     // Note: This permits classes with non-trivial copy or move ctors to be
@@ -6633,8 +6633,7 @@ static bool canPassInRegisters(Sema &S, CXXRecordDecl *D,
       }
     } else {
       for (const CXXConstructorDecl *CD : D->ctors()) {
-        if (CD->isCopyConstructor() && !CD->isDeleted() &&
-            !CD->isIneligibleOrNotSelected()) {
+        if (CD->isCopyConstructor() && !CD->isDeleted()) {
           if (CD->isTrivial())
             CopyCtorIsTrivial = true;
           if (CD->isTrivialForCall())

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

@@ -2481,9 +2481,6 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
         Constructor->getConstexprKind(), InheritedConstructor(),
         TrailingRequiresClause);
     Method->setRangeEnd(Constructor->getEndLoc());
-    if (Constructor->isDefaultConstructor() ||
-        Constructor->isCopyOrMoveConstructor())
-      Method->setIneligibleOrNotSelected(true);
   } else if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(D)) {
     Method = CXXDestructorDecl::Create(
         SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo,
@@ -2506,8 +2503,6 @@ Decl *TemplateDeclInstantiator::VisitCXXMethodDecl(
         SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo, SC,
         D->UsesFPIntrin(), D->isInlineSpecified(), D->getConstexprKind(),
         D->getEndLoc(), TrailingRequiresClause);
-    if (D->isMoveAssignmentOperator() || D->isCopyAssignmentOperator())
-      Method->setIneligibleOrNotSelected(true);
   }
 
   if (D->isInlined())