From d6a637f8c8a93863509fc1bc555513ff6504957d Mon Sep 17 00:00:00 2001 From: Anders Carlsson Date: Mon, 7 Dec 2009 08:24:59 +0000 Subject: [PATCH] Rework how virtual member functions are marked. If a class has no key function, we now wait until the end of the translation unit to mark its virtual member functions as references. This lays the groundwork for fixing PR5557. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@90752 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/Sema.cpp | 33 +++++++---- lib/Sema/Sema.h | 22 ++++++- lib/Sema/SemaDecl.cpp | 13 +--- lib/Sema/SemaDeclCXX.cpp | 59 ++++++++++++++++--- lib/Sema/SemaExpr.cpp | 2 + .../virtual-member-functions-key-function.cpp | 22 +++++++ 6 files changed, 118 insertions(+), 33 deletions(-) create mode 100644 test/SemaCXX/virtual-member-functions-key-function.cpp diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index 29efbd7bec..ef6147420b 100644 --- a/lib/Sema/Sema.cpp +++ b/lib/Sema/Sema.cpp @@ -728,18 +728,27 @@ void Sema::DeleteStmt(StmtTy *S) { /// translation unit when EOF is reached and all but the top-level scope is /// popped. void Sema::ActOnEndOfTranslationUnit() { - // C++: Perform implicit template instantiations. - // - // FIXME: When we perform these implicit instantiations, we do not carefully - // keep track of the point of instantiation (C++ [temp.point]). This means - // that name lookup that occurs within the template instantiation will - // always happen at the end of the translation unit, so it will find - // some names that should not be found. Although this is common behavior - // for C++ compilers, it is technically wrong. In the future, we either need - // to be able to filter the results of name lookup or we need to perform - // template instantiations earlier. - PerformPendingImplicitInstantiations(); - + + while (1) { + // C++: Perform implicit template instantiations. + // + // FIXME: When we perform these implicit instantiations, we do not carefully + // keep track of the point of instantiation (C++ [temp.point]). This means + // that name lookup that occurs within the template instantiation will + // always happen at the end of the translation unit, so it will find + // some names that should not be found. Although this is common behavior + // for C++ compilers, it is technically wrong. In the future, we either need + // to be able to filter the results of name lookup or we need to perform + // template instantiations earlier. + PerformPendingImplicitInstantiations(); + + /// If ProcessPendingClassesWithUnmarkedVirtualMembers ends up marking + /// any virtual member functions it might lead to more pending template + /// instantiations, which is why we need to loop here. + if (!ProcessPendingClassesWithUnmarkedVirtualMembers()) + break; + } + // Check for #pragma weak identifiers that were never declared // FIXME: This will cause diagnostics to be emitted in a non-determinstic // order! Iterating over a densemap like this is bad. diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index e4ee8ce73c..edb4528f7e 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -32,6 +32,7 @@ #include "llvm/ADT/OwningPtr.h" #include #include +#include #include #include @@ -2147,11 +2148,26 @@ public: /// as referenced. void MarkBaseAndMemberDestructorsReferenced(CXXDestructorDecl *Destructor); - /// MaybeMarkVirtualImplicitMembersReferenced - If the passed in method is the + /// ClassesWithUnmarkedVirtualMembers - Contains record decls whose virtual + /// members might need to be marked as referenced. This is either done when + /// the key function definition is emitted (this is handled by by + /// MaybeMarkVirtualMembersReferenced), or at the end of the translation unit + /// (done by ProcessPendingClassesWithUnmarkedVirtualMembers). + std::map ClassesWithUnmarkedVirtualMembers; + + /// MaybeMarkVirtualMembersReferenced - If the passed in method is the /// key function of the record decl, will mark virtual member functions as /// referenced. - void MaybeMarkVirtualImplicitMembersReferenced(SourceLocation Loc, - CXXMethodDecl *MD); + void MaybeMarkVirtualMembersReferenced(SourceLocation Loc, CXXMethodDecl *MD); + + /// MarkVirtualMembersReferenced - Will mark all virtual members of the given + /// CXXRecordDecl referenced. + void MarkVirtualMembersReferenced(SourceLocation Loc, CXXRecordDecl *RD); + + /// ProcessPendingClassesWithUnmarkedVirtualMembers - Will process classes + /// that might need to have their virtual members marked as referenced. + /// Returns false if no work was done. + bool ProcessPendingClassesWithUnmarkedVirtualMembers(); void AddImplicitlyDeclaredMembersToClass(CXXRecordDecl *ClassDecl); diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index a5314bb075..bb3f869425 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -4114,16 +4114,9 @@ Sema::DeclPtrTy Sema::ActOnFinishFunctionBody(DeclPtrTy D, StmtArg BodyArg, if (!FD->isInvalidDecl()) DiagnoseUnusedParameters(FD->param_begin(), FD->param_end()); - if (CXXMethodDecl *Method = dyn_cast(FD)) { - // C++ [basic.def.odr]p2: - // [...] A virtual member function is used if it is not pure. [...] - if (Method->isVirtual() && !Method->isPure()) - MarkDeclarationReferenced(Method->getLocation(), Method); - - if (!Method->isInlined()) - MaybeMarkVirtualImplicitMembersReferenced(Method->getLocation(), - Method); - } + if (CXXMethodDecl *Method = dyn_cast(FD)) + MaybeMarkVirtualMembersReferenced(Method->getLocation(), Method); + assert(FD == getCurFunctionDecl() && "Function parsing confused"); } else if (ObjCMethodDecl *MD = dyn_cast_or_null(dcl)) { assert(MD == getCurMethodDecl() && "Method parsing confused"); diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 1e96bf525a..bd191ab8c9 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -3163,8 +3163,6 @@ void Sema::DefineImplicitDefaultConstructor(SourceLocation CurrentLocation, } else { Constructor->setUsed(); } - - MaybeMarkVirtualImplicitMembersReferenced(CurrentLocation, Constructor); } void Sema::DefineImplicitDestructor(SourceLocation CurrentLocation, @@ -5083,8 +5081,8 @@ Sema::ActOnCXXConditionDeclaration(Scope *S, Declarator &D) { return Dcl; } -void Sema::MaybeMarkVirtualImplicitMembersReferenced(SourceLocation Loc, - CXXMethodDecl *MD) { +void Sema::MaybeMarkVirtualMembersReferenced(SourceLocation Loc, + CXXMethodDecl *MD) { // Ignore dependent types. if (MD->isDependentContext()) return; @@ -5095,6 +5093,16 @@ void Sema::MaybeMarkVirtualImplicitMembersReferenced(SourceLocation Loc, if (!RD->isDynamicClass()) return; + if (!MD->isOutOfLine()) { + // The only inline functions we care about are constructors. We also defer + // marking the virtual members as referenced until we've reached the end + // of the translation unit. We do this because we need to know the key + // function of the class in order to determine the key function. + if (isa(MD)) + ClassesWithUnmarkedVirtualMembers.insert(std::make_pair(RD, Loc)); + return; + } + const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD); if (!KeyFunction) { @@ -5107,10 +5115,45 @@ void Sema::MaybeMarkVirtualImplicitMembersReferenced(SourceLocation Loc, return; } - if (CXXDestructorDecl *Dtor = RD->getDestructor(Context)) { - if (Dtor->isImplicit() && Dtor->isVirtual()) - MarkDeclarationReferenced(Loc, Dtor); + // Mark the members as referenced. + MarkVirtualMembersReferenced(Loc, RD); + ClassesWithUnmarkedVirtualMembers.erase(RD); +} + +bool Sema::ProcessPendingClassesWithUnmarkedVirtualMembers() { + if (ClassesWithUnmarkedVirtualMembers.empty()) + return false; + + for (std::map::iterator i = + ClassesWithUnmarkedVirtualMembers.begin(), + e = ClassesWithUnmarkedVirtualMembers.end(); i != e; ++i) { + CXXRecordDecl *RD = i->first; + + const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD); + if (KeyFunction) { + // We know that the class has a key function. If the key function was + // declared in this translation unit, then it the class decl would not + // have been in the ClassesWithUnmarkedVirtualMembers map. + continue; + } + + SourceLocation Loc = i->second; + MarkVirtualMembersReferenced(Loc, RD); } - // FIXME: Need to handle the virtual assignment operator here too. + ClassesWithUnmarkedVirtualMembers.clear(); + return true; } + +void Sema::MarkVirtualMembersReferenced(SourceLocation Loc, CXXRecordDecl *RD) { + for (CXXRecordDecl::method_iterator i = RD->method_begin(), + e = RD->method_end(); i != e; ++i) { + CXXMethodDecl *MD = *i; + + // C++ [basic.def.odr]p2: + // [...] A virtual member function is used if it is not pure. [...] + if (MD->isVirtual() && !MD->isPure()) + MarkDeclarationReferenced(Loc, MD); + } +} + diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 987de9b5b4..6c5a1ec05c 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -6927,6 +6927,8 @@ void Sema::MarkDeclarationReferenced(SourceLocation Loc, Decl *D) { if (!Constructor->isUsed()) DefineImplicitCopyConstructor(Loc, Constructor, TypeQuals); } + + MaybeMarkVirtualMembersReferenced(Loc, Constructor); } else if (CXXDestructorDecl *Destructor = dyn_cast(D)) { if (Destructor->isImplicit() && !Destructor->isUsed()) DefineImplicitDestructor(Loc, Destructor); diff --git a/test/SemaCXX/virtual-member-functions-key-function.cpp b/test/SemaCXX/virtual-member-functions-key-function.cpp new file mode 100644 index 0000000000..4e7ff69b2e --- /dev/null +++ b/test/SemaCXX/virtual-member-functions-key-function.cpp @@ -0,0 +1,22 @@ +// RUN: clang-cc -fsyntax-only -verify %s +struct A { + virtual ~A(); +}; + +struct B : A { // expected-error {{no suitable member 'operator delete' in 'B'}} + B() { } // expected-note {{implicit default destructor for 'struct B' first required here}} + void operator delete(void *, int); // expected-note {{'operator delete' declared here}} +}; + +struct C : A { // expected-error {{no suitable member 'operator delete' in 'C'}} + void operator delete(void *, int); // expected-note {{'operator delete' declared here}} +}; + +void f() { + // new B should mark the constructor as used, which then marks + // all the virtual members as used, because B has no key function. + (void)new B; + + // Same here, except that C has an implicit constructor. + (void)new C; // expected-note {{implicit default destructor for 'struct C' first required here}} +} -- 2.40.0