[cfe-dev] Possible ASTMatchers bug

[David Rector via cfe-dev]

I think this is indeed a bug, not in ASTMatchers but in RecursiveASTVisitor, since the intent of RecursiveASTVisitor is to never visit a node more than once.

The problem is this:

RecursiveASTVisitor method `shouldVisitTemplateInstantiations()`.  By default it returns false, but ASTMatchers overrides it to return true.  When it returns true, immediately upon traversing any Class/Function/VarTemplateDecl, the visitor also traverses its implicit instantiations (and those of any partial specializations thereof).

Still though, the intent of this is to only visit implicit instantiations, not e.g. explicit specializations — the intent is to only visit stuff that won’t be visited during subsequent traversals.  

This works fine for classes and functions, but not for VarTemplateDecls, because of a peculiarity of implementation: implicit instantiations of VarTemplates are added to the enclosing DeclContext, presumably to aid lookup.

This results in the implicitly instantiated VarTemplateSpecializationDecls always being visited twice when shouldVisitTemplateInstantiations() returns true.

So one or other of the traversals of these nodes must be eliminated in RecursiveASTVIsitor.  Probably better to eliminate the traversal of implicit VTSDs while looping over a DeclContext, since that is an implementation detail, and keep the the traversals while looping over the template instantiations.

So, RecursiveASTVisitor should probably be modified as follows: the `canIgnoreChildDeclWhileTraversingDeclContext(const Decl *Child)` (https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/RecursiveASTVisitor.h#L1370 <https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/RecursiveASTVisitor.h#L1370>) should also check if getDerived().shouldVisitTemplateInstantiations()==true && (Child is an implicitly instantiated VarTemplateSpecializationDecl).

> On Feb 1, 2021, at 8:38 AM, Aaron Ballman via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> On Sun, Jan 31, 2021 at 4:06 PM Hartogs Siegfried via cfe-dev
> <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>> 
>> Hi!
>> 
>> On the following code
>> 
>> template<typename T>
>> class C{};
>> template<typename T>
>> C<T> c;
>> void func(){
>>    c<float>;
>> }
>> 
>> with the matcher
>> 
>> varDecl(hasType(classTemplateSpecializationDecl()))
>> 
>> I get two matches which are exactly the same:
>> Match #1:
>> 
>> Binding for "root":
>> VarTemplateSpecializationDecl 0x7fe879820a28 <test:16:1, col:6> col:6 used c 'C<float>':'C<float>' callinit
>> |-TemplateArgument type 'float'
>> | `-BuiltinType 0x7fe87901ee00 'float'
>> `-CXXConstructExpr 0x7fe879852190 <col:6> 'C<float>':'C<float>' 'void () noexcept'
>> 
>> 
>> Match #2:
>> 
>> Binding for "root":
>> VarTemplateSpecializationDecl 0x7fe879820a28 <test:16:1, col:6> col:6 used c 'C<float>':'C<float>' callinit
>> |-TemplateArgument type 'float'
>> | `-BuiltinType 0x7fe87901ee00 'float'
>> `-CXXConstructExpr 0x7fe879852190 <col:6> 'C<float>':'C<float>' 'void () noexcept'
>> 
>> Is this a bug? I'd expect there to be only one match.
>> The goal is to find variables whose type refers to a class instantiation, is this a good approach?
>> Behavior occurs in clang-query and the C++ API on Apple, on both clang 11.0.0 and 10.0.1; didn't yet have the possibility to test 11.0.1.
> 
> I think the AST matchers are properly reflecting the information in
> the AST: https://godbolt.org/z/efYW3P <https://godbolt.org/z/efYW3P>
> 
> |-VarTemplateDecl <line:3:1, line:4:6> col:6 c
> | |-TemplateTypeParmDecl <line:3:10, col:19> col:19 referenced
> typename depth 0 index 0 T
> | |-VarDecl <line:4:1, col:6> col:6 c 'C<T>'
> | `-VarTemplateSpecializationDecl <col:1, col:6> col:6 used c
> 'C<float>':'C<float>' callinit
> |   |-TemplateArgument type 'float'
> |   | `-BuiltinType 'float'
> |   `-CXXConstructExpr <col:6> 'C<float>':'C<float>' 'void () noexcept'
> |-FunctionDecl <line:5:1, line:7:1> line:5:6 func 'void ()'
> | `-CompoundStmt <col:12, line:7:1>
> |   `-DeclRefExpr <line:6:5, col:12> 'C<float>':'C<float>' lvalue
> VarTemplateSpecialization 0x55f3fcf22b50 'c' 'C<float>':'C<float>'
> `-VarTemplateSpecializationDecl <line:4:1, col:6> col:6 used c
> 'C<float>':'C<float>' callinit
>  |-TemplateArgument type 'float'
>  | `-BuiltinType 'float'
>  `-CXXConstructExpr <col:6> 'C<float>':'C<float>' 'void () noexcept'
> 
> So I don't think this is an AST matcher bug. However, I'm not certain
> why the AST reflects the same information twice -- this has been the
> behavior as far back as Clang 3.4: https://godbolt.org/z/xhao8d <https://godbolt.org/z/xhao8d>
> 
> ~Aaron
> 
>> 
>> Thanks & best,
>> Sigi
>> _______________________________________________
>> cfe-dev mailing list
>> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20210201/b1901935/attachment-0001.html>