[cfe-commits] [Patch][Review request] RecursiveASTVisitor can now traverse template specializations

[Benoit Belley]

Hi John,

Le 2010-08-30 à 21:37, John McCall a écrit :

> On Aug 30, 2010, at 6:08 PM, John McCall wrote:
>> On Aug 30, 2010, at 5:33 PM, Benoit Belley wrote:
>>> I think that I’ll need a bit more explanation here. The infinite recursion occurs while traversing the instantiations of the class template definition and I do want to traverse these instantiations. That’s the whole point of my proposed change.
>> 
>> Yes, I understood that.  My point is that you're visiting the instantiations regardless of whether the current template declaration you're visiting is a redeclaration or not.  This creates two problems:  first, you end up visiting the instantiations multiple times if the template is redeclared; and second, if the template is redeclared within itself (which can only be done with friend declarations), you end up visiting it recursively, which blows up the stack.  The solution is to only visit implicit instantiations when you visit a pattern definition.
>> 
>> template <class T> class A;  // <- don't visit any instantiations here
>> class B { template <class T> friend class A; };  // <- or here
>> template <class T> class A { ... }; // <- okay to visit implicit instantiations here
> 
> A few more notes.
> 
> First, note that my proposal means that you won't be visiting implicit instantiations for undefined templates.  I'm assuming that's okay;  otherwise, you'll need to use different logic (I think visiting instantiations for the first declaration should be sufficient).
> 

I’m also ok with not visiting implicit instantiations for undefined templates.

By undefined template, I am assuming that you mean something like:

----------------------------------------------
template <typename> class A;

const A<int>* foo(const A<int>* a)
{
    return a;
}
----------------------------------------------

where the template class A is never defined while the code still refers to one of its instantiations.
 
> Second, have you considered what you want to do with incomplete instantiations?  These arise when a template specialization is written but not required to be a complete type.  My thought is that these should be ignored, but you may feel differently.
> 

No, I have not considered them up to now. By incomplete instantiation, do you mean the following:

----------------------------------------------
template <typename> class A
{
public:
    void foo();
};

template <> class A<int>;

const A<int>* foo(const A<int>* a)
{
    return a;
}
----------------------------------------------


i.e. where I have declared that a specialization for A<int> exists even though I have not defined it. If so, my RecursiveASTVisitor to behave like expected, i.e. the instantiation for A<int> is never visited.

> Third, you need to decide what you want to do with instantiations of partial specializations.  A significant issue here is that it's possible to have complete instantiations of a template without having a definition for the primary template.  I think the most appropriate general solution is to visit instantiations whenever you reach their pattern;  for example:
>  template <class T> class A { ... }; // visit A<bool>, A<int> here
>  template <class T> class A<T*> { ... }; // visit A<char*>, A<int*> here
> 

Ok, I fixed this using:

    CXXRecordDecl* TempDecl = D->getTemplatedDecl();
    TRY_TO(TraverseDecl(TempDecl));
    TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));
    // Explicit class intantiations and specializations will be
    // traversed from the context of their declaration. There
    // is therefore no need to traverse them for here.
    //
    // In addition, we only traverse the class instantiations when the
    // class template is class template definition or the canonical
    // declaration when no definition of the class exists.
    if (TempDecl) {
      CXXRecordDecl* DefDecl  = TempDecl->getDefinition();
      CXXRecordDecl* CanoDecl = TempDecl->getCanonicalDecl();
      if (TempDecl == DefDecl || (!DefDecl && TempDecl == CanoDecl)) {
        TRY_TO(TraverseImplicitClassInstantiations(D));
      }
    }

Note that the behavior ends-up being slightly different from what you suggested. The instantiations are always traverse in the scope of a class declaration ClassTemplateDecl (either the definition or the canonical declaration when no definition exists). Instantiations of partial specialization are not traverse in the scope of the ClassTemplatePartialSpecializationDeclaration.

I am happy with this behavior, but would it be an issue for others ?   

> Finally, it looks like there's an existing bug (with a FIXME) where we're not visiting the bodies of explicit specializations.  If you could fix TraverseClassTemplateSpecializationDecl to visit the record when either your flag is set *or* it's an explicit specialization, that would be good.
> 

Sure.

> John.

Cheers,
Benoit 
 
Benoit Belley
Sr Principal Developer
M&E-Product Development Group    
 
Autodesk Canada Inc. 
10 Rue Duke
Montreal, Quebec  H3C 2L7 
Canada
 
Direct 514 954-7154
 
 

-------------- 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/20100901/2a4607bc/attachment.gif>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ATT00001..txt
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20100901/2a4607bc/attachment.txt>