Page MenuHomePhabricator

[AST] Traverse attributes inside DEF_TRAVERSE_DECL macro
ClosedPublic

Authored by ilya-biryukov on Jul 18 2019, 2:37 AM.

Details

Summary

Instead of traversing inside the TraverseDecl() function.
Previously the attributes were traversed after Travese(Some)Decl
returns.

Logically attributes are properties of particular Decls and should be
traversed alongside other "child" nodes.

None of the tests relied on this behavior, hopefully this is an indication
that the change is relatively safe.

This change started with a discussion on cfe-dev, for details see:
https://lists.llvm.org/pipermail/cfe-dev/2019-July/062899.html

Diff Detail

Repository
rL LLVM

Event Timeline

ilya-biryukov created this revision.Jul 18 2019, 2:37 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2019, 2:37 AM
  • Visit attributes before visiting the Decl

Wanted to write unit tests, but couldn't find any that check traversal order. I'm aware of Tooling/RecursiveASTVisitorTests, but they mostly check that implicit nodes are visited.
Do we have other tests for traversals?

Looks like a good idea to me.

Regarding tests, I couldn't find existing tests that check order either. Seems like you'd need to make some minimal infrastructure for that.

gribozavr accepted this revision.Aug 6 2019, 1:44 AM
gribozavr added inline comments.
clang/unittests/AST/RecursiveASTVisitorTest.cpp
107 ↗(On Diff #212592)

Please add a newline.

This revision is now accepted and ready to land.Aug 6 2019, 1:44 AM
  • Add the missing newline
ilya-biryukov marked an inline comment as done.Aug 6 2019, 3:01 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 6 2019, 8:48 AM