[Concepts] Implement overload resolution for destructors (P0848)

[royjacobson]

Summary

This patch implements a necessary part of P0848, the overload resolution for destructors.
It is now possible to overload destructors based on constraints, and the eligible destructor
will be selected at the end of the class.

The approach this patch takes is to perform the overload resolution in Sema::ActOnFields
and to mark the selected destructor using a new property in FunctionDeclBitfields.

CXXRecordDecl::getDestructor is then modified to use this property to return the correct
destructor.

This closes https://github.com/llvm/llvm-project/issues/45614.

[royjacobson]

Fix typo

[erichkeane]

How much of P0848 is missing after this? If nothing/not much, should we update cxx_status.html as well?

clang/lib/Sema/SemaTemplateInstantiate.cpp

2330

I don't think this ends up being acceptable (removing them from the AST). Instead, we should probably mark them as "invalid" and update getDestructor to only return the 'only' destructor, or the first not-invalid one.

[cor3ntin]

In D126194#3531267, @erichkeane wrote:

How much of P0848 is missing after this? If nothing/not much, should we update cxx_status.html as well?

P0848 applies to all special member function. At best we could mark it partial but most of the work still need to be done.
I gave a shot to P0848 a few months ago, but my assesment is that clang might have to significantly refactor of special member functions to do that cleanly

clang/lib/Sema/SemaTemplateInstantiate.cpp

2330

I'd rather we stay consistent with the wording, keep track of a selected destructor which would be returned by getDestructor. it's more surgery but it's a lot cleaner imo

[royjacobson]

In D126194#3531280, @cor3ntin wrote:

In D126194#3531267, @erichkeane wrote:

How much of P0848 is missing after this? If nothing/not much, should we update cxx_status.html as well?

P0848 applies to all special member function. At best we could mark it partial but most of the work still need to be done.
I gave a shot to P0848 a few months ago, but my assesment is that clang might have to significantly refactor of special member functions to do that cleanly

Corentin, are there other places like getDestructor where we need the constraints Sema information in the AST? I hoped the other special member functions go through LookupSpecialMember which does the needed overload resolution.

So as far as I understand the code base, the P0848 part that remains is computing the SMFs that are 'eligible' and update the type traits (trivial/trivially copyable) accordingly. Currently we mark those inside CXXRecordDecl::addedMember, so we'll probably have to override them. I also don't understand how exactly the eligibility checks interact with the deferred concept checking.

BTW, this also interacts funnily with other type traits. For example, this is apparently legal

#include <type_traits>
template<int N>
struct A {
  A& operator=(A&&) requires true;
  virtual A& operator=(A&&);
};
static_assert(!std::is_aggregate_v<A<1>>);

So ineligible SMFs are ineligible only for the purpose of [copy]triviality, and can still have other visible effects!

About the status page - we're going to break ABI when we implement the type traits change so I don't think we should update it yet.

clang/lib/Sema/SemaTemplateInstantiate.cpp

2330

Corentin, do you suggest doing this in CXXRecordDecl explicitly?

Erich - I agree, this seems like a better solution, although this is a bit of abuse to 'invalid' in the case of less-constrained candidates. Ideally we might want another AST property of 'eligible' and keep track of things there, but I don't really know the AST code well enough to do it.

[erichkeane]

clang/lib/Sema/SemaTemplateInstantiate.cpp

2330

I like the idea of marking them eligible that way. If this JUST has to do with Destructors for now, adding a bit to CXXDestructorDecl is a pretty trivial thing to do. I'm pretty sure it is defined in DeclCXX.h. At that point, we just need to make sure it is checked during 'getDestructor' (or at least, around any calls to that).

[royjacobson]

clang/lib/Sema/SemaTemplateInstantiate.cpp

2330

I think we'll need it soon for the other SMF as well. (Or maybe even for all member functions if CWG2421 is ever resolved).

So maybe I should just take some time to look at FunctionDeclBitfields and see if I can update it. (I hope updating the ast printers isn't too hard)

[erichkeane]

clang/lib/Sema/SemaTemplateInstantiate.cpp

2330

I'm pretty sure there is room in there. Updating the printers should be pretty easy as well.

[royjacobson]

Update the approach to use an AST property, and enable this for all language variants (not just CPP20).

There's still one test failing because OpenCL have got their own destructor thing going, I'll try to
see what I can do about it.

[erichkeane]

clang/lib/AST/DeclCXX.cpp

1915

What cases happen when the lookup for a destructor name ends up not being a destructor on a RecordDecl type? I guess I could see this happening with a pseudo-destructor, but that isn't a class type.

clang/lib/Sema/SemaDecl.cpp

17814

How does all of this play with the 'defaulted destructor is constexpr' stuff? We end up storing facts like that, destructor triviality/irrelevance/defaulted-and-constexpr/deleted/etc as a bit in the Decl, rather than calculating it. Can this feature (Allowing overloading) cause those to be inaccurate?

That is, could we do something like use a requires clause to select between a trivial, irrelevant, and constexpr destructor? Do we need to make sure we update those too? I would expect that the destructor here would need to store its OWN trivaility/relevance/constexpr/etc here, so we can then update those flags on the type once it is selected.

[royjacobson]

clang/lib/AST/DeclCXX.cpp

1915

It's a bit silly, but it's because we can have template destructors that we added to the AST even though they're invalid (it ends up as a FunctionTemplateDecl).

I noticed it because of SemaTemplate/destructor-template.cpp

clang/lib/Sema/SemaDecl.cpp

17814

You're right, I also found canPassInRegisters that will need to be adjusted a bit so it seems I need to write some codegen tests to make sure this gets the ABI correctly.

I won't have much time in the coming month, I hope to get to it by the start of July (but if someone wants to pick this up in the meantime feel free to do so).

[royjacobson]

Update the PR to now handle triviality, which turned out to require a different approach.
On the bright side it is now easier to see how to implemenet the rest of P0848.

Added an AST dump test that checks the bitfields of structs and some derived traits.

Also fixed the OpenCL test error - their destructor model is quite incompatible with clang
and already broken in some ways, so I tried my best to continue the current behaviour.

[erichkeane]

I probably need to spend more time on this at one point, but first look seemed fine to me. I think the approach is about right, and the solution is there.

@aaron.ballman is messing with the constexpr-ness of SM functions, so there is likely some collaboration that needs to be doen to make sure we get THIS right as well.

clang/docs/ReleaseNotes.rst

449

Do we have enough to update cxx_status.html?

clang/include/clang/AST/DeclBase.h

1603

Hrumph... its a shame to lose the bit here later. I wonder if at one point in a future patch we should figure out which of these flags can be moved to an 'allocated in Trailing-storage ONLY if required' type deal, like we just did with FunctionTypeBitFields...

clang/include/clang/AST/DeclCXX.h

1430

@aaron.ballman is messing around with addedMember recently... I wonder if there are OTHER decisions that need to be updated/moved here?

[royjacobson]

clang/docs/ReleaseNotes.rst

449

I think it's still partial enough that I don't want to communicate 'maybe it will work'.
I plan to finish the rest in time for Clang 15 so I prefer to just update it then :) But I don't feel very strongly about it either way.

clang/include/clang/AST/DeclCXX.h

1430

Do you know what papers/DRs he's working on? It seemed like most constexpr stuff is done per-function and not per-class.

I don't think there's something else that needs to change but it's definitely possible I missed something.. I went through addedMember and tried to remove everything destructor related basically.

[erichkeane]

clang/include/clang/AST/DeclCXX.h

1430

He's working on DR647, which does a lot of work with special member functions/constexpr/consteval constructors.

That said, I believe he said it isn't a problem, they are related, but not conflicting in a way that he had.

[aaron.ballman]

clang/include/clang/AST/DeclCXX.h

1430

I don't see anything here that should conflict with what I'm working on (this is mostly about destructors and I've not started touching those yet). I'm working specifically on implementing this bit: http://eel.is/c++draft/dcl.constexpr#7

[royjacobson]

Fix the AST test on windows and rebase on HEAD.

clang/docs/ReleaseNotes.rst

 
 - The mangling scheme for C++20 modules has incompatibly changed. The
   initial mangling was discovered not to be reversible, and the weak
   ownership design decision did not give the backwards compatibility
   that was hoped for. C++20 since added ``extern "C++"`` semantics
   that can be used for such compatibility. The demangler now demangles
   symbols with named module attachment.
 
+- As per "Conditionally Trivial Special Member Functions" (P0848), it is
+  now possible to overload destructors using concepts. Note that the rest
+  of the paper about other special member functions is not yet implemented.
+
 C++2b Feature Support
 ^^^^^^^^^^^^^^^^^^^^^
 
 - Implemented `P2128R6: Multidimensional subscript operator <https://wg21.link/P2128R6>`_.
 - Implemented `P0849R8: auto(x): decay-copy in the language <https://wg21.link/P0849R8>`_.
 - Implemented `P2242R3: Non-literal variables (and labels and gotos) in constexpr functions <https://wg21.link/P2242R3>`_.
 
 CUDA/HIP Language Changes in Clang

clang/include/clang/AST/Decl.h

public:
   bool isUserProvided() const {
     auto *DeclAsWritten = this;
     if (FunctionDecl *Pattern = getTemplateInstantiationPattern())
       DeclAsWritten = Pattern;
     return !(DeclAsWritten->isDeleted() ||
              DeclAsWritten->getCanonicalDecl()->isDefaulted());
   }
 
+  bool isIneligibleOrNotSelected() const {
+    return FunctionDeclBits.IsIneligibleOrNotSelected;
+  }
+  void setIneligibleOrNotSelected(bool II) {
+    FunctionDeclBits.IsIneligibleOrNotSelected = II;
+  }
+
   /// Whether falling off this function implicitly returns null/zero.
   /// If a more specific implicit return value is required, front-ends
   /// should synthesize the appropriate return statements.
   bool hasImplicitReturnZero() const {
     return FunctionDeclBits.HasImplicitReturnZero;
   }
 
   /// State that falling off this function implicitly returns null/zero.

clang/include/clang/AST/DeclBase.h

class FunctionDeclBitfields {
     /// This flag indicates whether this function is trivial for the purpose of
     /// calls. This is meaningful only when this function is a copy/move
     /// constructor or a destructor.
     uint64_t IsTrivialForCall : 1;
 
     uint64_t IsDefaulted : 1;
     uint64_t IsExplicitlyDefaulted : 1;
     uint64_t HasDefaultedFunctionInfo : 1;
+
+    /// For member functions of complete types, whether this is an ineligible
+    /// special member function or an unselected destructor. See
+    /// [class.mem.special].
+    uint64_t IsIneligibleOrNotSelected : 1;
+
     uint64_t HasImplicitReturnZero : 1;
     uint64_t IsLateTemplateParsed : 1;
 
     /// Kind of contexpr specifier as defined by ConstexprSpecKind.
     uint64_t ConstexprKind : 2;
     uint64_t InstantiationIsPending : 1;
 
     /// Indicates if the function uses __try.
class FunctionDeclBitfields {
     /// Store the ODRHash after first calculation.
     uint64_t HasODRHash : 1;
 
     /// Indicates if the function uses Floating Point Constrained Intrinsics
     uint64_t UsesFPIntrin : 1;
   };
 
   /// Number of non-inherited bits in FunctionDeclBitfields.
-  enum { NumFunctionDeclBits = 27 };
+  enum { NumFunctionDeclBits = 28 };
 
   /// Stores the bits used by CXXConstructorDecl. If modified
   /// NumCXXConstructorDeclBits and the accessor
   /// methods in CXXConstructorDecl should be updated appropriately.
   class CXXConstructorDeclBitfields {
     friend class CXXConstructorDecl;
     /// For the bits in DeclContextBitfields.
     uint64_t : NumDeclContextBits;
     /// For the bits in FunctionDeclBitfields.
     uint64_t : NumFunctionDeclBits;
 
-    /// 24 bits to fit in the remaining available space.
+    /// 23 bits to fit in the remaining available space.
     /// Note that this makes CXXConstructorDeclBitfields take
     /// exactly 64 bits and thus the width of NumCtorInitializers
     /// will need to be shrunk if some bit is added to NumDeclContextBitfields,
     /// NumFunctionDeclBitfields or CXXConstructorDeclBitfields.
-    uint64_t NumCtorInitializers : 21;
+    uint64_t NumCtorInitializers : 20;
     uint64_t IsInheritingConstructor : 1;
 
     /// Whether this constructor has a trail-allocated explicit specifier.
     uint64_t HasTrailingExplicitSpecifier : 1;
     /// If this constructor does't have a trail-allocated explicit specifier.
     /// Whether this constructor is explicit specified.
     uint64_t IsSimpleExplicit : 1;
   };

clang/include/clang/AST/DeclCXX.h

return (LangOpts.CPlusPlus20 ? hasConstexprDestructor()
             hasTrivialDefaultConstructor());
   }
 
   /// Determine whether this is a structural type.
   bool isStructural() const {
     return isLiteral() && data().StructuralIfLiteral;
   }
 
+  /// Notify the class that this destructor is now selected.
+  /// 
+  /// Important properties of the class depend on destructor properties. Since
+  /// C++20, it is possible to have multiple destructor declarations in a class
+  /// out of which one will be selected at the end.
+  /// This is called separately from addedMember because it has to be deferred
+  /// to the completion of the class.
+  void addedSelectedDestructor(CXXDestructorDecl *DD);
+
+  /// Notify the class that an eligible SMF has been added.
+  /// This updates triviality and destructor based properties of the class accordingly.
+  void addedEligibleSpecialMemberFunction(const CXXMethodDecl *MD, unsigned SMKind);
+
   /// If this record is an instantiation of a member class,
   /// retrieves the member class from which it was instantiated.
   ///
   /// This routine will return non-null for (non-templated) member
   /// classes of class templates. For example, given:
   ///
   /// \code
   /// template<typename T>

clang/include/clang/Basic/DiagnosticSemaKinds.td

def err_ovl_no_oper :
     Error<"type %0 does not provide a %select{subscript|call}1 operator">;
 def err_ovl_unresolvable : Error<
   "reference to %select{overloaded|multiversioned}1 function could not be "
   "resolved; did you mean to call it%select{| with no arguments}0?">;
 def err_bound_member_function : Error<
   "reference to non-static member function must be called"
   "%select{|; did you mean to call it with no arguments?}0">;
 def note_possible_target_of_call : Note<"possible target for call">;
+def err_no_viable_destructor : Error<
+  "no viable destructor found for class %0">;
+def err_ambiguous_destructor : Error<
+  "destructor of class %0 is ambiguous">;
 
 def err_ovl_no_viable_object_call : Error<
   "no matching function for call to object of type %0">;
 def err_ovl_ambiguous_object_call : Error<
   "call to object of type %0 is ambiguous">;
 def err_ovl_ambiguous_subscript_call : Error<
   "call to subscript operator of type %0 is ambiguous">;
 def err_ovl_deleted_object_call : Error<

clang/lib/AST/Decl.cpp

FunctionDecl::FunctionDecl(Kind DK, ASTContext &C, DeclContext *DC,
   FunctionDeclBits.HasInheritedPrototype = false;
   FunctionDeclBits.HasWrittenPrototype = true;
   FunctionDeclBits.IsDeleted = false;
   FunctionDeclBits.IsTrivial = false;
   FunctionDeclBits.IsTrivialForCall = false;
   FunctionDeclBits.IsDefaulted = false;
   FunctionDeclBits.IsExplicitlyDefaulted = false;
   FunctionDeclBits.HasDefaultedFunctionInfo = false;
+  FunctionDeclBits.IsIneligibleOrNotSelected = false;
   FunctionDeclBits.HasImplicitReturnZero = false;
   FunctionDeclBits.IsLateTemplateParsed = false;
   FunctionDeclBits.ConstexprKind = static_cast<uint64_t>(ConstexprKind);
   FunctionDeclBits.InstantiationIsPending = false;
   FunctionDeclBits.UsesSEHTry = false;
   FunctionDeclBits.UsesFPIntrin = UsesFPIntrin;
   FunctionDeclBits.HasSkippedBody = false;
   FunctionDeclBits.WillHaveBody = false;

clang/lib/AST/DeclCXX.cpp

if (const auto *Constructor = dyn_cast<CXXConstructorDecl>(DUnderlying)) {
     //   (possibly inherited from a base class) that is not a copy or move
     //   constructor [...]
     if (Constructor->isConstexpr() && !Constructor->isCopyOrMoveConstructor())
       data().HasConstexprNonCopyMoveConstructor = true;
     if (!isa<CXXConstructorDecl>(D) && Constructor->isDefaultConstructor())
       data().HasInheritedDefaultConstructor = true;
   }
 
-  // Handle destructors.
-  if (const auto *DD = dyn_cast<CXXDestructorDecl>(D)) {
-    SMKind |= SMF_Destructor;
-
-    if (DD->isUserProvided())
-      data().HasIrrelevantDestructor = false;
-    // If the destructor is explicitly defaulted and not trivial or not public
-    // or if the destructor is deleted, we clear HasIrrelevantDestructor in
-    // finishedDefaultedOrDeletedMember.
-
-    // C++11 [class.dtor]p5:
-    //   A destructor is trivial if [...] the destructor is not virtual.
-    if (DD->isVirtual()) {
-      data().HasTrivialSpecialMembers &= ~SMF_Destructor;
-      data().HasTrivialSpecialMembersForCall &= ~SMF_Destructor;
-    }
-
-    if (DD->isNoReturn())
-      data().IsAnyDestructorNoReturn = true;
-  }
-
   // Handle member functions.
   if (const auto *Method = dyn_cast<CXXMethodDecl>(D)) {
+    if (const auto *DD = dyn_cast<CXXDestructorDecl>(D))
+      SMKind |= SMF_Destructor;
+
     if (Method->isCopyAssignmentOperator()) {
       SMKind |= SMF_CopyAssignment;
 
       const auto *ParamTy =
           Method->getParamDecl(0)->getType()->getAs<ReferenceType>();
       if (!ParamTy || ParamTy->getPointeeType().isConstQualified())
         data().HasDeclaredCopyAssignmentWithConstParam = true;
     }
if (const auto *Method = dyn_cast<CXXMethodDecl>(D)) {
     if (SMKind) {
       // If this is the first declaration of a special member, we no longer have
       // an implicit trivial special member.
       data().HasTrivialSpecialMembers &=
           data().DeclaredSpecialMembers | ~SMKind;
       data().HasTrivialSpecialMembersForCall &=
           data().DeclaredSpecialMembers | ~SMKind;
 
-      if (!Method->isImplicit() && !Method->isUserProvided()) {
-        // This method is user-declared but not user-provided. We can't work out
-        // whether it's trivial yet (not until we get to the end of the class).
-        // We'll handle this method in finishedDefaultedOrDeletedMember.
-      } else if (Method->isTrivial()) {
-        data().HasTrivialSpecialMembers |= SMKind;
-        data().HasTrivialSpecialMembersForCall |= SMKind;
-      } else if (Method->isTrivialForCall()) {
-        data().HasTrivialSpecialMembersForCall |= SMKind;
-        data().DeclaredNonTrivialSpecialMembers |= SMKind;
-      } else {
-        data().DeclaredNonTrivialSpecialMembers |= SMKind;
-        // If this is a user-provided function, do not set
-        // DeclaredNonTrivialSpecialMembersForCall here since we don't know
-        // yet whether the method would be considered non-trivial for the
-        // purpose of calls (attribute "trivial_abi" can be dropped from the
-        // class later, which can change the special method's triviality).
-        if (!Method->isUserProvided())
-          data().DeclaredNonTrivialSpecialMembersForCall |= SMKind;
-      }
-
       // Note when we have declared a declared special member, and suppress the
       // implicit declaration of this special member.
       data().DeclaredSpecialMembers |= SMKind;
-
       if (!Method->isImplicit()) {
         data().UserDeclaredSpecialMembers |= SMKind;
 
         // C++03 [class]p4:
         //   A POD-struct is an aggregate class that has [...] no user-defined
         //   copy assignment operator and no user-defined destructor.
         //
         // Since the POD bit is meant to be C++03 POD-ness, and in C++03,
         // aggregates could not have any constructors, clear it even for an
         // explicitly defaulted or deleted constructor.
         // type is technically an aggregate in C++0x since it wouldn't be in 03.
         //
         // Also, a user-declared move assignment operator makes a class non-POD.
         // This is an extension in C++03.
         data().PlainOldData = false;
       }
+      // 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);
+      }
     }
 
     return;
   }
 
   // Handle non-static data members.
   if (const auto *Field = dyn_cast<FieldDecl>(D)) {
     ASTContext &Context = getASTContext();
if (Using->getDeclName().getNameKind() ==
       data().Aggregate = false;
     }
 
     if (Using->getDeclName().getCXXOverloadedOperator() == OO_Equal)
       data().HasInheritedAssignment = true;
   }
 }
 
+void CXXRecordDecl::addedSelectedDestructor(CXXDestructorDecl *DD) {
+  DD->setIneligibleOrNotSelected(false);
+  addedEligibleSpecialMemberFunction(DD, SMF_Destructor);
+}
+
+void CXXRecordDecl::addedEligibleSpecialMemberFunction(const CXXMethodDecl *MD,
+                                               unsigned SMKind) {
+  if (const auto *DD = dyn_cast<CXXDestructorDecl>(MD)) {
+    if (DD->isUserProvided())
+      data().HasIrrelevantDestructor = false;
+    // If the destructor is explicitly defaulted and not trivial or not public
+    // or if the destructor is deleted, we clear HasIrrelevantDestructor in
+    // finishedDefaultedOrDeletedMember.
+
+    // C++11 [class.dtor]p5:
+    //   A destructor is trivial if [...] the destructor is not virtual.
+    if (DD->isVirtual()) {
+      data().HasTrivialSpecialMembers &= ~SMF_Destructor;
+      data().HasTrivialSpecialMembersForCall &= ~SMF_Destructor;
+    }
+
+    if (DD->isNoReturn())
+      data().IsAnyDestructorNoReturn = true;
+  }
+
+  if (!MD->isImplicit() && !MD->isUserProvided()) {
+    // This method is user-declared but not user-provided. We can't work
+    // out whether it's trivial yet (not until we get to the end of the
+    // class). We'll handle this method in
+    // finishedDefaultedOrDeletedMember.
+  } else if (MD->isTrivial()) {
+    data().HasTrivialSpecialMembers |= SMKind;
+    data().HasTrivialSpecialMembersForCall |= SMKind;
+  } else if (MD->isTrivialForCall()) {
+    data().HasTrivialSpecialMembersForCall |= SMKind;
+    data().DeclaredNonTrivialSpecialMembers |= SMKind;
+  } else {
+    data().DeclaredNonTrivialSpecialMembers |= SMKind;
+    // If this is a user-provided function, do not set
+    // DeclaredNonTrivialSpecialMembersForCall here since we don't know
+    // yet whether the method would be considered non-trivial for the
+    // purpose of calls (attribute "trivial_abi" can be dropped from the
+    // class later, which can change the special method's triviality).
+    if (!MD->isUserProvided())
+      data().DeclaredNonTrivialSpecialMembersForCall |= SMKind;
+  }
+}
+
 void CXXRecordDecl::finishedDefaultedOrDeletedMember(CXXMethodDecl *D) {
   assert(!D->isImplicit() && !D->isUserProvided());
 
   // The kind of special member this declaration is, if any.
   unsigned SMKind = 0;
 
   if (const auto *Constructor = dyn_cast<CXXConstructorDecl>(D)) {
     if (Constructor->isDefaultConstructor()) {
CXXDestructorDecl *CXXRecordDecl::getDestructor() const {
   QualType ClassType = Context.getTypeDeclType(this);
 
   DeclarationName Name
     = Context.DeclarationNames.getCXXDestructorName(
                                           Context.getCanonicalType(ClassType));
 
   DeclContext::lookup_result R = lookup(Name);
 
-  return R.empty() ? nullptr : dyn_cast<CXXDestructorDecl>(R.front());
+  // If a destructor was marked as not selected, we skip it. We don't always
+  // have a selected destructor: dependent types, unnamed structs.
+  for (auto *Decl : R) {
+    auto* DD = dyn_cast<CXXDestructorDecl>(Decl);
+    if (DD && !DD->isIneligibleOrNotSelected())
+      return DD;
+  }
+  return nullptr;
 }
 
 static bool isDeclContextInNamespace(const DeclContext *DC) {
   while (!DC->isTranslationUnit()) {
     if (DC->isNamespace())
       return true;
     DC = DC->getParent();
   }

clang/lib/AST/TextNodeDumper.cpp

if (D->isDefaulted()) {
     if (D->isDeleted())
       OS << "_delete";
   }
   if (D->isDeletedAsWritten())
     OS << " delete";
   if (D->isTrivial())
     OS << " trivial";
 
+  if (D->isIneligibleOrNotSelected())
+    OS << (isa<CXXDestructorDecl>(D) ? " not_selected" : " ineligible");
+
   if (const auto *FPT = D->getType()->getAs<FunctionProtoType>()) {
     FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
     switch (EPI.ExceptionSpec.Type) {
     default:
       break;
     case EST_Unevaluated:
       OS << " noexcept-unevaluated " << EPI.ExceptionSpec.SourceDecl;
       break;

clang/lib/Sema/SemaDecl.cpp

if (Name.getNameKind() == DeclarationName::CXXConstructorName) {
     if (DC->isRecord()) {
       R = SemaRef.CheckDestructorDeclarator(D, R, SC);
       CXXRecordDecl *Record = cast<CXXRecordDecl>(DC);
       CXXDestructorDecl *NewDD = CXXDestructorDecl::Create(
           SemaRef.Context, Record, D.getBeginLoc(), NameInfo, R, TInfo,
           SemaRef.getCurFPFeatures().isFPConstrained(), isInline,
           /*isImplicitlyDeclared=*/false, ConstexprKind,
           TrailingRequiresClause);
+      // User defined destructors start as not selected if the class definition is still
+      // not done.
+      if (Record->isBeingDefined())
+        NewDD->setIneligibleOrNotSelected(true);
 
       // If the destructor needs an implicit exception specification, set it
       // now. FIXME: It'd be nice to be able to create the right type to start
       // with, but the type needs to reference the destructor declaration.
       if (SemaRef.getLangOpts().CPlusPlus11)
         SemaRef.AdjustDestructorExceptionSpec(NewDD);
 
       IsVirtualOkay = true;
Ivar = ObjCIvarDecl::Create(Context, cast<ObjCContainerDecl>(CurContext),
                               Context.CharTy,
                               Context.getTrivialTypeSourceInfo(Context.CharTy,
                                                                DeclLoc),
                               ObjCIvarDecl::Private, BW,
                               true);
   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
+///   an empty argument list to select the destructor for the class, also
+///   known as the selected destructor.
+///
+/// We do the overload resolution here, then mark the selected constructor in the AST.
+/// Later CXXRecordDecl::getDestructor() will return the selected constructor.
+void ComputeSelectedDestructor(Sema &S, CXXRecordDecl *Record) {
+  if (!Record->hasUserDeclaredDestructor()) {
+    return;
+  }
+
+  SourceLocation Loc = Record->getLocation();
+  OverloadCandidateSet OCS(Loc, OverloadCandidateSet::CSK_Normal);
+
+  for (auto *Decl : Record->decls()) {
+    if (auto *DD = dyn_cast<CXXDestructorDecl>(Decl)) {
+      if (DD->isInvalidDecl())
+        continue;
+      S.AddOverloadCandidate(DD, DeclAccessPair::make(DD, DD->getAccess()), {},
+                             OCS);
+      assert(DD->isIneligibleOrNotSelected() && "Selecting a destructor but a destructor was already selected.");
+    }
+  }
+
+  if (OCS.empty()) {
+    return;
+  }
+  OverloadCandidateSet::iterator Best;
+  unsigned Msg = 0;
+  OverloadCandidateDisplayKind DisplayKind;
+
+  switch (OCS.BestViableFunction(S, Loc, Best)) {
+  case OR_Success:
+  case OR_Deleted:
+    Record->addedSelectedDestructor(dyn_cast<CXXDestructorDecl>(Best->Function));
+    break;
+
+  case OR_Ambiguous:
+    Msg = diag::err_ambiguous_destructor;
+    DisplayKind = OCD_AmbiguousCandidates;
+    break;
+
+  case OR_No_Viable_Function:
+    Msg = diag::err_no_viable_destructor;
+    DisplayKind = OCD_AllCandidates;
+    break;
+  }
+
+  if (Msg) {
+    // OpenCL have got their own thing going with destructors. It's slightly broken,
+    // but we allow it.
+    if (!S.LangOpts.OpenCL) {
+      PartialDiagnostic Diag = S.PDiag(Msg) << Record;
+      OCS.NoteCandidates(PartialDiagnosticAt(Loc, Diag), S, DisplayKind, {});
+      Record->setInvalidDecl();
+    }
+    // It's a bit hacky: At this point we've raised an error but we want the
+    // rest of the compiler to continue somehow working. However almost
+    // everything we'll try to do with the class will depend on there being a
+    // destructor. So let's pretend the first one is selected and hope for the
+    // best.
+    Record->addedSelectedDestructor(dyn_cast<CXXDestructorDecl>(OCS.begin()->Function));
+  }
+}
+} // namespace
+
 void Sema::ActOnFields(Scope *S, SourceLocation RecLoc, Decl *EnclosingDecl,
                        ArrayRef<Decl *> Fields, SourceLocation LBrac,
                        SourceLocation RBrac,
                        const ParsedAttributesView &Attrs) {
   assert(EnclosingDecl && "missing record or interface decl");
 
   // If this is an Objective-C @implementation or category and we have
   // new fields here we should reset the layout of the interface since
case Decl::ObjCImplementation:
         ResetObjCLayout(cast<ObjCImplementationDecl>(DC)->getClassInterface());
       break;
     }
   }
 
   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;
   if (Record) {
     for (const auto *I : Record->decls()) {
       if (const auto *IFD = dyn_cast<IndirectFieldDecl>(I))
         if (IFD->getDeclName())
           ++NumNamedMembers;

clang/lib/Sema/SemaDeclCXX.cpp

if (S.getLangOpts().CPlusPlus11 && D->needsImplicitMoveConstructor() &&
     HasNonDeletedCopyOrMove = true;
   }
 
   if (D->needsImplicitDestructor() && !D->defaultedDestructorIsDeleted() &&
       !D->hasTrivialDestructorForCall())
     return false;
 
   for (const CXXMethodDecl *MD : D->methods()) {
-    if (MD->isDeleted())
+    if (MD->isDeleted() || MD->isIneligibleOrNotSelected())
       continue;
 
     auto *CD = dyn_cast<CXXConstructorDecl>(MD);
     if (CD && CD->isCopyOrMoveConstructor())
       HasNonDeletedCopyOrMove = true;
     else if (!isa<CXXDestructorDecl>(MD))
       continue;
 

clang/lib/Sema/SemaTemplateInstantiate.cpp

 
 } // namespace
 
 bool Sema::SubstTypeConstraint(
     TemplateTypeParmDecl *Inst, const TypeConstraint *TC,
     const MultiLevelTemplateArgumentList &TemplateArgs) {
   const ASTTemplateArgumentListInfo *TemplArgInfo =
       TC->getTemplateArgsAsWritten();
   TemplateArgumentListInfo InstArgs;
 
   if (TemplArgInfo) {
     InstArgs.setLAngleLoc(TemplArgInfo->LAngleLoc);
     InstArgs.setRAngleLoc(TemplArgInfo->RAngleLoc);
     if (SubstTemplateArguments(TemplArgInfo->arguments(), TemplateArgs,
                                InstArgs))
       return true;
   }
assert(
        (TSK == TSK_ImplicitInstantiation && Instantiation->isLocalClass())) &&
       "Unexpected template specialization kind!");
   for (auto *D : Instantiation->decls()) {
     bool SuppressNew = false;
     if (auto *Function = dyn_cast<FunctionDecl>(D)) {
       if (FunctionDecl *Pattern =
               Function->getInstantiatedFromMemberFunction()) {
 
+        if (Function->isIneligibleOrNotSelected())
+          continue;
+
         if (Function->getTrailingRequiresClause()) {
           ConstraintSatisfaction Satisfaction;
           if (CheckFunctionConstraints(Function, Satisfaction) ||
               !Satisfaction.IsSatisfied) {
             continue;
           }
         }
 

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

Method = CXXConstructorDecl::Create(
         Constructor->getConstexprKind(), InheritedConstructor(),
         TrailingRequiresClause);
     Method->setRangeEnd(Constructor->getEndLoc());
   } else if (CXXDestructorDecl *Destructor = dyn_cast<CXXDestructorDecl>(D)) {
     Method = CXXDestructorDecl::Create(
         SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo,
         Destructor->UsesFPIntrin(), Destructor->isInlineSpecified(), false,
         Destructor->getConstexprKind(), TrailingRequiresClause);
+    Method->setIneligibleOrNotSelected(true);
     Method->setRangeEnd(Destructor->getEndLoc());
     Method->setDeclName(SemaRef.Context.DeclarationNames.getCXXDestructorName(
         SemaRef.Context.getCanonicalType(
             SemaRef.Context.getTypeDeclType(Record))));
   } else if (CXXConversionDecl *Conversion = dyn_cast<CXXConversionDecl>(D)) {
     Method = CXXConversionDecl::Create(
         SemaRef.Context, Record, StartLoc, NameInfo, T, TInfo,
         Conversion->UsesFPIntrin(), Conversion->isInlineSpecified(),

clang/test/AST/ast-dump-decl.cpp

 // CHECK-NEXT:  |-TemplateTypeParmDecl 0x{{.+}} <col:12, col:21> col:21 typename depth 0 index 0 T
 // CHECK-NEXT:  |-CXXRecordDecl 0x{{.+}} <col:24, line:[[@LINE-36]]:3> line:[[@LINE-42]]:30 class TestClassTemplate definition
 // CHECK-NEXT:  | |-DefinitionData standard_layout has_user_declared_ctor can_const_default_init
 // CHECK-NEXT:  | | |-DefaultConstructor exists non_trivial user_provided
 // CHECK-NEXT:  | | |-CopyConstructor simple trivial has_const_param needs_implicit implicit_has_const_param
 // CHECK-NEXT:  | | |-MoveConstructor
 // CHECK-NEXT:  | | |-CopyAssignment simple trivial has_const_param needs_implicit implicit_has_const_param
 // CHECK-NEXT:  | | |-MoveAssignment
-// CHECK-NEXT:  | | `-Destructor non_trivial user_declared
+// CHECK-NEXT:  | | `-Destructor
 // CHECK-NEXT:  | |-CXXRecordDecl 0x{{.+}} <col:24, col:30> col:30 implicit referenced class TestClassTemplate
 // CHECK-NEXT:  | |-AccessSpecDecl 0x{{.+}} <line:[[@LINE-50]]:3, col:9> col:3 public
 // CHECK-NEXT:  | |-CXXConstructorDecl 0x{{.+}} <line:[[@LINE-50]]:5, col:23> col:5 TestClassTemplate<T> 'void ()'
 // CHECK-NEXT:  | |-CXXDestructorDecl 0x{{.+}} <line:[[@LINE-50]]:5, col:24> col:5 ~TestClassTemplate<T> 'void ()'
 // CHECK-NEXT:  | |-CXXMethodDecl 0x{{.+}} <line:[[@LINE-50]]:5, col:11> col:9 j 'int ()'
 // CHECK-NEXT:  | `-FieldDecl 0x{{.+}} <line:[[@LINE-50]]:5, col:9> col:9 i 'int'
 // CHECK-NEXT:  |-ClassTemplateSpecializationDecl 0x{{.+}} <line:[[@LINE-56]]:3, line:[[@LINE-50]]:3> line:[[@LINE-56]]:30 class TestClassTemplate definition
 // CHECK-NEXT:  | |-DefinitionData standard_layout has_user_declared_ctor can_const_default_init

clang/test/AST/overloaded-destructors.cpp

+// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-linux -ast-dump=json %s | FileCheck %s
+// RUN: %clang_cc1 -std=c++20 -triple x86_64-pc-win32 -ast-dump=json %s | FileCheck %s -check-prefixes=CHECK,WIN32
+
+// This test validates that we compute correct AST properties of classes after choosing
+// their destructor when doing destructor overload resolution with concepts.
+
+template <int N>
+struct A {
+  ~A() requires(N == 1) = default;
+  ~A() requires(N == 2) = delete;
+  ~A() requires(N == 3);
+  constexpr ~A() requires(N == 4);
+
+private:
+  ~A() requires(N == 5) = default;
+};
+
+
+template struct A<1>;
+// CHECK:             "kind": "ClassTemplateSpecializationDecl",
+// CHECK:             "definitionData": {
+// CHECK-NEXT:          "canConstDefaultInit": true,
+// CHECK-NEXT:          "canPassInRegisters": true,
+// CHECK-NEXT:          "copyAssign": {
+
+// CHECK:               "dtor": {
+// CHECK-NEXT:            "irrelevant": true,
+// CHECK-NEXT:            "trivial": true,
+// CHECK-NEXT:            "userDeclared": true
+// CHECK-NEXT:          },
+// CHECK-NEXT:          "hasConstexprNonCopyMoveConstructor": true,
+// CHECK-NEXT:          "isAggregate": true,
+// CHECK-NEXT:          "isEmpty": true,
+// CHECK-NEXT:          "isLiteral": true,
+// CHECK-NEXT:          "isStandardLayout": true,
+// CHECK-NEXT:          "isTrivial": true,
+// CHECK-NEXT:          "isTriviallyCopyable": true,
+// CHECK-NEXT:          "moveAssign": {},
+// CHECK-NEXT:          "moveCtor": {}
+
+template struct A<2>;
+// CHECK:             "kind": "ClassTemplateSpecializationDecl",
+// CHECK:             "definitionData": {
+// CHECK-NEXT:          "canConstDefaultInit": true,
+// CHECK-NEXT:          "canPassInRegisters": true,
+// CHECK-NEXT:          "copyAssign": {
+
+// CHECK:               "dtor": {
+// CHECK-NEXT:            "trivial": true,
+// CHECK-NEXT:            "userDeclared": true
+// CHECK-NEXT:          },
+// CHECK-NEXT:          "hasConstexprNonCopyMoveConstructor": true,
+// CHECK-NEXT:          "isAggregate": true,
+// CHECK-NEXT:          "isEmpty": true,
+// CHECK-NEXT:          "isStandardLayout": true,
+// CHECK-NEXT:          "isTrivial": true,
+// CHECK-NEXT:          "isTriviallyCopyable": true,
+// CHECK-NEXT:          "moveAssign": {},
+// CHECK-NEXT:          "moveCtor": {}
+
+template struct A<3>;
+// CHECK:             "kind": "ClassTemplateSpecializationDecl",
+// CHECK:             "definitionData": {
+// CHECK-NEXT:          "canConstDefaultInit": true,
+// WIN32-NEXT:          "canPassInRegisters": true,
+// CHECK-NEXT:          "copyAssign": {
+
+// CHECK:               "dtor": {
+// CHECK-NEXT:            "nonTrivial": true,
+// CHECK-NEXT:            "userDeclared": true
+// CHECK-NEXT:          },
+// CHECK-NEXT:          "hasConstexprNonCopyMoveConstructor": true,
+// CHECK-NEXT:          "isAggregate": true,
+// CHECK-NEXT:          "isEmpty": true,
+// CHECK-NEXT:          "isStandardLayout": true,
+// CHECK-NEXT:          "moveAssign": {},
+// CHECK-NEXT:          "moveCtor": {}
+
+template struct A<4>;
+// CHECK:             "kind": "ClassTemplateSpecializationDecl",
+// CHECK:             "definitionData": {
+// CHECK-NEXT:          "canConstDefaultInit": true,
+// WIN32-NEXT:          "canPassInRegisters": true,
+// CHECK-NEXT:          "copyAssign": {
+
+// CHECK:               "dtor": {
+// CHECK-NEXT:            "nonTrivial": true,
+// CHECK-NEXT:            "userDeclared": true
+// CHECK-NEXT:          },
+// CHECK-NEXT:          "hasConstexprNonCopyMoveConstructor": true,
+// CHECK-NEXT:          "isAggregate": true,
+// CHECK-NEXT:          "isEmpty": true,
+// CHECK-NEXT:          "isLiteral": true,
+// CHECK-NEXT:          "isStandardLayout": true,
+// CHECK-NEXT:          "moveAssign": {},
+// CHECK-NEXT:          "moveCtor": {}
+
+template struct A<5>;
+// CHECK:             "kind": "ClassTemplateSpecializationDecl",
+// CHECK:             "definitionData": {
+// CHECK-NEXT:          "canConstDefaultInit": true,
+// CHECK-NEXT:          "canPassInRegisters": true,
+// CHECK-NEXT:          "copyAssign": {
+
+// CHECK:               "dtor": {
+// CHECK-NEXT:            "trivial": true,
+// CHECK-NEXT:            "userDeclared": true
+// CHECK-NEXT:          },
+// CHECK-NEXT:          "hasConstexprNonCopyMoveConstructor": true,
+// CHECK-NEXT:          "isAggregate": true,
+// CHECK-NEXT:          "isEmpty": true,
+// CHECK-NEXT:          "isLiteral": true,
+// CHECK-NEXT:          "isStandardLayout": true,
+// CHECK-NEXT:          "isTrivial": true,
+// CHECK-NEXT:          "isTriviallyCopyable": true,
+// CHECK-NEXT:          "moveAssign": {},
+// CHECK-NEXT:          "moveCtor": {}
+

clang/test/CXX/class/class.dtor/p4.cpp

+// RUN: %clang_cc1 -std=c++20 -verify %s
+
+template <int N>
+struct A {
+  ~A() = delete;                  // expected-note {{explicitly marked deleted}}
+  ~A() requires(N == 1) = delete; // expected-note {{explicitly marked deleted}}
+};
+
+// FIXME: We should probably make it illegal to mix virtual and non-virtual methods
+// this way. See CWG2488 and some discussion in https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105699.
+template <int N>
+struct B {
+  ~B() requires(N == 1) = delete; // expected-note {{explicitly marked deleted}}
+  virtual ~B() = delete;          // expected-note {{explicitly marked deleted}}
+};
+
+template <int N>
+concept CO1 = N == 1;
+
+template <int N>
+concept CO2 = N >
+0;
+
+template <int N>
+struct C {
+  ~C() = delete; // expected-note {{explicitly marked deleted}}
+  ~C() requires(CO1<N>) = delete;
+  ~C() requires(CO1<N> &&CO2<N>) = delete; // expected-note {{explicitly marked deleted}}
+};
+
+template <int N>
+struct D {
+  ~D() requires(N != 0) = delete; // expected-note {{explicitly marked deleted}}
+  // expected-note@-1 {{candidate function has been explicitly deleted}}
+  // expected-note@-2 {{candidate function not viable: constraints not satisfied}}
+  // expected-note@-3 {{evaluated to false}}
+  ~D() requires(N == 1) = delete;
+  // expected-note@-1 {{candidate function has been explicitly deleted}}
+  // expected-note@-2 {{candidate function not viable: constraints not satisfied}}
+  // expected-note@-3 {{evaluated to false}}
+};
+
+template <class T>
+concept Foo = requires(T t) {
+  {t.foo()};
+};
+
+template <int N>
+struct E {
+  void foo();
+  ~E();
+  ~E() requires Foo<E> = delete; // expected-note {{explicitly marked deleted}}
+};
+
+template struct A<1>;
+template struct A<2>;
+template struct B<1>;
+template struct B<2>;
+template struct C<1>;
+template struct C<2>;
+template struct D<0>; // expected-error {{no viable destructor found for class 'D<0>'}} expected-note {{in instantiation of template}}
+template struct D<1>; // expected-error {{destructor of class 'D<1>' is ambiguous}} expected-note {{in instantiation of template}}
+template struct D<2>;
+template struct E<1>;
+
+int main() {
+  A<1> a1; // expected-error {{attempt to use a deleted function}}
+  A<2> a2; // expected-error {{attempt to use a deleted function}}
+  B<1> b1; // expected-error {{attempt to use a deleted function}}
+  B<2> b2; // expected-error {{attempt to use a deleted function}}
+  C<1> c1; // expected-error {{attempt to use a deleted function}}
+  C<2> c2; // expected-error {{attempt to use a deleted function}}
+  D<0> d0;
+  D<1> d1;
+  D<2> d2; // expected-error {{attempt to use a deleted function}}
+  E<1> e1; // expected-error {{attempt to use a deleted function}}
+}

clang/test/CXX/over/over.match/over.match.viable/p3.cpp

 
 template<typename T>
 struct S {
   void foo(int) requires false;
   void foo(A) requires true;
   S(A) requires false;
   S(double) requires true;
   ~S() requires false;
-  // expected-note@-1 2{{because 'false' evaluated to false}}
   ~S() requires true;
   operator int() requires true;
   operator int() requires false;
 };
 
 void bar() {
   WrapsStatics<int>::foo(A{});
   S<int>{1.}.foo(A{});
-  // expected-error@-1{{invalid reference to function '~S': constraints not satisfied}}
-  // Note - this behavior w.r.t. constrained dtors is a consequence of current
-  // wording, which does not invoke overload resolution when a dtor is called.
-  // P0848 is set to address this issue.
+
   S<int> s = 1;
-  // expected-error@-1{{invalid reference to function '~S': constraints not satisfied}}
   int a = s;
 }

clang/test/SemaTemplate/destructor-template.cpp

class PR33189
   ~PR33189() { } // expected-error{{destructor cannot be declared as a template}}
 };
 
 namespace PR38671 {
 struct S {
   template <class>
   ~S(); // expected-error{{destructor cannot be declared as a template}}
 };
-struct T : S {    // expected-note{{destructor of 'T' is implicitly deleted because base class 'PR38671::S' has no destructor}}
-  ~T() = default; // expected-warning{{explicitly defaulted destructor is implicitly deleted}}
+struct T : S {
+  ~T() = default;
 };
 } // namespace PR38671