royjacobson created this revision.
Herald added a project: All.
royjacobson requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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 after
instantiating the members
in Sema::InstantiateClass, and to remove from the CXXRecordDecl all
declarations of non-selected
destructors.
This is the approach that I found that can satisfy the following requirements:
1. The overload resolution and constraint checking is performed after the other
members are defined but before the type is complete. This is important because
constraints can check other members of the class, but the result of the
overload resolution can change the type traits (triviallity etc.) of the class.
2. The order of (visible) functions in the AST is kept. This is important
because it affects ABI through the order of functions in the vtable.
Note: this is only enabled for CPP20 - I tested it without the CPP20
restriction and
all the tests passed, so I can remove the restriction. I slightly prefer to
remove it because
it will be easier to get coverage for this and it seems safer.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126194
Files:
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/test/CXX/class/class.dtor/p4.cpp
clang/test/CXX/over/over.match/over.match.viable/p3.cpp
Index: clang/test/CXX/over/over.match/over.match.viable/p3.cpp
===================================================================
--- clang/test/CXX/over/over.match/over.match.viable/p3.cpp
+++ clang/test/CXX/over/over.match/over.match.viable/p3.cpp
@@ -49,7 +49,6 @@
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;
@@ -58,11 +57,7 @@
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;
}
Index: clang/test/CXX/class/class.dtor/p4.cpp
===================================================================
--- /dev/null
+++ clang/test/CXX/class/class.dtor/p4.cpp
@@ -0,0 +1,74 @@
+// 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}}
+};
+
+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}}
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2320,6 +2320,80 @@
}
};
+ /// [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.
+ ///
+ /// To achieve that, we remove all non-selected destructors from the AST,
+ /// which is a bit unusual. We can't let those declarations be in AST and rely
+ /// on LookupSpecialMember to return the correct declaration because a lot of
+ /// code relies on CXXRecordDecl::getDestructor() which assumes there can only
+ /// be one destructor for a class.
+ /// The reason that we actually have to insert those declarations in the first
+ /// place (instead of keeping them at the side and adding the selected one
+ /// later) is that this will change the order of declarations in the class
+ /// which determines the vtable order, so this would break ABI unless we
+ /// insert the destructor at the original location, at which point it is
+ /// easier to just remove the unselected ones.
+ LLVM_ATTRIBUTE_NOINLINE // Noinline e're sensitive about the stack size of `InstantiateClass`
+ void ComputeSelectedDestructor(Sema &S, CXXRecordDecl *Instantiation) {
+
+ if (!Instantiation->hasUserDeclaredDestructor())
+ return;
+
+ // FIXME: Is this the correct location?
+ SourceLocation Loc = Instantiation->getLocation();
+ OverloadCandidateSet OCS(Loc, OverloadCandidateSet::CSK_Normal);
+
+ SmallVector<CXXDestructorDecl *, 4> ProspectiveDestructors;
+ for (auto *Decl : Instantiation->decls()) {
+ if (auto *DD = dyn_cast<CXXDestructorDecl>(Decl)) {
+ if (DD->isInvalidDecl())
+ continue;
+ ProspectiveDestructors.push_back(DD);
+ S.AddOverloadCandidate(DD, DeclAccessPair::make(DD, DD->getAccess()),
+ {}, OCS);
+ }
+ }
+ assert(
+ !OCS.empty() &&
+ "No candidates found even though hasUserDeclaredDestructor is true.");
+
+ OverloadCandidateSet::iterator Best;
+ unsigned Msg = 0;
+ OverloadCandidateDisplayKind DisplayKind;
+
+ switch (OCS.BestViableFunction(S, Loc, Best)) {
+ case OR_Success:
+ case OR_Deleted:
+ for (auto *DD : ProspectiveDestructors) {
+ if (DD != Best->Function) {
+ Instantiation->removeDecl(DD);
+ // FIXME: We should maybe `delete DD` here, but it segfaults.
+ }
+ }
+ 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) {
+ PartialDiagnostic Diag = S.PDiag(Msg) << Instantiation;
+ OCS.NoteCandidates(PartialDiagnosticAt(Loc, Diag), S, DisplayKind, {});
+ Instantiation->setInvalidDecl();
+ }
+ }
+
} // namespace
bool Sema::SubstTypeConstraint(
@@ -2789,6 +2863,9 @@
}
}
+ if (getLangOpts().CPlusPlus20)
+ ComputeSelectedDestructor(*this, Instantiation);
+
// Finish checking fields.
ActOnFields(nullptr, Instantiation->getLocation(), Instantiation, Fields,
SourceLocation(), SourceLocation(), ParsedAttributesView());
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4715,6 +4715,10 @@
"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">;
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -400,6 +400,11 @@
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
+ type traits support for trivial and trivially copyable types is not
+ yet implemented.
+
C++2b Feature Support
^^^^^^^^^^^^^^^^^^^^^
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
