rnk updated this revision to Diff 253933.
rnk added a comment.
- Remove definition data bit tracking, use destructor isUsed bit
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77081/new/
https://reviews.llvm.org/D77081
Files:
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/CXX/class.access/p4.cpp
clang/test/CodeGenCXX/microsoft-abi-vbase-dtor.cpp
clang/test/SemaCXX/ms-implicit-complete-dtor.cpp
Index: clang/test/SemaCXX/ms-implicit-complete-dtor.cpp
===================================================================
--- /dev/null
+++ clang/test/SemaCXX/ms-implicit-complete-dtor.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -std=c++17 -verify -Wno-defaulted-function-deleted %s -triple x86_64-windows-msvc
+
+// MSVC emits the complete destructor as if it were its own special member.
+// Clang attempts to do the same. This affects the diagnostics clang emits,
+// because deleting a type with a user declared constructor implicitly
+// references the destructors of virtual bases, which might be deleted or access
+// controlled.
+
+namespace t1 {
+struct A {
+ ~A() = delete; // expected-note {{deleted here}}
+};
+struct B { // expected-error {{attempt to use a deleted function}}
+ B() = default;
+ A o; // expected-note {{destructor of 'B' is implicitly deleted because field 'o' has a deleted destructor}}
+};
+struct C : virtual B {
+ ~C();
+};
+void delete1(C *p) { delete p; } // expected-note {{in implicit destructor for 't1::C' first required here}}
+void delete2(C *p) { delete p; }
+}
+
+namespace t2 {
+struct A {
+private:
+ ~A();
+};
+struct B { // expected-error {{attempt to use a deleted function}}
+ B() = default;
+ A o; // expected-note {{destructor of 'B' is implicitly deleted because field 'o' has an inaccessible destructor}}
+};
+struct C : virtual B {
+ ~C();
+};
+void useCompleteDtor(C *p) { delete p; } // expected-note {{in implicit destructor for 't2::C' first required here}}
+}
+
+namespace t3 {
+template <unsigned N>
+class Base { ~Base(); }; // expected-note 1{{declared private here}}
+// No diagnostic.
+class Derived0 : virtual Base<0> { ~Derived0(); };
+class Derived1 : virtual Base<1> {};
+// Emitting complete dtor causes a diagnostic.
+struct Derived2 : // expected-error {{inherited virtual base class 'Base<2>' has private destructor}}
+ virtual Base<2> {
+ ~Derived2();
+};
+void useCompleteDtor(Derived2 *p) { delete p; } // expected-note {{in implicit destructor for 't3::Derived2' first required here}}
+}
Index: clang/test/CodeGenCXX/microsoft-abi-vbase-dtor.cpp
===================================================================
--- /dev/null
+++ clang/test/CodeGenCXX/microsoft-abi-vbase-dtor.cpp
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -std=c++17 -emit-llvm %s -triple x86_64-windows-msvc -o - | FileCheck %s
+
+// Make sure virtual base base destructors get referenced and emitted if
+// necessary when the complete ("vbase") destructor is emitted. In this case,
+// clang previously did not emit ~DefaultedDtor.
+struct HasDtor { ~HasDtor(); };
+struct DefaultedDtor {
+ ~DefaultedDtor() = default;
+ HasDtor o;
+};
+struct HasCompleteDtor : virtual DefaultedDtor {
+ ~HasCompleteDtor();
+};
+void useCompleteDtor(HasCompleteDtor *p) { delete p; }
+
+// CHECK-LABEL: define dso_local void @"?useCompleteDtor@@YAXPEAUHasCompleteDtor@@@Z"(%struct.HasCompleteDtor* %p)
+// CHECK: call void @"??_DHasCompleteDtor@@QEAAXXZ"({{.*}})
+
+// CHECK-LABEL: define linkonce_odr dso_local void @"??_DHasCompleteDtor@@QEAAXXZ"(%struct.HasCompleteDtor* %this)
+// CHECK: call void @"??1HasCompleteDtor@@QEAA@XZ"({{.*}})
+// CHECK: call void @"??1DefaultedDtor@@QEAA@XZ"({{.*}})
+
+// CHECK-LABEL: define linkonce_odr dso_local void @"??1DefaultedDtor@@QEAA@XZ"(%struct.DefaultedDtor* %this)
+// CHECK: call void @"??1HasDtor@@QEAA@XZ"(%struct.HasDtor* %{{.*}})
+
Index: clang/test/CXX/class.access/p4.cpp
===================================================================
--- clang/test/CXX/class.access/p4.cpp
+++ clang/test/CXX/class.access/p4.cpp
@@ -1,6 +1,9 @@
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++98 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++11 %s
-// RUN: %clang_cc1 -fcxx-exceptions -fexceptions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++98 %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple %itanium_abi_triple -fcxx-exceptions -fexceptions -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-compatibility-version=19 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++98 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-compatibility-version=19 -fcxx-exceptions -fexceptions -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-compatibility-version=19 -fcxx-exceptions -fexceptions -fsyntax-only -verify %s
// C++0x [class.access]p4:
@@ -139,7 +142,7 @@
A local; // expected-error {{variable of type 'test3::A' has private destructor}}
}
-#if __cplusplus < 201103L
+#if __cplusplus < 201103L && !defined(_MSC_VER)
template <unsigned N> class Base { ~Base(); }; // expected-note 14 {{declared private here}}
class Base2 : virtual Base<2> { ~Base2(); }; // expected-note 3 {{declared private here}} \
// expected-error {{base class 'Base<2>' has private destructor}}
@@ -161,15 +164,43 @@
class Derived3 : // expected-error 2 {{inherited virtual base class 'Base<2>' has private destructor}} \
// expected-error 2 {{inherited virtual base class 'Base<3>' has private destructor}} \
- // expected-note 2{{implicit default constructor}}
+ // expected-note 2{{implicit default constructor}}
Base<0>, // expected-error 2 {{base class 'Base<0>' has private destructor}}
virtual Base<1>, // expected-error 2 {{base class 'Base<1>' has private destructor}}
Base2, // expected-error 2 {{base class 'test3::Base2' has private destructor}}
virtual Base3
- {};
- Derived3 d3; // expected-note 3{{implicit default constructor}}\
- // expected-note{{implicit destructor}}}
-#else
+ {};
+ Derived3 d3; // expected-note{{implicit destructor}}} \
+ // expected-note 3 {{implicit default constructor}}
+#elif __cplusplus < 201103L && defined(_MSC_VER)
+ template <unsigned N> class Base { ~Base(); }; // expected-note 14 {{declared private here}}
+ class Base2 : virtual Base<2> { ~Base2(); }; // expected-note 3 {{declared private here}} \
+ // expected-error {{base class 'Base<2>' has private destructor}}
+ class Base3 : virtual Base<3> { public: ~Base3(); }; // expected-error {{base class 'Base<3>' has private destructor}}
+
+ // These don't cause diagnostics because we don't need the destructor.
+ class Derived0 : Base<0> { ~Derived0(); };
+ class Derived1 : Base<1> { };
+
+ class Derived2 : // expected-error {{inherited virtual base class 'Base<2>' has private destructor}} \
+ // expected-error {{inherited virtual base class 'Base<3>' has private destructor}}
+ Base<0>, // expected-error {{base class 'Base<0>' has private destructor}}
+ virtual Base<1>, // expected-error {{base class 'Base<1>' has private destructor}}
+ Base2, // expected-error {{base class 'test3::Base2' has private destructor}}
+ virtual Base3
+ {
+ ~Derived2() {} // expected-note 2{{in implicit destructor}}
+ };
+
+ class Derived3 : // expected-error 2 {{inherited virtual base class 'Base<2>' has private destructor}} \
+ // expected-error 2 {{inherited virtual base class 'Base<3>' has private destructor}}
+ Base<0>, // expected-error 2 {{base class 'Base<0>' has private destructor}}
+ virtual Base<1>, // expected-error 2 {{base class 'Base<1>' has private destructor}}
+ Base2, // expected-error 2 {{base class 'test3::Base2' has private destructor}}
+ virtual Base3
+ {};
+ Derived3 d3; // expected-note{{implicit destructor}}} expected-note {{implicit default constructor}}
+#elif __cplusplus >= 201103L && !defined(_MSC_VER)
template <unsigned N> class Base { ~Base(); }; // expected-note 4{{declared private here}}
class Base2 : virtual Base<2> { ~Base2(); }; // expected-note 1{{declared private here}}
class Base3 : virtual Base<3> { public: ~Base3(); };
@@ -193,8 +224,40 @@
virtual Base<1>,
Base2,
virtual Base3
- {};
+ {};
Derived3 d3; // expected-error {{implicitly-deleted default constructor}}
+#elif __cplusplus >= 201103L && defined(_MSC_VER)
+ template <unsigned N> class Base { ~Base(); }; // expected-note 6{{declared private here}}
+ // expected-error@+1 {{inherited virtual base class 'Base<2>' has private destructor}}
+ class Base2 : virtual Base<2> { ~Base2(); }; // expected-note 1{{declared private here}}
+ // expected-error@+1 {{inherited virtual base class 'Base<3>' has private destructor}}
+ class Base3 : virtual Base<3> { public: ~Base3(); };
+
+ // These don't cause diagnostics because we don't need the destructor.
+ class Derived0 : Base<0> { ~Derived0(); };
+ class Derived1 : Base<1> { };
+
+ class Derived2 : // expected-error {{inherited virtual base class 'Base<2>' has private destructor}} \
+ // expected-error {{inherited virtual base class 'Base<3>' has private destructor}}
+ Base<0>, // expected-error {{base class 'Base<0>' has private destructor}}
+ virtual Base<1>, // expected-error {{base class 'Base<1>' has private destructor}}
+ Base2, // expected-error {{base class 'test3::Base2' has private destructor}}
+ virtual Base3
+ {
+ // expected-note@+2 {{in implicit destructor for 'test3::Base2' first required here}}
+ // expected-note@+1 {{in implicit destructor for 'test3::Base3' first required here}}
+ ~Derived2() {}
+ };
+
+ class Derived3 :
+ Base<0>, // expected-note {{deleted because base class 'Base<0>' has an inaccessible destructor}}
+ virtual Base<1>,
+ Base2,
+ virtual Base3
+ {};
+ Derived3 d3; // expected-error {{implicitly-deleted default constructor}}
+#else
+#error "missing case of MSVC cross C++ versions"
#endif
}
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -16113,6 +16113,20 @@
if (funcHasParameterSizeMangling(*this, Func))
CheckCompleteParameterTypesForMangler(*this, Func, Loc);
+ // In the MS C++ ABI, the compiler emits destructor variants where they are
+ // used. If the destructor is used here but defined elsewhere, mark the
+ // virtual base destructors referenced. If those virtual base destructors
+ // are inline, this will ensure they are defined when emitting the complete
+ // destructor variant. This checking may be redundant if the destructor is
+ // provided later in this TU.
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+ if (auto *Dtor = dyn_cast<CXXDestructorDecl>(Func)) {
+ CXXRecordDecl *Parent = Dtor->getParent();
+ if (Parent->getNumVBases() > 0 && !Dtor->getBody())
+ CheckCompleteDestructorVariant(Loc, Dtor);
+ }
+ }
+
Func->markUsed(Context);
}
}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -5443,6 +5443,15 @@
// subobjects.
bool VisitVirtualBases = !ClassDecl->isAbstract();
+ // If the destructor exists and has already been marked used in the MS ABI,
+ // then virtual base destructors have already been checked and marked used.
+ // Skip checking them again to avoid duplicate diagnostics.
+ if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+ CXXDestructorDecl *Dtor = ClassDecl->getDestructor();
+ if (Dtor && Dtor->isUsed())
+ VisitVirtualBases = false;
+ }
+
llvm::SmallPtrSet<const RecordType *, 8> DirectVirtualBases;
// Bases.
@@ -5477,16 +5486,21 @@
DiagnoseUseOfDecl(Dtor, Location);
}
- if (!VisitVirtualBases)
- return;
+ if (VisitVirtualBases)
+ MarkVirtualBaseDestructorsReferenced(Location, ClassDecl,
+ &DirectVirtualBases);
+}
+void Sema::MarkVirtualBaseDestructorsReferenced(
+ SourceLocation Loc, CXXRecordDecl *Record,
+ SmallPtrSetImpl<const RecordType *> *DirectVirtualBases) {
// Virtual bases.
for (const auto &VBase : ClassDecl->vbases()) {
// Bases are always records in a well-formed non-dependent class.
const RecordType *RT = VBase.getType()->castAs<RecordType>();
- // Ignore direct virtual bases.
- if (DirectVirtualBases.count(RT))
+ // Ignore already visited direct virtual bases.
+ if (DirectVirtualBases && DirectVirtualBases.count(RT))
continue;
CXXRecordDecl *BaseClassDecl = cast<CXXRecordDecl>(RT->getDecl());
@@ -13202,6 +13216,25 @@
}
}
+void Sema::CheckCompleteDestructorVariant(SourceLocation CurrentLocation,
+ CXXDestructorDecl *Destructor) {
+ if (Destructor->isInvalidDecl())
+ return;
+
+ CXXRecordDecl *ClassDecl = Destructor->getParent();
+ assert(Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+ "implicit complete dtors unneeded outside MS ABI");
+ assert(ClassDecl->getNumVBases() > 0 &&
+ "complete dtor only exists for classes with vbases");
+
+ SynthesizedFunctionScope Scope(*this, Destructor);
+
+ // Add a context note for diagnostics produced after this point.
+ Scope.addContextNote(CurrentLocation);
+
+ MarkVirtualBaseDestructorsReferenced(CurrentLocation, ClassDecl);
+}
+
/// Perform any semantic analysis which needs to be delayed until all
/// pending class member declarations have been parsed.
void Sema::ActOnFinishCXXMemberDecls() {
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -6649,6 +6649,18 @@
void MarkBaseAndMemberDestructorsReferenced(SourceLocation Loc,
CXXRecordDecl *Record);
+ void MarkVirtualBaseDestructorsReferenced(
+ SourceLocation Loc, CXXRecordDecl *Record,
+ SmallPtrSetImpl<const RecordType *> *DirectVirtualBases = nullptr);
+
+ /// Mark destructors of virtual bases of the parent of this destructor
+ /// referenced. This is the minimum amount of semantic checking necessary to
+ /// emit the complete destructor variant by itself, which is done in the MS
+ /// C++ ABI. This is a subset of the checks done by DefineImplicitDestructor
+ /// and MarkBaseAndMemberDestructorsReferenced.
+ void CheckCompleteDestructorVariant(SourceLocation CurrentLocation,
+ CXXDestructorDecl *Dtor);
+
/// The list of classes whose vtables have been used within
/// this translation unit, and the source locations at which the
/// first use occurred.
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
