This is an archive of the discontinued LLVM Phabricator instance.

[AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor
ClosedPublic

Authored by johannes on Aug 22 2017, 2:09 AM.

Details

Summary

We need to specialize this because RecursiveASTVisitor visits template
template parameters after the templated declaration, unlike the order in
which they appear in the source code.

Diff Detail

Repository
rL LLVM

Event Timeline

johannes created this revision.Aug 22 2017, 2:09 AM
johannes updated this revision to Diff 112671.Aug 25 2017, 2:54 AM
johannes retitled this revision from [AST] Make TraverseTemplateParameterListHelper public to [AST] Traverse templates in LexicallyOrderedRecursiveASTVisitor.
johannes edited the summary of this revision. (Show Details)
johannes added a reviewer: arphaman.

use LexicallyOrderedRecursiveASTVisitor

johannes added inline comments.Aug 25 2017, 2:55 AM
unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
146 ↗(On Diff #112671)

This test also works before the change; what is the best way to enforce an ordering here?

arphaman edited edge metadata.Aug 25 2017, 3:15 AM

Awesome, thanks!

unittests/Tooling/LexicallyOrderedRecursiveASTVisitorTest.cpp
146 ↗(On Diff #112671)

You can add a flag to LexicallyOrderedDeclVisitor that emits the ordering index when it's true and turn it on for this test. Then you'd should get something like /f/T@0 and /f/T@1.

Please test a class template as well.

johannes updated this revision to Diff 112707.Aug 25 2017, 10:04 AM

test ordering, class template

This is currently broken, if a user provides a TraverseClassTemplateDecl, then the same method in this class will not be called, I think I will add a flag (probably not user visible) in RecursiveASTVisitor.h to switch the order for templates

johannes updated this revision to Diff 113546.Sep 1 2017, 9:21 AM

fix by adding an option in RecursiveASTVisitor

arphaman added inline comments.Sep 1 2017, 10:01 AM
include/clang/AST/RecursiveASTVisitor.h
507–508 ↗(On Diff #113546)

I don't think you need to make this public with the new patch, right?

johannes updated this revision to Diff 113581.Sep 1 2017, 12:52 PM

undo visibility change

This revision is now accepted and ready to land.Sep 4 2017, 12:48 AM
This revision was automatically updated to reflect the committed changes.
This comment was removed by pdouglas.