Page MenuHomePhabricator

Traverse-ignore explicit template instantiations
ClosedPublic

Authored by steveire on Wed, Nov 4, 7:07 AM.

Details

Summary

Continue to dump and match on explicit template specializations, but
omit explicit instantiation declarations and definitions.

Diff Detail

Event Timeline

steveire created this revision.Wed, Nov 4, 7:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptWed, Nov 4, 7:07 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
steveire requested review of this revision.Wed, Nov 4, 7:07 AM
aaron.ballman added inline comments.Thu, Nov 5, 12:54 PM
clang/lib/ASTMatchers/ASTMatchFinder.cpp
503

Please spell this type out rather than use auto.

506

Same here, though this could also be simplified to:

ScopedTraversal = (SK == TSK_ExplicitInstantiationDeclaration || SK == TSK_ExplicitInstantiationDefinition);
clang/unittests/AST/ASTTraverserTest.cpp
1092

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.

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
2280

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?

steveire marked 2 inline comments as done.Thu, Nov 5, 2:51 PM
steveire added inline comments.
clang/unittests/AST/ASTTraverserTest.cpp
1092

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.

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
2280

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.

steveire added inline comments.Thu, Nov 5, 3:11 PM
clang/lib/ASTMatchers/ASTMatchFinder.cpp
506

If ScopedTraversal is set to true above, this could wrongly set it to false.

aaron.ballman accepted this revision.Fri, Nov 6, 4:29 AM

LGTM aside from a tiny commenting request.

clang/lib/ASTMatchers/ASTMatchFinder.cpp
506

If ScopedTraversal is set to true above, this could wrongly set it to false.

Good catch!

clang/unittests/AST/ASTTraverserTest.cpp
1092

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.

clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
2280

Thanks for the new comment, that clarifies nicely! Can you add a full stop to the end of the comment though?

This revision is now accepted and ready to land.Fri, Nov 6, 4:29 AM
This revision was landed with ongoing or failed builds.Fri, Nov 6, 7:27 AM
This revision was automatically updated to reflect the committed changes.