lukasza added inline comments.
================
Comment at: include/clang/AST/RecursiveASTVisitor.h:1728
@@ -1708,2 +1727,3 @@
+ TRY_TO(TraverseDeclTemplateParameterLists(D));
TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc()));
----------------
I think that tweaks of EnumDecl traversal together with tweaks of
TraverseRecordHelper tweaks, take care of all the relevant node types derived
from TagDecl:
- EnumDecl - traversal now calls TraverseDeclTemplateParameterLists (+EnumDecl
is covered by the new unit tests)
- RecordDecl - TraverseRecordHelper now calls
TraverseDeclTemplateParameterLists (+RecordDecl is covered by the new unit
tests)
- CXXRecordDecl - traversal includes a call to TraverseRecordHelper
I believe that nodes derived further from CXXRecordDecl cannot have a template
parameter list (?):
- ClassTemplateSpecializationDecl
- ClassTemplatePartialSpecializationDecl
If this is not correct, then can you please help me see the shape of unit tests
that would cover these nodes?
================
Comment at: include/clang/AST/RecursiveASTVisitor.h:1823
@@ -1802,2 +1822,3 @@
bool RecursiveASTVisitor<Derived>::TraverseDeclaratorHelper(DeclaratorDecl *D)
{
+ TRY_TO(TraverseDeclTemplateParameterLists(D));
TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc()));
----------------
I think that TraverseDeclaratorHelper together with TraverseFunctionHelper
tweaks, this takes care of all node types derived from DeclaratorDecl:
- FieldDecl - traversal calls TraverseDeclaratorHelper
- FunctionDecl - traversal calls TraverseFunctionHelper
- CXXMethodDecl - traversal calls TraverseFunctionHelper (+CXXMethodDecl is
covered by the new unit tests)
- CXXConstructorDecl - traversal calls TraverseFunctionHelper
- CXXConversionDecl - traversal calls TraverseFunctionHelper
- CXXDestructorDecl - traversal calls TraverseFunctionHelper
- VarDecl - TraverseVarHelper calls TraverseDeclaratorHelper (+VarDecl is
covered by the new unit tests)
I believe that nodes derived further from VarDecl cannot have a template
parameter list (?), but most of them should still be safe (because their
traversal goes through TraverseVarHelper):
- DecompositionDecl - traversal calls TraverseVarHelper
- ImplicitParamDecl - traversal calls TraverseVarHelper
- OMPCapturedExprDecl - traversal calls TraverseVarHelper
- ParmVarDecl - traversal calls TraverseVarHelper
- VarTemplateSpecializationDecl
- VarTemplatePartialSpecializationDecl
Please let me know if I am wrong above and it is indeed possible to have
template parameter lists next to VarTemplateSpecializationDecl and/or
VarTemplatePartialSpecializationDecl (and I would also appreciate if you could
help me see the shape of unit tests that would cover these nodes).
================
Comment at: include/clang/AST/RecursiveASTVisitor.h:1870
@@ -1848,2 +1869,3 @@
bool RecursiveASTVisitor<Derived>::TraverseFunctionHelper(FunctionDecl *D) {
+ TRY_TO(TraverseDeclTemplateParameterLists(D));
TRY_TO(TraverseNestedNameSpecifierLoc(D->getQualifierLoc()));
----------------
Richard, in https://llvm.org/bugs/show_bug.cgi?id=29042#c6 you've suggested
reusing TraverseDeclaratorHelper here. I agree that there are refactoring and
code reusing opportunities here (e.g. TraverseNestedNameSpecifier[Loc] are
getting called from both TraverseDeclaratorHelper and from
TraverseFunctionHelper), but I didn't pursue them in this patch, because I was
worried about little details:
1. traversal order (i.e. TraverseDeclaratorHelper traverses NestedNameSpecifier
immediately before traversing TypeSourceInfo; this is different than
TraverseFunctionHelper which traverses DeclarationNameInfo in-between
traversing NestedNameSpecifier and TypeSourceInfo)
2. traversal in absence of TypeSourceInfo (i.e. when TypeSourceInfo is missing,
then TraverseDeclaratorHelper traverses Type; this is different than
TraverseFunctionHelper which in this case 1) checks shouldVisitImplicitCode
condition and only then 2) traverses function parameters).
Because of the above, I think that for now it is better to call
TraverseDeclTemplateParameterLists directly from TraverseFunctionHelper, rather
than trying to reuse the whole (or bigger part of) TraverseDeclaratorHelper
from TraverseFunctionHelper.
================
Comment at: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp:1730
@@ +1729,3 @@
+ return type && InnerMatcher.matches(*type, Finder, Builder);
+}
+
----------------
I've needed this custom AST matcher in
https://codereview.chromium.org/2256913002 - let me try to sneak in a question
related to this other CL: Can I somehow avoid introducing a new, custom AST
matcher? (i.e. is it possible to express hasUnderlyingType in terms of
built-in matchers?)
https://reviews.llvm.org/D24268
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
