[cfe-commits] Fix for RecursiveASTMatcher not traversing nested template instatiations

[Richard Smith]

Hi,

On Thu, Apr 19, 2012 at 5:40 AM, Daniel Jasper <djasper at google.com> wrote:

> Please find a new version of the patch attached. This also fixes the same
> problem in nested function templates and includes corresponding tests. I am
> still not sure whether this is just a workaround for an underlying problem:
> Shouldn't the Decl fulfill isThisDeclarationADefinition()?


When a template is a member of another template, instantiating the outer
template doesn't cause an instantiation of the inner template. In your
added testcase, we instantiate X<A>::Y<A> directly from X::Y, and never
instantiate X<A>::Y. Since isThisDeclarationADefinition is used to
determine whether we have an available definition (rather than a definition
which could be instantiated), it has to return false for X<A>::Y.

I don't think we have any convenient way of asking the question which this
code wants to ask (which is, "is this the declaration with which the
instantiations of this template should be associated?"), but what you have
there looks reasonable.

I have one concern: it looks like we don't currently visit instantiations
of template classes which are declared, but not defined. With your change,
I think we will visit those instantiations for member templates of class
templates:

// I think we don't visit the instantiations of A
template<typename T> struct A;
A<int> *p;

template<typename T> struct B {
  // With your patch, I think we do visit the instantiations of B<int>::C
  template<typename U> struct C;
};
B<int>::C<int> *q;

I'm not sure to what extent this matters, but it would be good to be
consistent. Perhaps we should follow the getInstantiatedFromMemberTemplate
chain back to the original template, and check whether that declaration is
a definition?


> Kind regards,
> Daniel
>
> On Thu, Apr 12, 2012 at 10:32 PM, Daniel Jasper <djasper at google.com>wrote:
>
>> Hi all,
>>
>> we have noticed that the RecursiveASTMatcher does not seem to traverse
>> nested template instantiations as in this code segment:
>>
>> template <typename Rhs> struct Outer {
>>   template <typename Lhs> struct Inner {
>>     void Compare() {
>>       Lhs y;
>>       y.x();
>>     }
>>   };
>> };
>> struct A { void x(); };
>> int main() {
>>   (new Outer<A>::Inner<A>())->Compare();
>> }
>>
>> Here, the templates Outer and Inner are instantiated with type A, but the
>> Inner-instantiation is not traversed even if the RecursiveASTMatcher's
>> subclass implements shouldVisitTemplateInstantiations() to return true. The
>> reason is, that isThisDeclarationADefinition() does (erroneously?) return
>> false for this instantiation. The attached patch works around this. As far
>> as I can tell, isThisDeclarationADefinition() is checked in order to
>> prevent forward-declared templates to be traversed multiple times. However,
>> if this template is instantiated from a member template, this cannot happen.
>>
>> Kind regards,
>> Daniel
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120423/aa2c0f89/attachment.html>