r260277 - Simplify and rename ASTMatchFinder's getCXXRecordDecl to make it more obvious

[Manuel Klimek via cfe-commits]

On Tue, Feb 9, 2016 at 10:03 PM Richard Smith via cfe-commits <
cfe-commits at 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.


> interface.
> +    CXXRecordDecl *ClassDecl =
> getAsCXXRecordDeclOrPrimaryTemplate(TypeNode);
>      if (!ClassDecl)
>        continue;
>      if (ClassDecl == Declaration) {
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160210/2823be0e/attachment-0001.html>