From: Benjamin Kramer Date: Sat, 25 Jul 2015 15:07:25 +0000 (+0000) Subject: [AST] Turn the callbacks of lookupInBases and forallBases into a function_ref X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=af49942c7cf06570b90b206a51b828d191465e3f;p=clang [AST] Turn the callbacks of lookupInBases and forallBases into a function_ref This lets us pass functors (and lambdas) without void * tricks. On the downside we can't pass CXXRecordDecl's Find* members (which are now type safe) to lookupInBases directly, but a lambda trampoline is a small price to pay. No functionality change intended. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@243217 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/AST/CXXInheritance.h b/include/clang/AST/CXXInheritance.h index f7612f21f2..008efb3647 100644 --- a/include/clang/AST/CXXInheritance.h +++ b/include/clang/AST/CXXInheritance.h @@ -162,10 +162,9 @@ class CXXBasePaths { void ComputeDeclsFound(); - bool lookupInBases(ASTContext &Context, - const CXXRecordDecl *Record, - CXXRecordDecl::BaseMatchesCallback *BaseMatches, - void *UserData); + bool lookupInBases(ASTContext &Context, const CXXRecordDecl *Record, + CXXRecordDecl::BaseMatchesCallback BaseMatches); + public: typedef std::list::iterator paths_iterator; typedef std::list::const_iterator const_paths_iterator; diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index 08451c051b..87e3803a66 100644 --- a/include/clang/AST/DeclCXX.h +++ b/include/clang/AST/DeclCXX.h @@ -1468,8 +1468,8 @@ public: /// \param BaseDefinition the definition of the base class /// /// \returns true if this base matched the search criteria - typedef bool ForallBasesCallback(const CXXRecordDecl *BaseDefinition, - void *UserData); + typedef llvm::function_ref + ForallBasesCallback; /// \brief Determines if the given callback holds for all the direct /// or indirect base classes of this type. @@ -1481,13 +1481,10 @@ public: /// class of this type, or if \p AllowShortCircuit is true then until a call /// returns false. /// - /// \param UserData Passed as the second argument of every call to - /// \p BaseMatches. - /// /// \param AllowShortCircuit if false, forces the callback to be called /// for every base class, even if a dependent or non-matching base was /// found. - bool forallBases(ForallBasesCallback *BaseMatches, void *UserData, + bool forallBases(ForallBasesCallback BaseMatches, bool AllowShortCircuit = true) const; /// \brief Function type used by lookupInBases() to determine whether a @@ -1499,13 +1496,9 @@ public: /// \param Path the current path, from the most-derived class down to the /// base named by the \p Specifier. /// - /// \param UserData a single pointer to user-specified data, provided to - /// lookupInBases(). - /// /// \returns true if this base matched the search criteria, false otherwise. - typedef bool BaseMatchesCallback(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, - void *UserData); + typedef llvm::function_ref BaseMatchesCallback; /// \brief Look for entities within the base classes of this C++ class, /// transitively searching all base class subobjects. @@ -1520,14 +1513,12 @@ public: /// \param BaseMatches callback function used to determine whether a given /// base matches the user-defined search criteria. /// - /// \param UserData user data pointer that will be provided to \p BaseMatches. - /// /// \param Paths used to record the paths from this class to its base class /// subobjects that match the search criteria. /// /// \returns true if there exists any path from this class to a base class /// subobject that matches the search criteria. - bool lookupInBases(BaseMatchesCallback *BaseMatches, void *UserData, + bool lookupInBases(BaseMatchesCallback BaseMatches, CXXBasePaths &Paths) const; /// \brief Base-class lookup callback that determines whether the given @@ -1535,10 +1526,10 @@ public: /// /// This callback can be used with \c lookupInBases() to determine whether /// a given derived class has is a base class subobject of a particular type. - /// The user data pointer should refer to the canonical CXXRecordDecl of the + /// The base record pointer should refer to the canonical CXXRecordDecl of the /// base class that we are searching for. static bool FindBaseClass(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, void *BaseRecord); + CXXBasePath &Path, const CXXRecordDecl *BaseRecord); /// \brief Base-class lookup callback that determines whether the /// given base class specifier refers to a specific class @@ -1546,39 +1537,38 @@ public: /// /// This callback can be used with \c lookupInBases() to determine /// whether a given derived class has is a virtual base class - /// subobject of a particular type. The user data pointer should + /// subobject of a particular type. The base record pointer should /// refer to the canonical CXXRecordDecl of the base class that we /// are searching for. static bool FindVirtualBaseClass(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, void *BaseRecord); + CXXBasePath &Path, + const CXXRecordDecl *BaseRecord); /// \brief Base-class lookup callback that determines whether there exists /// a tag with the given name. /// /// This callback can be used with \c lookupInBases() to find tag members - /// of the given name within a C++ class hierarchy. The user data pointer - /// is an opaque \c DeclarationName pointer. + /// of the given name within a C++ class hierarchy. static bool FindTagMember(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, void *Name); + CXXBasePath &Path, DeclarationName Name); /// \brief Base-class lookup callback that determines whether there exists /// a member with the given name. /// /// This callback can be used with \c lookupInBases() to find members - /// of the given name within a C++ class hierarchy. The user data pointer - /// is an opaque \c DeclarationName pointer. + /// of the given name within a C++ class hierarchy. static bool FindOrdinaryMember(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, void *Name); + CXXBasePath &Path, DeclarationName Name); /// \brief Base-class lookup callback that determines whether there exists /// a member with the given name that can be used in a nested-name-specifier. /// - /// This callback can be used with \c lookupInBases() to find membes of + /// This callback can be used with \c lookupInBases() to find members of /// the given name within a C++ class hierarchy that can occur within /// nested-name-specifiers. static bool FindNestedNameSpecifierMember(const CXXBaseSpecifier *Specifier, CXXBasePath &Path, - void *UserData); + DeclarationName Name); /// \brief Retrieve the final overriders for each virtual member /// function in the class hierarchy where this class is the diff --git a/lib/AST/CXXInheritance.cpp b/lib/AST/CXXInheritance.cpp index 800c8f83b8..c8529196c8 100644 --- a/lib/AST/CXXInheritance.cpp +++ b/lib/AST/CXXInheritance.cpp @@ -85,9 +85,13 @@ bool CXXRecordDecl::isDerivedFrom(const CXXRecordDecl *Base, return false; Paths.setOrigin(const_cast(this)); - return lookupInBases(&FindBaseClass, - const_cast(Base->getCanonicalDecl()), - Paths); + + const CXXRecordDecl *BaseDecl = Base->getCanonicalDecl(); + return lookupInBases( + [BaseDecl](const CXXBaseSpecifier *Specifier, CXXBasePath &Path) { + return FindBaseClass(Specifier, Path, BaseDecl); + }, + Paths); } bool CXXRecordDecl::isVirtuallyDerivedFrom(const CXXRecordDecl *Base) const { @@ -102,20 +106,19 @@ bool CXXRecordDecl::isVirtuallyDerivedFrom(const CXXRecordDecl *Base) const { Paths.setOrigin(const_cast(this)); - const void *BasePtr = static_cast(Base->getCanonicalDecl()); - return lookupInBases(&FindVirtualBaseClass, - const_cast(BasePtr), - Paths); -} - -static bool BaseIsNot(const CXXRecordDecl *Base, void *OpaqueTarget) { - // OpaqueTarget is a CXXRecordDecl*. - return Base->getCanonicalDecl() != (const CXXRecordDecl*) OpaqueTarget; + const CXXRecordDecl *BaseDecl = Base->getCanonicalDecl(); + return lookupInBases( + [BaseDecl](const CXXBaseSpecifier *Specifier, CXXBasePath &Path) { + return FindVirtualBaseClass(Specifier, Path, BaseDecl); + }, + Paths); } bool CXXRecordDecl::isProvablyNotDerivedFrom(const CXXRecordDecl *Base) const { - return forallBases(BaseIsNot, - const_cast(Base->getCanonicalDecl())); + const CXXRecordDecl *TargetDecl = Base->getCanonicalDecl(); + return forallBases([TargetDecl](const CXXRecordDecl *Base) { + return Base->getCanonicalDecl() != TargetDecl; + }); } bool @@ -129,8 +132,7 @@ CXXRecordDecl::isCurrentInstantiation(const DeclContext *CurContext) const { return false; } -bool CXXRecordDecl::forallBases(ForallBasesCallback *BaseMatches, - void *OpaqueData, +bool CXXRecordDecl::forallBases(ForallBasesCallback BaseMatches, bool AllowShortCircuit) const { SmallVector Queue; @@ -156,7 +158,7 @@ bool CXXRecordDecl::forallBases(ForallBasesCallback *BaseMatches, } Queue.push_back(Base); - if (!BaseMatches(Base, OpaqueData)) { + if (!BaseMatches(Base)) { if (AllowShortCircuit) return false; AllMatches = false; continue; @@ -171,10 +173,9 @@ bool CXXRecordDecl::forallBases(ForallBasesCallback *BaseMatches, return AllMatches; } -bool CXXBasePaths::lookupInBases(ASTContext &Context, - const CXXRecordDecl *Record, - CXXRecordDecl::BaseMatchesCallback *BaseMatches, - void *UserData) { +bool CXXBasePaths::lookupInBases( + ASTContext &Context, const CXXRecordDecl *Record, + CXXRecordDecl::BaseMatchesCallback BaseMatches) { bool FoundPath = false; // The access of the path down to this record. @@ -248,7 +249,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context, // Track whether there's a path involving this specific base. bool FoundPathThroughBase = false; - if (BaseMatches(&BaseSpec, ScratchPath, UserData)) { + if (BaseMatches(&BaseSpec, ScratchPath)) { // We've found a path that terminates at this base. FoundPath = FoundPathThroughBase = true; if (isRecordingPaths()) { @@ -263,7 +264,7 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context, CXXRecordDecl *BaseRecord = cast(BaseSpec.getType()->castAs() ->getDecl()); - if (lookupInBases(Context, BaseRecord, BaseMatches, UserData)) { + if (lookupInBases(Context, BaseRecord, BaseMatches)) { // C++ [class.member.lookup]p2: // A member name f in one sub-object B hides a member name f in // a sub-object A if A is a base class sub-object of B. Any @@ -296,11 +297,10 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context, return FoundPath; } -bool CXXRecordDecl::lookupInBases(BaseMatchesCallback *BaseMatches, - void *UserData, +bool CXXRecordDecl::lookupInBases(BaseMatchesCallback BaseMatches, CXXBasePaths &Paths) const { // If we didn't find anything, report that. - if (!Paths.lookupInBases(getASTContext(), this, BaseMatches, UserData)) + if (!Paths.lookupInBases(getASTContext(), this, BaseMatches)) return false; // If we're not recording paths or we won't ever find ambiguities, @@ -353,7 +353,7 @@ bool CXXRecordDecl::lookupInBases(BaseMatchesCallback *BaseMatches, bool CXXRecordDecl::FindBaseClass(const CXXBaseSpecifier *Specifier, CXXBasePath &Path, - void *BaseRecord) { + const CXXRecordDecl *BaseRecord) { assert(((Decl *)BaseRecord)->getCanonicalDecl() == BaseRecord && "User data for FindBaseClass is not canonical!"); return Specifier->getType()->castAs()->getDecl() @@ -362,7 +362,7 @@ bool CXXRecordDecl::FindBaseClass(const CXXBaseSpecifier *Specifier, bool CXXRecordDecl::FindVirtualBaseClass(const CXXBaseSpecifier *Specifier, CXXBasePath &Path, - void *BaseRecord) { + const CXXRecordDecl *BaseRecord) { assert(((Decl *)BaseRecord)->getCanonicalDecl() == BaseRecord && "User data for FindBaseClass is not canonical!"); return Specifier->isVirtual() && @@ -372,12 +372,11 @@ bool CXXRecordDecl::FindVirtualBaseClass(const CXXBaseSpecifier *Specifier, bool CXXRecordDecl::FindTagMember(const CXXBaseSpecifier *Specifier, CXXBasePath &Path, - void *Name) { + DeclarationName Name) { RecordDecl *BaseRecord = Specifier->getType()->castAs()->getDecl(); - DeclarationName N = DeclarationName::getFromOpaquePtr(Name); - for (Path.Decls = BaseRecord->lookup(N); + for (Path.Decls = BaseRecord->lookup(Name); !Path.Decls.empty(); Path.Decls = Path.Decls.slice(1)) { if (Path.Decls.front()->isInIdentifierNamespace(IDNS_Tag)) @@ -389,13 +388,12 @@ bool CXXRecordDecl::FindTagMember(const CXXBaseSpecifier *Specifier, bool CXXRecordDecl::FindOrdinaryMember(const CXXBaseSpecifier *Specifier, CXXBasePath &Path, - void *Name) { + DeclarationName Name) { RecordDecl *BaseRecord = Specifier->getType()->castAs()->getDecl(); const unsigned IDNS = IDNS_Ordinary | IDNS_Tag | IDNS_Member; - DeclarationName N = DeclarationName::getFromOpaquePtr(Name); - for (Path.Decls = BaseRecord->lookup(N); + for (Path.Decls = BaseRecord->lookup(Name); !Path.Decls.empty(); Path.Decls = Path.Decls.slice(1)) { if (Path.Decls.front()->isInIdentifierNamespace(IDNS)) @@ -408,12 +406,11 @@ bool CXXRecordDecl::FindOrdinaryMember(const CXXBaseSpecifier *Specifier, bool CXXRecordDecl:: FindNestedNameSpecifierMember(const CXXBaseSpecifier *Specifier, CXXBasePath &Path, - void *Name) { + DeclarationName Name) { RecordDecl *BaseRecord = Specifier->getType()->castAs()->getDecl(); - DeclarationName N = DeclarationName::getFromOpaquePtr(Name); - for (Path.Decls = BaseRecord->lookup(N); + for (Path.Decls = BaseRecord->lookup(Name); !Path.Decls.empty(); Path.Decls = Path.Decls.slice(1)) { // FIXME: Refactor the "is it a nested-name-specifier?" check diff --git a/lib/AST/DeclCXX.cpp b/lib/AST/DeclCXX.cpp index d905fcf13a..32e64b6e4e 100644 --- a/lib/AST/DeclCXX.cpp +++ b/lib/AST/DeclCXX.cpp @@ -385,17 +385,11 @@ void CXXRecordDecl::addedClassSubobject(CXXRecordDecl *Subobj) { } } -/// Callback function for CXXRecordDecl::forallBases that acknowledges -/// that it saw a base class. -static bool SawBase(const CXXRecordDecl *, void *) { - return true; -} - bool CXXRecordDecl::hasAnyDependentBases() const { if (!isDependentContext()) return false; - return !forallBases(SawBase, nullptr); + return !forallBases([](const CXXRecordDecl *) { return true; }); } bool CXXRecordDecl::isTriviallyCopyable() const { diff --git a/lib/AST/VTableBuilder.cpp b/lib/AST/VTableBuilder.cpp index ca5f0aad00..359e349802 100644 --- a/lib/AST/VTableBuilder.cpp +++ b/lib/AST/VTableBuilder.cpp @@ -2649,12 +2649,6 @@ struct InitialOverriddenDefinitionCollector { } // end namespace -static bool BaseInSet(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, void *BasesSet) { - BasesSetVectorTy *Bases = (BasesSetVectorTy *)BasesSet; - return Bases->count(Specifier->getType()->getAsCXXRecordDecl()); -} - // Let's study one class hierarchy as an example: // struct A { // virtual void f(); @@ -2720,8 +2714,12 @@ VFTableBuilder::ComputeThisOffset(FinalOverriders::OverriderInfo Overrider) { return Overrider.Offset; CXXBasePaths Paths; - Overrider.Method->getParent()->lookupInBases(BaseInSet, &Collector.Bases, - Paths); + Overrider.Method->getParent()->lookupInBases( + [&Collector](const CXXBaseSpecifier *Specifier, CXXBasePath &) { + return Collector.Bases.count( + Specifier->getType()->getAsCXXRecordDecl()); + }, + Paths); // This will hold the smallest this offset among overridees of MD. // This implies that an offset of a non-virtual base will dominate an offset diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index 0eea0e689d..661edc74c5 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -6500,50 +6500,45 @@ bool Sema::CheckVariableDeclaration(VarDecl *NewVD, LookupResult &Previous) { return false; } -/// \brief Data used with FindOverriddenMethod -struct FindOverriddenMethodData { +namespace { +struct FindOverriddenMethod { Sema *S; CXXMethodDecl *Method; -}; -/// \brief Member lookup function that determines whether a given C++ -/// method overrides a method in a base class, to be used with -/// CXXRecordDecl::lookupInBases(). -static bool FindOverriddenMethod(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, - void *UserData) { - RecordDecl *BaseRecord = Specifier->getType()->getAs()->getDecl(); + /// Member lookup function that determines whether a given C++ + /// method overrides a method in a base class, to be used with + /// CXXRecordDecl::lookupInBases(). + bool operator()(const CXXBaseSpecifier *Specifier, CXXBasePath &Path) { + RecordDecl *BaseRecord = + Specifier->getType()->getAs()->getDecl(); - FindOverriddenMethodData *Data - = reinterpret_cast(UserData); - - DeclarationName Name = Data->Method->getDeclName(); - - // FIXME: Do we care about other names here too? - if (Name.getNameKind() == DeclarationName::CXXDestructorName) { - // We really want to find the base class destructor here. - QualType T = Data->S->Context.getTypeDeclType(BaseRecord); - CanQualType CT = Data->S->Context.getCanonicalType(T); - - Name = Data->S->Context.DeclarationNames.getCXXDestructorName(CT); - } - - for (Path.Decls = BaseRecord->lookup(Name); - !Path.Decls.empty(); - Path.Decls = Path.Decls.slice(1)) { - NamedDecl *D = Path.Decls.front(); - if (CXXMethodDecl *MD = dyn_cast(D)) { - if (MD->isVirtual() && !Data->S->IsOverload(Data->Method, MD, false)) - return true; + DeclarationName Name = Method->getDeclName(); + + // FIXME: Do we care about other names here too? + if (Name.getNameKind() == DeclarationName::CXXDestructorName) { + // We really want to find the base class destructor here. + QualType T = S->Context.getTypeDeclType(BaseRecord); + CanQualType CT = S->Context.getCanonicalType(T); + + Name = S->Context.DeclarationNames.getCXXDestructorName(CT); + } + + for (Path.Decls = BaseRecord->lookup(Name); !Path.Decls.empty(); + Path.Decls = Path.Decls.slice(1)) { + NamedDecl *D = Path.Decls.front(); + if (CXXMethodDecl *MD = dyn_cast(D)) { + if (MD->isVirtual() && !S->IsOverload(Method, MD, false)) + return true; + } } + + return false; } - - return false; -} +}; + +enum OverrideErrorKind { OEK_All, OEK_NonDeleted, OEK_Deleted }; +} // end anonymous namespace -namespace { - enum OverrideErrorKind { OEK_All, OEK_NonDeleted, OEK_Deleted }; -} /// \brief Report an error regarding overriding, along with any relevant /// overriden methods. /// @@ -6571,13 +6566,13 @@ static void ReportOverrides(Sema& S, unsigned DiagID, const CXXMethodDecl *MD, bool Sema::AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD) { // Look for methods in base classes that this method might override. CXXBasePaths Paths; - FindOverriddenMethodData Data; - Data.Method = MD; - Data.S = this; + FindOverriddenMethod FOM; + FOM.Method = MD; + FOM.S = this; bool hasDeletedOverridenMethods = false; bool hasNonDeletedOverridenMethods = false; bool AddedAny = false; - if (DC->lookupInBases(&FindOverriddenMethod, &Data, Paths)) { + if (DC->lookupInBases(FOM, Paths)) { for (auto *I : Paths.found_decls()) { if (CXXMethodDecl *OldMD = dyn_cast(I)) { MD->addOverriddenMethod(OldMD->getCanonicalDecl()); diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp index 3395381b6a..64e61b5c38 100644 --- a/lib/Sema/SemaDeclAttr.cpp +++ b/lib/Sema/SemaDeclAttr.cpp @@ -432,11 +432,10 @@ static bool checkRecordTypeForCapability(Sema &S, QualType Ty) { // Else check if any base classes have a capability. if (CXXRecordDecl *CRD = dyn_cast(RD)) { CXXBasePaths BPaths(false, false); - if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &P, - void *) { - return BS->getType()->getAs() - ->getDecl()->hasAttr(); - }, nullptr, BPaths)) + if (CRD->lookupInBases([](const CXXBaseSpecifier *BS, CXXBasePath &) { + const auto *Type = BS->getType()->getAs(); + return Type->getDecl()->hasAttr(); + }, BPaths)) return true; } return false; diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 50890fbff8..3bd4a23eee 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -6268,77 +6268,75 @@ bool Sema::SpecialMemberIsTrivial(CXXMethodDecl *MD, CXXSpecialMember CSM, return true; } -/// \brief Data used with FindHiddenVirtualMethod namespace { - struct FindHiddenVirtualMethodData { - Sema *S; - CXXMethodDecl *Method; - llvm::SmallPtrSet OverridenAndUsingBaseMethods; - SmallVector OverloadedMethods; - }; -} - -/// \brief Check whether any most overriden method from MD in Methods -static bool CheckMostOverridenMethods(const CXXMethodDecl *MD, - const llvm::SmallPtrSetImpl& Methods) { - if (MD->size_overridden_methods() == 0) - return Methods.count(MD->getCanonicalDecl()); - for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(), - E = MD->end_overridden_methods(); - I != E; ++I) - if (CheckMostOverridenMethods(*I, Methods)) - return true; - return false; -} +struct FindHiddenVirtualMethod { + Sema *S; + CXXMethodDecl *Method; + llvm::SmallPtrSet OverridenAndUsingBaseMethods; + SmallVector OverloadedMethods; -/// \brief Member lookup function that determines whether a given C++ -/// method overloads virtual methods in a base class without overriding any, -/// to be used with CXXRecordDecl::lookupInBases(). -static bool FindHiddenVirtualMethod(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, - void *UserData) { - RecordDecl *BaseRecord = Specifier->getType()->getAs()->getDecl(); - - FindHiddenVirtualMethodData &Data - = *static_cast(UserData); - - DeclarationName Name = Data.Method->getDeclName(); - assert(Name.getNameKind() == DeclarationName::Identifier); - - bool foundSameNameMethod = false; - SmallVector overloadedMethods; - for (Path.Decls = BaseRecord->lookup(Name); - !Path.Decls.empty(); - Path.Decls = Path.Decls.slice(1)) { - NamedDecl *D = Path.Decls.front(); - if (CXXMethodDecl *MD = dyn_cast(D)) { - MD = MD->getCanonicalDecl(); - foundSameNameMethod = true; - // Interested only in hidden virtual methods. - if (!MD->isVirtual()) - continue; - // If the method we are checking overrides a method from its base - // don't warn about the other overloaded methods. Clang deviates from GCC - // by only diagnosing overloads of inherited virtual functions that do not - // override any other virtual functions in the base. GCC's - // -Woverloaded-virtual diagnoses any derived function hiding a virtual - // function from a base class. These cases may be better served by a - // warning (not specific to virtual functions) on call sites when the call - // would select a different function from the base class, were it visible. - // See FIXME in test/SemaCXX/warn-overload-virtual.cpp for an example. - if (!Data.S->IsOverload(Data.Method, MD, false)) +private: + /// Check whether any most overriden method from MD in Methods + static bool CheckMostOverridenMethods( + const CXXMethodDecl *MD, + const llvm::SmallPtrSetImpl &Methods) { + if (MD->size_overridden_methods() == 0) + return Methods.count(MD->getCanonicalDecl()); + for (CXXMethodDecl::method_iterator I = MD->begin_overridden_methods(), + E = MD->end_overridden_methods(); + I != E; ++I) + if (CheckMostOverridenMethods(*I, Methods)) return true; - // Collect the overload only if its hidden. - if (!CheckMostOverridenMethods(MD, Data.OverridenAndUsingBaseMethods)) - overloadedMethods.push_back(MD); - } + return false; } - if (foundSameNameMethod) - Data.OverloadedMethods.append(overloadedMethods.begin(), - overloadedMethods.end()); - return foundSameNameMethod; -} +public: + /// Member lookup function that determines whether a given C++ + /// method overloads virtual methods in a base class without overriding any, + /// to be used with CXXRecordDecl::lookupInBases(). + bool operator()(const CXXBaseSpecifier *Specifier, CXXBasePath &Path) { + RecordDecl *BaseRecord = + Specifier->getType()->getAs()->getDecl(); + + DeclarationName Name = Method->getDeclName(); + assert(Name.getNameKind() == DeclarationName::Identifier); + + bool foundSameNameMethod = false; + SmallVector overloadedMethods; + for (Path.Decls = BaseRecord->lookup(Name); !Path.Decls.empty(); + Path.Decls = Path.Decls.slice(1)) { + NamedDecl *D = Path.Decls.front(); + if (CXXMethodDecl *MD = dyn_cast(D)) { + MD = MD->getCanonicalDecl(); + foundSameNameMethod = true; + // Interested only in hidden virtual methods. + if (!MD->isVirtual()) + continue; + // If the method we are checking overrides a method from its base + // don't warn about the other overloaded methods. Clang deviates from + // GCC by only diagnosing overloads of inherited virtual functions that + // do not override any other virtual functions in the base. GCC's + // -Woverloaded-virtual diagnoses any derived function hiding a virtual + // function from a base class. These cases may be better served by a + // warning (not specific to virtual functions) on call sites when the + // call would select a different function from the base class, were it + // visible. + // See FIXME in test/SemaCXX/warn-overload-virtual.cpp for an example. + if (!S->IsOverload(Method, MD, false)) + return true; + // Collect the overload only if its hidden. + if (!CheckMostOverridenMethods(MD, OverridenAndUsingBaseMethods)) + overloadedMethods.push_back(MD); + } + } + + if (foundSameNameMethod) + OverloadedMethods.append(overloadedMethods.begin(), + overloadedMethods.end()); + return foundSameNameMethod; + } +}; +} // end anonymous namespace /// \brief Add the most overriden methods from MD to Methods static void AddMostOverridenMethods(const CXXMethodDecl *MD, @@ -6361,9 +6359,9 @@ void Sema::FindHiddenVirtualMethods(CXXMethodDecl *MD, CXXBasePaths Paths(/*FindAmbiguities=*/true, // true to look in all bases. /*bool RecordPaths=*/false, /*bool DetectVirtual=*/false); - FindHiddenVirtualMethodData Data; - Data.Method = MD; - Data.S = this; + FindHiddenVirtualMethod FHVM; + FHVM.Method = MD; + FHVM.S = this; // Keep the base methods that were overriden or introduced in the subclass // by 'using' in a set. A base method not in this set is hidden. @@ -6374,11 +6372,11 @@ void Sema::FindHiddenVirtualMethods(CXXMethodDecl *MD, if (UsingShadowDecl *shad = dyn_cast(*I)) ND = shad->getTargetDecl(); if (CXXMethodDecl *MD = dyn_cast(ND)) - AddMostOverridenMethods(MD, Data.OverridenAndUsingBaseMethods); + AddMostOverridenMethods(MD, FHVM.OverridenAndUsingBaseMethods); } - if (DC->lookupInBases(&FindHiddenVirtualMethod, &Data, Paths)) - OverloadedMethods = Data.OverloadedMethods; + if (DC->lookupInBases(FHVM, Paths)) + OverloadedMethods = FHVM.OverloadedMethods; } void Sema::NoteHiddenVirtualMethods(CXXMethodDecl *MD, @@ -8468,40 +8466,26 @@ bool Sema::CheckUsingDeclQualifier(SourceLocation UsingLoc, // in the UsingDecl and UsingShadowDecl so that these checks didn't // need to be repeated. - struct UserData { - llvm::SmallPtrSet Bases; - - static bool collect(const CXXRecordDecl *Base, void *OpaqueData) { - UserData *Data = reinterpret_cast(OpaqueData); - Data->Bases.insert(Base); - return true; - } - - bool hasDependentBases(const CXXRecordDecl *Class) { - return !Class->forallBases(collect, this); - } - - /// Returns true if the base is dependent or is one of the - /// accumulated base classes. - static bool doesNotContain(const CXXRecordDecl *Base, void *OpaqueData) { - UserData *Data = reinterpret_cast(OpaqueData); - return !Data->Bases.count(Base); - } - - bool mightShareBases(const CXXRecordDecl *Class) { - return Bases.count(Class) || !Class->forallBases(doesNotContain, this); - } + llvm::SmallPtrSet Bases; + auto Collect = [&Bases](const CXXRecordDecl *Base) { + Bases.insert(Base); + return true; }; - UserData Data; - - // Returns false if we find a dependent base. - if (Data.hasDependentBases(cast(CurContext))) + // Collect all bases. Return false if we find a dependent base. + if (!cast(CurContext)->forallBases(Collect)) return false; - // Returns false if the class has a dependent base or if it or one + // Returns true if the base is dependent or is one of the accumulated base + // classes. + auto IsNotBase = [&Bases](const CXXRecordDecl *Base) { + return !Bases.count(Base); + }; + + // Return false if the class has a dependent base or if it or one // of its bases is present in the base set of the current context. - if (Data.mightShareBases(cast(NamedContext))) + if (Bases.count(cast(NamedContext)) || + !cast(NamedContext)->forallBases(IsNotBase)) return false; Diag(SS.getRange().getBegin(), diff --git a/lib/Sema/SemaExprMember.cpp b/lib/Sema/SemaExprMember.cpp index 94716b4a71..afeabf5d91 100644 --- a/lib/Sema/SemaExprMember.cpp +++ b/lib/Sema/SemaExprMember.cpp @@ -27,18 +27,15 @@ using namespace clang; using namespace sema; typedef llvm::SmallPtrSet BaseSet; -static bool BaseIsNotInSet(const CXXRecordDecl *Base, void *BasesPtr) { - const BaseSet &Bases = *reinterpret_cast(BasesPtr); - return !Bases.count(Base->getCanonicalDecl()); -} /// Determines if the given class is provably not derived from all of /// the prospective base classes. static bool isProvablyNotDerivedFrom(Sema &SemaRef, CXXRecordDecl *Record, const BaseSet &Bases) { - void *BasesPtr = const_cast(reinterpret_cast(&Bases)); - return BaseIsNotInSet(Record, BasesPtr) && - Record->forallBases(BaseIsNotInSet, BasesPtr); + auto BaseIsNotInSet = [&Bases](const CXXRecordDecl *Base) { + return !Bases.count(Base->getCanonicalDecl()); + }; + return BaseIsNotInSet(Record) && Record->forallBases(BaseIsNotInSet); } enum IMAKind { diff --git a/lib/Sema/SemaLookup.cpp b/lib/Sema/SemaLookup.cpp index 99b945597b..ac09b3c085 100644 --- a/lib/Sema/SemaLookup.cpp +++ b/lib/Sema/SemaLookup.cpp @@ -1691,12 +1691,10 @@ static bool LookupQualifiedNameInUsingDirectives(Sema &S, LookupResult &R, /// \brief Callback that looks for any member of a class with the given name. static bool LookupAnyMember(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, - void *Name) { + CXXBasePath &Path, DeclarationName Name) { RecordDecl *BaseRecord = Specifier->getType()->getAs()->getDecl(); - DeclarationName N = DeclarationName::getFromOpaquePtr(Name); - Path.Decls = BaseRecord->lookup(N); + Path.Decls = BaseRecord->lookup(Name); return !Path.Decls.empty(); } @@ -1814,7 +1812,8 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx, Paths.setOrigin(LookupRec); // Look for this member in our base classes - CXXRecordDecl::BaseMatchesCallback *BaseCallback = nullptr; + bool (*BaseCallback)(const CXXBaseSpecifier *Specifier, CXXBasePath &Path, + DeclarationName Name) = nullptr; switch (R.getLookupKind()) { case LookupObjCImplicitSelfParam: case LookupOrdinaryName: @@ -1847,8 +1846,12 @@ bool Sema::LookupQualifiedName(LookupResult &R, DeclContext *LookupCtx, break; } - if (!LookupRec->lookupInBases(BaseCallback, - R.getLookupName().getAsOpaquePtr(), Paths)) + DeclarationName Name = R.getLookupName(); + if (!LookupRec->lookupInBases( + [=](const CXXBaseSpecifier *Specifier, CXXBasePath &Path) { + return BaseCallback(Specifier, Path, Name); + }, + Paths)) return false; R.setNamingClass(LookupRec); diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index c4f6fd8df1..c5d92a62bf 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -3512,16 +3512,14 @@ public: CXXCatchStmt *getFoundHandler() const { return FoundHandler; } CanQualType getFoundHandlerType() const { return FoundHandlerType; } - static bool FindPublicBasesOfType(const CXXBaseSpecifier *S, CXXBasePath &, - void *User) { - auto &PBOT = *reinterpret_cast(User); + bool operator()(const CXXBaseSpecifier *S, CXXBasePath &) { if (S->getAccessSpecifier() == AccessSpecifier::AS_public) { - CatchHandlerType Check(S->getType(), PBOT.CheckAgainstPointer); - auto M = PBOT.TypesToCheck; + CatchHandlerType Check(S->getType(), CheckAgainstPointer); + auto M = TypesToCheck; auto I = M.find(Check); if (I != M.end()) { - PBOT.FoundHandler = I->second; - PBOT.FoundHandlerType = PBOT.Ctx.getCanonicalType(S->getType()); + FoundHandler = I->second; + FoundHandlerType = Ctx.getCanonicalType(S->getType()); return true; } } @@ -3589,8 +3587,7 @@ StmtResult Sema::ActOnCXXTryBlock(SourceLocation TryLoc, Stmt *TryBlock, CXXBasePaths Paths; Paths.setOrigin(RD); CatchTypePublicBases CTPB(Context, HandledTypes, HandlerCHT.isPointer()); - if (RD->lookupInBases(CatchTypePublicBases::FindPublicBasesOfType, &CTPB, - Paths)) { + if (RD->lookupInBases(CTPB, Paths)) { const CXXCatchStmt *Problem = CTPB.getFoundHandler(); if (!Paths.isAmbiguous(CTPB.getFoundHandlerType())) { Diag(H->getExceptionDecl()->getTypeSpecStartLoc(), diff --git a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp index 3f608ba79e..32d3fa089f 100644 --- a/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp +++ b/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp @@ -690,9 +690,11 @@ static bool hasMember(const ASTContext &Ctx, const CXXRecordDecl *RD, return true; CXXBasePaths Paths(false, false, false); - if (RD->lookupInBases(&CXXRecordDecl::FindOrdinaryMember, - DeclName.getAsOpaquePtr(), - Paths)) + if (RD->lookupInBases( + [DeclName](const CXXBaseSpecifier *Specifier, CXXBasePath &Path) { + return CXXRecordDecl::FindOrdinaryMember(Specifier, Path, DeclName); + }, + Paths)) return true; return false;