[PATCH] D61637: [Syntax] Introduce syntax trees

sammccall added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:43 + + struct Roles { +static constexpr NodeRole eof = 1; ilya-biryukov wrote: > sammccall wrote: > > we discussed offline - with 256 values, is it possible to come up wi

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added a comment. Relanded in rL365466 with a fix to the crash. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 __

[PATCH] D61637: [Syntax] Introduce syntax trees

RKSimon reopened this revision. RKSimon added a comment. This revision is now accepted and ready to land. @ilya-biryukov I'm sorry but I've reverted this at rL365465 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added a comment. In D61637#1575542 , @RKSimon wrote: > @ilya-biryukov We're seeing buildbot failures in SyntaxTests.exe : > > http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/50927 > > http://lab.llvm.o

[PATCH] D61637: [Syntax] Introduce syntax trees

RKSimon added a comment. @ilya-biryukov We're seeing buildbot failures in SyntaxTests.exe : http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/50927 http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast/builds/26822 Failing Tests (1):

[PATCH] D61637: [Syntax] Introduce syntax trees

This revision was automatically updated to reflect the committed changes. Closed by commit rGb736969eddce: [Syntax] Introduce syntax trees (authored by ilya-biryukov). Changed prior to commit: https://reviews.llvm.org/D61637?vs=208454&id=208455#toc Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:35 +/// A root node for a translation unit. Parent is always null. +class TranslationUnitDeclaration final : public Tree { +public: sammccall wrote: > I don't think TU is

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 208454. ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. - s/TranslationUnitDeclaration/TranslationUnit - Remove accessor from 'eof', add a FIXME to remove it from the tree altogether Repository: rG LLVM Github Monorepo

[PATCH] D61637: [Syntax] Introduce syntax trees

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Nice, let's land this! Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:35 +/// A root node for a translation unit. Parent is always null. +class TranslationUnitD

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added a comment. This is now in a pretty good shape, I've incorporated changes after our offline discussions about child roles. The builder interface is also much richer now, removing a requirement that the tree has to be traversed left-to-right (bottom-up is still required!). Re

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 206715. ilya-biryukov added a comment. - Remove (outdated) changes to gn files Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 Files: clang/include/clang/Toolin

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 206714. ilya-biryukov added a comment. - Introduce roles to allow distinguishing the child nodes. - Remove recovery node, use an unknown role instead. - TreeBuidler now can consume children at any point, not just suffix nodes. Repository: rG LLVM Git

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov marked 4 inline comments as done. ilya-biryukov added a comment. Although there are still rough edges, I believe the storage model is agreed upon and we can hopefully address the rest in the follow-ups. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov marked 13 inline comments as done. ilya-biryukov added a comment. This is ready for another round now. Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:37 + /// and \p Last are added as children of the new node. + void learnNode(SourceLocation Fist, SourceLoca

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 205974. ilya-biryukov added a comment. - A few more renames and docs - Cleanups and comments - Reformat the code Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 F

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 205589. ilya-biryukov added a comment. - Do the renames Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 Files: clang/include/clang/Tooling/Syntax/BuildTree.h

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added a comment. This is not 100% ready yet, but wanted to send it out anyway, as I'll be on vacation until Tuesday. I've addressed most major comments. In particular, `TreeBuilder` now looks simpler (and more structured) to my taste. One thing that's missing is adding children i

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 204488. ilya-biryukov marked 7 inline comments as done. ilya-biryukov added a comment. - A leaf node stores a single token - Restructure code to avoid special-casing leaf nodes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D61637: [Syntax] Introduce syntax trees

sammccall added a comment. In D61637#1527727 , @ilya-biryukov wrote: > I've addressed most of the comments, except the naming ones. > We need a convention for naming the language nodes and names for composite > and leaf structural nodes. > > For "langua

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:11 +// +// The code is divided in the following way: +// - 'Tree/Cascade.h' defines the basic structure of the syntax tree, This needs an update, will do in the next

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added a comment. I've addressed most of the comments, except the naming ones. We need a convention for naming the language nodes and names for composite and leaf structural nodes. For "language" nodes, I suggest we use `CompoundStatement`, `Recovery`, `TopLevelDeclaration`, `Temp

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 202745. ilya-biryukov marked 24 inline comments as done. ilya-biryukov added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Corpus.h:38 + std::pair> + tokenizeBuffer(std::unique_ptr Buffer); + sammccall wrote: > ilya-biryukov wrote: > > sammccall wrote: > > > Are you planning to have a way to add tok

[PATCH] D61637: [Syntax] Introduce syntax trees

sammccall added a comment. Definitely like the choice of CompositeNode owning the concrete storage! Comment at: clang/include/clang/Tooling/Syntax/Arena.h:1 +//===- Arena.h - memory arena and bookkeeping for syntax trees --- C++ -*-===// +// From a user's point

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 198606. ilya-biryukov marked 5 inline comments as done. ilya-biryukov added a comment. - Make traverse() internal to its only use-site. - s/Corpus/Arena. - Address some other comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov updated this revision to Diff 198609. ilya-biryukov added a comment. - s/corpus/arena - Remove an accidental cmake change Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61637/new/ https://reviews.llvm.org/D61637 Files: clang/include

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Corpus.h:23 +/// token buffers, source manager, etc. +class Corpus { +public: sammccall wrote: > I think plain SyntaxArena might be a better name here :-/ > Corpus refers to texts

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:40 +/// node. +void traverse(Node *N, llvm::function_ref Visit); +void traverse(const Node *N, llvm::function_ref Visit); sammccall wrote: > I've been burned with adding th

[PATCH] D61637: [Syntax] Introduce syntax trees

sammccall added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Corpus.h:23 +/// token buffers, source manager, etc. +class Corpus { +public: I think plain SyntaxArena might be a better name here :-/ Corpus refers to texts (the use in dex is by an

[PATCH] D61637: [Syntax] Introduce syntax trees

ilya-biryukov created this revision. ilya-biryukov added a reviewer: sammccall. Herald added a subscriber: mgorny. Herald added a project: clang. ilya-biryukov added a parent revision: D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library. An tooling-focused alternative to the