aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM aside from a tiny commenting request.
================
Comment at: clang/lib/ASTMatchers/ASTMatchFinder.cpp:506
+ if (const auto *CTSD = Node.get<ClassTemplateSpecializationDecl>()) {
+ auto SK = CTSD->getSpecializationKind();
+ if (SK == TSK_ExplicitInstantiationDeclaration ||
----------------
steveire wrote:
> aaron.ballman wrote:
> > Same here, though this could also be simplified to:
> > ```
> > ScopedTraversal = (SK == TSK_ExplicitInstantiationDeclaration || SK ==
> > TSK_ExplicitInstantiationDefinition);
> > ```
> If `ScopedTraversal` is set to `true` above, this could wrongly set it to
> `false`.
> If ScopedTraversal is set to true above, this could wrongly set it to false.
Good catch!
================
Comment at: clang/unittests/AST/ASTTraverserTest.cpp:1092
+
+// Explicit instantiation of template functions do not appear in the AST
+template float timesTwo(float);
----------------
steveire wrote:
> aaron.ballman wrote:
> > Huh, do you have any idea if that's a bug? We have
> > `ClassTemplateSpecializationDecl` and `VarTemplateSpecializationDecl`, but
> > we have `FunctionTemplateSpecializationInfo` that doesn't generate an AST
> > node and no mention of why in the comments that I've spotted yet.
> I don't know why, but this is part of the confusion in the test in the
> discussion below.
>
> I can look into it after this is merged if you don't beat me to it.
> I don't know why, but this is part of the confusion in the test in the
> discussion below.
Okay, I kind of thought that might be the case, thank you for confirming.
================
Comment at: clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:2280
+ EXPECT_TRUE(matches(Code, traverse(TK_AsIs, M)));
+ EXPECT_TRUE(matches(Code, traverse(TK_IgnoreUnlessSpelledInSource, M)));
+ }
----------------
steveire wrote:
> aaron.ballman wrote:
> > Explicitly instantiating a function template in ignore mode returns false,
> > but explicitly instantiating a class template returns true? Is this
> > intentional or just fallout from the lack of explicit instantiation
> > information in the AST for functions?
> I've added some more tests and comments to try to clarify this.
>
> We should be able to match on the template arguments of explicit
> instantiations, but not the contents of the instantiations.
>
> Lack of representation of explicit function instantiations makes the expected
> test results confusing, but hopefully the comments now clarify.
Thanks for the new comment, that clarifies nicely! Can you add a full stop to
the end of the comment though?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90763/new/
https://reviews.llvm.org/D90763
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
