This is an archive of the discontinued LLVM Phabricator instance.

[Syntax] Build mapping from AST to syntax tree nodes
ClosedPublic

Authored by ilya-biryukov on Jan 9 2020, 5:43 AM.

Details

Reviewers
gribozavr2
Summary

Only used to assign roles to child nodes for now. This is more efficient
than doing range-based queries.

In the future, will be exposed in the public API of syntax trees.

Diff Detail

Event Timeline

ilya-biryukov created this revision.Jan 9 2020, 5:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 9 2020, 5:43 AM

Remove the (now redundant) NodeAndRole class

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

  • Remove debug logs
  • Cosmetics

Unit tests: unknown.

clang-tidy: unknown.

clang-format: unknown.

Build artifacts: diff.json, console-log.txt

gribozavr2 accepted this revision.Jan 14 2020, 9:55 AM
gribozavr2 added inline comments.
clang/lib/Tooling/Syntax/BuildTree.cpp
147

Also assert that From is not null either?

160

I think it is possible to use a PointerUnion as a DenseMap key (there's a DenseMapInfo for it).

491

I don't understand the first fixme (maybe it is stale).

The second fixme is not applicable after this patch.

This revision is now accepted and ready to land.Jan 14 2020, 9:55 AM
hlopko added a subscriber: hlopko.Mar 20 2020, 4:04 AM

Taking over the patch in https://reviews.llvm.org/D76355.

clang/lib/Tooling/Syntax/BuildTree.cpp
147

Done.

160

Done.

491

Removed both FIXMEs now, asking Ilya offline for clarification.