This is an archive of the discontinued LLVM Phabricator instance.

[SyntaxTree] Extend the syntax tree dump
ClosedPublic

Authored by eduucaldas on Aug 5 2020, 10:31 AM.

Details

Summary

Functional changes in the dump:

  • Surround Leaf tokens with '
  • Append Node dumps with NodeRole information, except for unknown roles

Non-functional changes:

  • ::dumpTokens(llvm::raw_ostream, ArrayRef<syntax::Token>, const SourceManager &SM) always received as parameter a syntax::Token * pointing to Leaf::token(). Changed the function to dumpLeaf(llvm::raw_ostream, syntax::Leaf *, const SourceManager&)
  • dumpTree acted on a Node, rename to dumpNode
  • use SourceManager in parameters for dump instead of Arena

TODO:
Adapt all the tree dumps in TreeTest.cpp to this new format.

Diff Detail

Event Timeline

eduucaldas created this revision.Aug 5 2020, 10:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2020, 10:31 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
eduucaldas requested review of this revision.Aug 5 2020, 10:31 AM
eduucaldas edited the summary of this revision. (Show Details)Aug 5 2020, 10:33 AM
eduucaldas added a reviewer: gribozavr2.

Rollback strOfLeaf

eduucaldas edited the summary of this revision. (Show Details)Aug 5 2020, 11:08 AM
gribozavr2 added inline comments.Aug 5 2020, 12:34 PM
clang/lib/Tooling/Syntax/Tree.cpp
149–150

Maybe the marks should be moved after the primary identifier of the node, WDYT? That would be more consistent with AST dump: first the node kind, then all its info.

Answer comment.

Please suggest more descriptive markers.
Options I thought of:
I -> unmodifiable
M -> not backed by source code / synthesized

Adapt Statement tests to new testing infrastructure

eduucaldas added inline comments.Aug 21 2020, 3:29 AM
clang/lib/Tooling/Syntax/Tree.cpp
149–150

I agree

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
107–110 ↗(On Diff #286983)

I just noticed that we didn't yet use annotations on statement tests.
I think that might help a bit, should I do a quick patch for adding those?

Output format LGTM!

clang/lib/Tooling/Syntax/Tree.cpp
172–174
175
207–210
clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
107–110 ↗(On Diff #286983)

Yes, that's a good idea!

eduucaldas marked an inline comment as done.

Update tests to dump role

eduucaldas marked 4 inline comments as done.
  • Nits
gribozavr2 accepted this revision.Aug 24 2020, 3:09 PM
This revision is now accepted and ready to land.Aug 24 2020, 3:09 PM

reorganize and nits