[cfe-commits] [Patch][Review request] RecursiveASTVisitor can now traverse template specializations
Benoit Belley
Benoit.Belley at autodesk.com
Mon Aug 30 14:27:41 PDT 2010
Hi John,
Thanks for taking the time to review my changes. The following patch should address all of your concerns. I have clarified that I want to traverse all of the template instantiations (not just the explicit ones).
/// This visitor always recurses in the declarator of explicit
/// template instantiations, such as for example "template class
/// set<int>'. But by default, it will not recurse in the
/// instantiations (implicit or explicit) of the template (function or
/// class) themselves since the instatiated AST isn't written in the
/// source code anywhere. This behavior can be changed by overriding
/// shouldVisitTemplateInstantiations() in the derived class to return
/// true.
Unfortunately, while testing my changes, I have come across an issue and I don’t know what the correct solution is. The following code snippet (inspired from the Intel TBB header files) demonstrates the issue:
—————————————————
// This is fine.
class A
{
friend class A;
}
// This class template definiton is also fine...
template<typename Container>
class vector_iterator
{
template <typename C> friend class vector_iterator;
};
// ... until it gets instantiated.
vector_iterator<int> it_int;
--------------------------------
It will throw the RecursiveASTVisitor in an infinite loop when shouldVisitTemplateInstantiations() == true. The attached file tracing.txt contains a trace of the visitor in action and points out to cause of the problem:
getFriendDecl() on the FriendDecl contained in the ClassTemplateSpecialization for vector_iterator<int> points back to the exact same ClassTemplateSpecialization object. Thus, the RecuriveASTVisitor recurses forever.
1) Is this behavior expected or is this a bug ?
2) If the behavior is expected, what’s the best way to solve this ?
a) Not traversing FriendDecl contained in ClassTemplateSpecializations ?
b) Keeping track of AST nodes that have already been visited (potentially costly) ?
Cheers,
Benoit
Le 2010-08-26 à 16:48, John McCall a écrit :
> On Aug 26, 2010, at 12:24 PM, Benoit Belley wrote:
>> I have modified the RecursiveASTVisitor so that it can now optionally visit every template specialization (class and fucntion templates, implicit and explicit instantiations).
>>
>> By default this visitor will not recurse in template specializations since the instatiated class isn’t written in the source code anywhere. This behavior can now be changed by calling setVisitingTemplateSpecializations(true) before starting the traversal.
>
> The idiomatic way of doing this with the CRTP is to call methods on the derived class instead of consulting flags.
>
> That is, TRAVERSE_DECL(ClassTemplateDecl) should TRY_TO(TraverseImplicitInstantantiations(D)), TraverseImplicitInstantiations should check getDerived().shouldTraverseImplicitInstantiations() before proceeding, and shouldTraverseImplicitInstantiations() should return false (which you would override in your subclass, of course).
>
> The way to check whether a function template specialization is an implicit instantiation is to consult getTemplateSpecializationInfo()->getTemplateSpecializationKind(). For a class template specialization, it's just getSpecializationKind().
>
> John.
Index: include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- include/clang/AST/RecursiveASTVisitor.h (revision 111934)
+++ include/clang/AST/RecursiveASTVisitor.h (working copy)
@@ -123,12 +123,26 @@
/// users may override Traverse* and WalkUpFrom* to implement custom
/// traversal strategies. Returning false from one of these overridden
/// functions will abort the entire traversal.
+///
+/// This visitor always recurses in the declarator of explicit
+/// template instantiations, such as for example "template class
+/// set<int>'. But by default, it will not recurse in the
+/// instantiations (implicit or explicit) of the template (function or
+/// class) themselves since the instatiated AST isn't written in the
+/// source code anywhere. This behavior can be changed by overriding
+/// shouldVisitTemplateInstantiations() in the derived class to return
+/// true.
+
template<typename Derived>
class RecursiveASTVisitor {
public:
/// \brief Return a reference to the derived class.
Derived &getDerived() { return *static_cast<Derived*>(this); }
+ /// \brief Return whether this visitor should recurse into
+ /// template instantiations.
+ bool shouldVisitTemplateInstantiations() const { return false; }
+
/// \brief Recursively visit a statement or expression, by
/// dispatching to Traverse*() based on the argument's dynamic type.
///
@@ -351,7 +365,9 @@
private:
// These are helper methods used by more than one Traverse* method.
bool TraverseTemplateParameterListHelper(TemplateParameterList *TPL);
- bool TraverseTemplateArgumentLocsHelper(const TemplateArgumentLoc *TAL,
+ bool TraverseImplicitClassInstantiations(ClassTemplateDecl* D);
+ bool TraverseFunctionInstantiations(FunctionTemplateDecl* D) ;
+ bool TraverseTemplateArgumentLocsHelper(const TemplateArgumentLoc *TAL,
unsigned Count);
bool TraverseArrayTypeLocHelper(ArrayTypeLoc TL);
bool TraverseRecordHelper(RecordDecl *D);
@@ -1092,19 +1108,96 @@
return true;
}
+// A helper method for traversing the implicit instantiations of a
+// class.
+template<typename Derived>
+bool RecursiveASTVisitor<Derived>::TraverseImplicitClassInstantiations(
+ ClassTemplateDecl* D) {
+ // By default, we do not traverse the instantiations of
+ // class templates since they do not apprear in the user code. The
+ // following code optionally traverses them.
+ if (!getDerived().shouldVisitTemplateInstantiations())
+ return true;
+
+ ClassTemplateDecl::spec_iterator end = D->spec_end();
+ for (ClassTemplateDecl::spec_iterator it = D->spec_begin(); it != end; ++it) {
+ ClassTemplateSpecializationDecl* SD = *it;
+
+ switch (SD->getSpecializationKind()) {
+ case TSK_Undeclared:
+ assert(false && "Semantically correct ASTs cannot contain "
+ "undeclared specializations.");
+ case TSK_ImplicitInstantiation:
+ TRY_TO(TraverseClassTemplateSpecializationDecl(*it));
+ break;
+ case TSK_ExplicitInstantiationDeclaration:
+ case TSK_ExplicitInstantiationDefinition:
+ case TSK_ExplicitSpecialization:
+ break;
+ default:
+ assert(false && "Unknown specialization kind.");
+ }
+ }
+
+ return true;
+}
+
DEF_TRAVERSE_DECL(ClassTemplateDecl, {
TRY_TO(TraverseDecl(D->getTemplatedDecl()));
TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));
- // We should not traverse the specializations/partial
- // specializations. Those will show up in other contexts.
- // getInstantiatedFromMemberTemplate() is just a link from a
- // template instantiation back to the template from which it was
- // instantiated, and thus should not be traversed either.
+ // Explicit class intantiations and specializations will be
+ // traversed from the context of their declaration. There
+ // is therefore no need to traverse them for here.
+ TRY_TO(TraverseImplicitClassInstantiations(D));
+
+ // Partial specializations are already traversed from their
+ // definition context. There is therefore no need to traverse them
+ // from here.
+
+ // Note that getInstantiatedFromMemberTemplate() is just a link
+ // from a template instantiation back to the template from which
+ // it was instantiated, and thus should not be traversed.
})
+// A helper method for traversing the instantiations of a
+// function while skipping its specializations.
+template<typename Derived>
+bool RecursiveASTVisitor<Derived>::TraverseFunctionInstantiations(
+ FunctionTemplateDecl* D) {
+ // By default, we do not traverse the instantiations of
+ // function templates since they do not apprear in the user code. The
+ // following code optionally traverses them.
+ if (!getDerived().shouldVisitTemplateInstantiations())
+ return true;
+
+ FunctionTemplateDecl::spec_iterator end = D->spec_end();
+ for (FunctionTemplateDecl::spec_iterator it = D->spec_begin(); it != end; ++it) {
+ FunctionDecl* FD = *it;
+ switch (FD->getTemplateSpecializationKind()) {
+ case TSK_Undeclared: // Declaration of the template definition.
+ case TSK_ImplicitInstantiation:
+ case TSK_ExplicitInstantiationDeclaration:
+ case TSK_ExplicitInstantiationDefinition:
+ TRY_TO(TraverseFunctionDecl(*it));
+ break;
+
+ case TSK_ExplicitSpecialization:
+ break;
+ default:
+ assert(false && "Unknown specialization kind.");
+ }
+ }
+
+ return true;
+}
+
DEF_TRAVERSE_DECL(FunctionTemplateDecl, {
TRY_TO(TraverseDecl(D->getTemplatedDecl()));
TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));
+ // Explicit function specializations will be traversed from the
+ // context of their declaration. There is therefore no need to
+ // traverse them for here.
+ TRY_TO(TraverseFunctionInstantiations(D));
})
DEF_TRAVERSE_DECL(TemplateTemplateParmDecl, {
@@ -1192,18 +1285,22 @@
})
DEF_TRAVERSE_DECL(ClassTemplateSpecializationDecl, {
- // For implicit instantiations ("set<int> x;"), we don't want to
- // recurse at all, since the instatiated class isn't written in
- // the source code anywhere. (Note the instatiated *type* --
- // set<int> -- is written, and will still get a callback of
- // TemplateSpecializationType). For explicit instantiations
- // ("template set<int>;"), we do need a callback, since this
- // is the only callback that's made for this instantiation.
- // We use getTypeAsWritten() to distinguish.
- // FIXME: see how we want to handle template specializations.
- if (TypeSourceInfo *TSI = D->getTypeAsWritten())
- TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
- return true;
+ if (shouldVisitTemplateInstantiations()) {
+ TRY_TO(TraverseCXXRecordHelper(D));
+ }
+ else {
+ // For implicit instantiations ("set<int> x;"), we don't want to
+ // recurse at all, since the instatiated class isn't written in
+ // the source code anywhere. (Note the instatiated *type* --
+ // set<int> -- is written, and will still get a callback of
+ // TemplateSpecializationType). For explicit instantiations
+ // ("template set<int>;"), we do need a callback, since this
+ // is the only callback that's made for this instantiation.
+ // We use getTypeAsWritten() to distinguish.
+ // FIXME: see how we want to handle template specializations.
+ if (TypeSourceInfo *TSI = D->getTypeAsWritten())
+ TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
+ }
})
template <typename Derived>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: RecursiveASTVisitor-TemplSpec.patch
Type: application/octet-stream
Size: 7834 bytes
Desc: RecursiveASTVisitor-TemplSpec.patch
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100830/5f0f595b/attachment.obj>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ATT00001..txt
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100830/5f0f595b/attachment.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: tracing.txt
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100830/5f0f595b/attachment-0001.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ATT00002..txt
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100830/5f0f595b/attachment-0002.txt>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: image002.gif
Type: image/gif
Size: 651 bytes
Desc: image002.gif
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100830/5f0f595b/attachment.gif>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ATT00003..txt
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100830/5f0f595b/attachment-0003.txt>
More information about the cfe-commits
mailing list