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