Author: Richard Smith Date: 2020-12-01T16:35:03-08:00 New Revision: 1f40d60a3b7f310ff3f77bb8643a27d979a703cb
URL: https://github.com/llvm/llvm-project/commit/1f40d60a3b7f310ff3f77bb8643a27d979a703cb DIFF: https://github.com/llvm/llvm-project/commit/1f40d60a3b7f310ff3f77bb8643a27d979a703cb.diff LOG: Remove CXXBasePaths::found_decls and simplify and modernize its only caller. This function did not satisfy its documented contract: it only considered the first lookup result on each base path, not all lookup results. It also performed unnecessary memory allocations. This change results in a minor change to our representation: we now include overridden methods that are found by any derived-to-base path (not involving another override) in the list of overridden methods for a function, rather than filtering out functions from bases that are both direct virtual bases and indirect virtual bases for which the indirect virtual base path contains another override for the function. (That filtering rule is part of the class-scope name lookup rules, and doesn't really have much to do with enumerating overridden methods.) The users of the list of overridden methods do not appear to rely on this filtering having happened, and it's simpler to not do it. Added: Modified: clang/include/clang/AST/CXXInheritance.h clang/lib/AST/CXXInheritance.cpp clang/lib/Sema/SemaDecl.cpp Removed: ################################################################################ diff --git a/clang/include/clang/AST/CXXInheritance.h b/clang/include/clang/AST/CXXInheritance.h index 8b1bcb367b3b..709f08bff82a 100644 --- a/clang/include/clang/AST/CXXInheritance.h +++ b/clang/include/clang/AST/CXXInheritance.h @@ -149,12 +149,6 @@ class CXXBasePaths { /// to help build the set of paths. CXXBasePath ScratchPath; - /// Array of the declarations that have been found. This - /// array is constructed only if needed, e.g., to iterate over the - /// results within LookupResult. - std::unique_ptr<NamedDecl *[]> DeclsFound; - unsigned NumDeclsFound = 0; - /// FindAmbiguities - Whether Sema::IsDerivedFrom should try find /// ambiguous paths while it is looking for a path from a derived /// type to a base type. @@ -170,8 +164,6 @@ class CXXBasePaths { /// is also recorded. bool DetectVirtual; - void ComputeDeclsFound(); - bool lookupInBases(ASTContext &Context, const CXXRecordDecl *Record, CXXRecordDecl::BaseMatchesCallback BaseMatches, bool LookupInDependent = false); @@ -198,8 +190,6 @@ class CXXBasePaths { using decl_range = llvm::iterator_range<decl_iterator>; - decl_range found_decls(); - /// Determine whether the path from the most-derived type to the /// given base type is ambiguous (i.e., it refers to multiple subobjects of /// the same base type). diff --git a/clang/lib/AST/CXXInheritance.cpp b/clang/lib/AST/CXXInheritance.cpp index 816b5d10773a..c87bcf31d120 100644 --- a/clang/lib/AST/CXXInheritance.cpp +++ b/clang/lib/AST/CXXInheritance.cpp @@ -33,29 +33,6 @@ using namespace clang; -/// Computes the set of declarations referenced by these base -/// paths. -void CXXBasePaths::ComputeDeclsFound() { - assert(NumDeclsFound == 0 && !DeclsFound && - "Already computed the set of declarations"); - - llvm::SmallSetVector<NamedDecl *, 8> Decls; - for (paths_iterator Path = begin(), PathEnd = end(); Path != PathEnd; ++Path) - Decls.insert(Path->Decls.front()); - - NumDeclsFound = Decls.size(); - DeclsFound = std::make_unique<NamedDecl *[]>(NumDeclsFound); - std::copy(Decls.begin(), Decls.end(), DeclsFound.get()); -} - -CXXBasePaths::decl_range CXXBasePaths::found_decls() { - if (NumDeclsFound == 0) - ComputeDeclsFound(); - - return decl_range(decl_iterator(DeclsFound.get()), - decl_iterator(DeclsFound.get() + NumDeclsFound)); -} - /// isAmbiguous - Determines whether the set of paths provided is /// ambiguous, i.e., there are two or more paths that refer to /// diff erent base class subobjects of the same type. BaseType must be diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 2116b3f7b78e..7da854f4fb9e 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -8077,73 +8077,54 @@ bool Sema::CheckVariableDeclaration(VarDecl *NewVD, LookupResult &Previous) { return false; } -namespace { -struct FindOverriddenMethod { - Sema *S; - CXXMethodDecl *Method; - - /// 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()->castAs<RecordType>()->getDecl(); +/// AddOverriddenMethods - See if a method overrides any in the base classes, +/// and if so, check that it's a valid override and remember it. +bool Sema::AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD) { + llvm::SmallPtrSet<const CXXMethodDecl*, 4> Overridden; - DeclarationName Name = Method->getDeclName(); + // Look for methods in base classes that this method might override. + CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/false, + /*DetectVirtual=*/false); + auto VisitBase = [&] (const CXXBaseSpecifier *Specifier, CXXBasePath &Path) { + CXXRecordDecl *BaseRecord = Specifier->getType()->getAsCXXRecordDecl(); + DeclarationName Name = MD->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<CXXMethodDecl>(D)) { - if (MD->isVirtual() && - !S->IsOverload( - Method, MD, /*UseMemberUsingDeclRules=*/false, - /*ConsiderCudaAttrs=*/true, - // C++2a [class.virtual]p2 does not consider requires clauses - // when overriding. - /*ConsiderRequiresClauses=*/false)) - return true; + QualType T = Context.getTypeDeclType(BaseRecord); + CanQualType CT = Context.getCanonicalType(T); + Name = Context.DeclarationNames.getCXXDestructorName(CT); + } + + for (NamedDecl *BaseND : BaseRecord->lookup(Name)) { + CXXMethodDecl *BaseMD = + dyn_cast<CXXMethodDecl>(BaseND->getCanonicalDecl()); + if (!BaseMD || !BaseMD->isVirtual() || + IsOverload(MD, BaseMD, /*UseMemberUsingDeclRules=*/false, + /*ConsiderCudaAttrs=*/true, + // C++2a [class.virtual]p2 does not consider requires + // clauses when overriding. + /*ConsiderRequiresClauses=*/false)) + continue; + + if (Overridden.insert(BaseMD).second) { + MD->addOverriddenMethod(BaseMD); + CheckOverridingFunctionReturnType(MD, BaseMD); + CheckOverridingFunctionAttributes(MD, BaseMD); + CheckOverridingFunctionExceptionSpec(MD, BaseMD); + CheckIfOverriddenFunctionIsMarkedFinal(MD, BaseMD); } + + // A method can only override one function from each base class. We + // don't track indirectly overridden methods from bases of bases. + return true; } return false; - } -}; -} // end anonymous namespace - -/// AddOverriddenMethods - See if a method overrides any in the base classes, -/// and if so, check that it's a valid override and remember it. -bool Sema::AddOverriddenMethods(CXXRecordDecl *DC, CXXMethodDecl *MD) { - // Look for methods in base classes that this method might override. - CXXBasePaths Paths; - FindOverriddenMethod FOM; - FOM.Method = MD; - FOM.S = this; - bool AddedAny = false; - if (DC->lookupInBases(FOM, Paths)) { - for (auto *I : Paths.found_decls()) { - if (CXXMethodDecl *OldMD = dyn_cast<CXXMethodDecl>(I)) { - MD->addOverriddenMethod(OldMD->getCanonicalDecl()); - if (!CheckOverridingFunctionReturnType(MD, OldMD) && - !CheckOverridingFunctionAttributes(MD, OldMD) && - !CheckOverridingFunctionExceptionSpec(MD, OldMD) && - !CheckIfOverriddenFunctionIsMarkedFinal(MD, OldMD)) { - AddedAny = true; - } - } - } - } + }; - return AddedAny; + DC->lookupInBases(VisitBase, Paths); + return !Overridden.empty(); } namespace { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
