On Wed, Feb 10, 2016 at 10:04 AM Manuel Klimek <kli...@google.com> wrote:
> On Tue, Feb 9, 2016 at 10:03 PM Richard Smith via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: rsmith >> Date: Tue Feb 9 14:59:05 2016 >> New Revision: 260277 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=260277&view=rev >> Log: >> Simplify and rename ASTMatchFinder's getCXXRecordDecl to make it more >> obvious >> what it's actually trying to do. >> >> Modified: >> cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp >> >> Modified: cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp?rev=260277&r1=260276&r2=260277&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp (original) >> +++ cfe/trunk/lib/ASTMatchers/ASTMatchFinder.cpp Tue Feb 9 14:59:05 2016 >> @@ -744,46 +744,25 @@ private: >> MemoizationMap ResultCache; >> }; >> >> -static CXXRecordDecl *getAsCXXRecordDecl(const Type *TypeNode) { >> - // Type::getAs<...>() drills through typedefs. >> - if (TypeNode->getAs<DependentNameType>() != nullptr || >> - TypeNode->getAs<DependentTemplateSpecializationType>() != nullptr >> || >> - TypeNode->getAs<TemplateTypeParmType>() != nullptr) >> - // Dependent names and template TypeNode parameters will be matched >> when >> - // the template is instantiated. >> - return nullptr; >> - TemplateSpecializationType const *TemplateType = >> - TypeNode->getAs<TemplateSpecializationType>(); >> - if (!TemplateType) { >> - return TypeNode->getAsCXXRecordDecl(); >> - } >> - if (TemplateType->getTemplateName().isDependent()) >> - // Dependent template specializations will be matched when the >> - // template is instantiated. >> - return nullptr; >> - >> - // For template specialization types which are specializing a template >> - // declaration which is an explicit or partial specialization of >> another >> - // template declaration, getAsCXXRecordDecl() returns the corresponding >> - // ClassTemplateSpecializationDecl. >> - // >> - // For template specialization types which are specializing a template >> - // declaration which is neither an explicit nor partial specialization >> of >> - // another template declaration, getAsCXXRecordDecl() returns NULL and >> - // we get the CXXRecordDecl of the templated declaration. >> - CXXRecordDecl *SpecializationDecl = TemplateType->getAsCXXRecordDecl(); >> - if (SpecializationDecl) { >> - return SpecializationDecl; >> - } >> - NamedDecl *Templated = >> - >> TemplateType->getTemplateName().getAsTemplateDecl()->getTemplatedDecl(); >> - if (CXXRecordDecl *TemplatedRecord = >> dyn_cast<CXXRecordDecl>(Templated)) { >> - return TemplatedRecord; >> - } >> - // Now it can still be that we have an alias template. >> - TypeAliasDecl *AliasDecl = dyn_cast<TypeAliasDecl>(Templated); >> - assert(AliasDecl); >> - return getAsCXXRecordDecl(AliasDecl->getUnderlyingType().getTypePtr()); >> +static CXXRecordDecl * >> +getAsCXXRecordDeclOrPrimaryTemplate(const Type *TypeNode) { >> + if (auto *RD = TypeNode->getAsCXXRecordDecl()) >> + return RD; >> + >> + // Find the innermost TemplateSpecializationType that isn't an alias >> template. >> + auto *TemplateType = TypeNode->getAs<TemplateSpecializationType>(); >> + while (TemplateType && TemplateType->isTypeAlias()) >> + TemplateType = >> + >> TemplateType->getAliasedType()->getAs<TemplateSpecializationType>(); >> + >> + // If this is the name of a (dependent) template specialization, use >> the >> + // definition of the template, even though it might be specialized >> later. >> + if (TemplateType) >> + if (auto *ClassTemplate = dyn_cast_or_null<ClassTemplateDecl>( >> + TemplateType->getTemplateName().getAsTemplateDecl())) >> + return ClassTemplate->getTemplatedDecl(); >> + >> + return nullptr; >> } >> >> // Returns true if the given class is directly or indirectly derived >> @@ -800,7 +779,10 @@ bool MatchASTVisitor::classIsDerivedFrom >> if (typeHasMatchingAlias(TypeNode, Base, Builder)) >> return true; >> >> - CXXRecordDecl *ClassDecl = getAsCXXRecordDecl(TypeNode); >> + // FIXME: Going to the primary template here isn't really correct, >> but >> + // unfortunately we accept a Decl matcher for the base class not a >> Type >> + // matcher, so it's the best thing we can do with our current > > > Why not? Specifically, it seems to match what the user wants in most > cases. Now, I'm all for also exposing a more complex interface to the user, > where they can / have to control exactly where to dive into the primary > templates, but experience shows that this is really hard for anybody who's > not a C++ expert to understand, and the current behavior here matches what > people expect when they want to match classes in the inheritance hierarchy. > Read up on the other thread, and understand your concern now. The problem in matchers is: a) we want to be able to match primary templates, even if they're not instantiated b) the most common pattern to match on is names In template<typename T> class X : public Y<T> {} I think it would be very surprising if we didn't match that X is derived from a class called Y. > > >> interface. >> + CXXRecordDecl *ClassDecl = >> getAsCXXRecordDeclOrPrimaryTemplate(TypeNode); >> if (!ClassDecl) >> continue; >> if (ClassDecl == Declaration) { >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
