rnk updated this revision to Diff 253715.
rnk added a comment.
- add def data bit
- add tests
- fix test
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77081/new/
https://reviews.llvm.org/D77081
Files:
clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
clang/include/clang/AST/DeclCXX.h
clang/include/clang/Sema/Sema.h
clang/lib/AST/DeclCXX.cpp
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,6 @@
-// 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
// C++0x [class.access]p4:
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15998,10 +15998,20 @@
} else if (CXXDestructorDecl *Destructor =
dyn_cast<CXXDestructorDecl>(Func)) {
Destructor = cast<CXXDestructorDecl>(Destructor->getFirstDecl());
- if (Destructor->isDefaulted() && !Destructor->isDeleted()) {
- if (Destructor->isTrivial() && !Destructor->hasAttr<DLLExportAttr>())
- return;
- DefineImplicitDestructor(Loc, Destructor);
+ if (!Destructor->isDeleted()) {
+ if (Destructor->isDefaulted()) {
+ if (Destructor->isTrivial() &&
+ !Destructor->hasAttr<DLLExportAttr>())
+ return;
+ DefineImplicitDestructor(Loc, Destructor);
+ } else if (Context.getTargetInfo().getCXXABI().isMicrosoft()) {
+ // In the MS ABI, the complete destructor is implicitly defined,
+ // even if the base destructor is user defined.
+ CXXRecordDecl *Parent = Destructor->getParent();
+ if (Parent->getNumVBases() > 0 &&
+ !Parent->definedImplicitCompleteDestructor())
+ DefineImplicitCompleteDestructor(Loc, Destructor);
+ }
}
if (Destructor->isVirtual() && getLangOpts().AppleKext)
MarkVTableUsed(Loc, Destructor->getParent());
Index: clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -13202,6 +13202,55 @@
}
}
+void Sema::DefineImplicitCompleteDestructor(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);
+
+ // No need to resolve the exception specification of the base destructor or
+ // mark vtables referenced. That will be done when the base dtor is defined.
+
+ // Similar to what is done above in MarkBaseAndMemberDestructorsReferenced,
+ // but only for virtual bases.
+ for (const CXXBaseSpecifier &VBase : ClassDecl->vbases()) {
+ // Bases are always records in a well-formed non-dependent class.
+ CXXRecordDecl *BaseRD = VBase.getType()->getAsCXXRecordDecl();
+
+ // If our base class is invalid, we probably can't get its dtor anyway.
+ if (!BaseRD || BaseRD->isInvalidDecl() || BaseRD->hasIrrelevantDestructor())
+ continue;
+
+ CXXDestructorDecl *Dtor = LookupDestructor(BaseRD);
+ assert(Dtor && "No dtor found for BaseRD!");
+ if (CheckDestructorAccess(
+ ClassDecl->getLocation(), Dtor,
+ PDiag(diag::err_access_dtor_vbase)
+ << Context.getTypeDeclType(ClassDecl) << VBase.getType(),
+ Context.getTypeDeclType(ClassDecl)) == AR_accessible) {
+ CheckDerivedToBaseConversion(Context.getTypeDeclType(ClassDecl),
+ VBase.getType(), diag::err_access_dtor_vbase,
+ 0, ClassDecl->getLocation(), SourceRange(),
+ DeclarationName(), nullptr);
+ }
+
+ MarkFunctionReferenced(Dtor->getLocation(), Dtor);
+ DiagnoseUseOfDecl(Dtor, Dtor->getLocation());
+ }
+
+ ClassDecl->setDefinedImplicitDestructor();
+}
+
/// Perform any semantic analysis which needs to be delayed until all
/// pending class member declarations have been parsed.
void Sema::ActOnFinishCXXMemberDecls() {
Index: clang/lib/AST/DeclCXX.cpp
===================================================================
--- clang/lib/AST/DeclCXX.cpp
+++ clang/lib/AST/DeclCXX.cpp
@@ -103,7 +103,8 @@
ImplicitCopyConstructorCanHaveConstParamForNonVBase(true),
ImplicitCopyAssignmentHasConstParam(true),
HasDeclaredCopyConstructorWithConstParam(false),
- HasDeclaredCopyAssignmentWithConstParam(false), IsLambda(false),
+ HasDeclaredCopyAssignmentWithConstParam(false),
+ DefinedImplicitCompleteDestructor(false), IsLambda(false),
IsParsingBaseSpecifiers(false), ComputedVisibleConversions(false),
HasODRHash(false), Definition(D) {}
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -5559,6 +5559,11 @@
void DefineImplicitDestructor(SourceLocation CurrentLocation,
CXXDestructorDecl *Destructor);
+ /// Define an implicit complete constructor for classes with vbases in the MS
+ /// ABI.
+ void DefineImplicitCompleteDestructor(SourceLocation CurrentLocation,
+ CXXDestructorDecl *Dtor);
+
/// Build an exception spec for destructors that don't have one.
///
/// C++11 says that user-defined destructors with no exception spec get one
Index: clang/include/clang/AST/DeclCXX.h
===================================================================
--- clang/include/clang/AST/DeclCXX.h
+++ clang/include/clang/AST/DeclCXX.h
@@ -956,6 +956,18 @@
return data().NeedOverloadResolutionForDestructor;
}
+ /// Indicates if the complete destructor has been implicitly declared
+ /// yet. Only relevant in the Microsoft C++.
+ bool definedImplicitCompleteDestructor() const {
+ return data().DefinedImplicitCompleteDestructor;
+ }
+
+ void setDefinedImplicitDestructor() {
+ assert(!data().DefinedImplicitCompleteDestructor &&
+ "defining complete dtor more than once");
+ data().DefinedImplicitCompleteDestructor = true;
+ }
+
/// Determine whether this class describes a lambda function object.
bool isLambda() const {
// An update record can't turn a non-lambda into a lambda.
Index: clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
===================================================================
--- clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
+++ clang/include/clang/AST/CXXRecordDeclDefinitionBits.def
@@ -233,4 +233,8 @@
/// const-qualified reference parameter or a non-reference parameter.
FIELD(HasDeclaredCopyAssignmentWithConstParam, 1, MERGE_OR)
+/// Indicates if the complete destructor has been implicitly declared
+/// yet. Only relevant in the Microsoft C++.
+FIELD(DefinedImplicitCompleteDestructor, 1, MERGE_OR)
+
#undef FIELD
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
