[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-10 Thread Eduardo Caldas via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa90c78ac5261: [SyntaxTree] Implement the List construct. (authored by eduucaldas). Changed prior to commit:

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-10 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:249 + + /// Return whether *under valid code* the list can be empty. + /// gribozavr2 wrote: > "Whether this list can be empty in syntactically and semantically correct >

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-10 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 284292. eduucaldas marked 3 inline comments as done. eduucaldas added a comment. Answer code-review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85295/new/ https://reviews.llvm.org/D85295 Files:

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-10 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 accepted this revision. gribozavr2 added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:194 +/// A Tree that represents a syntactic list of elements. +/// """ A list of

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:247 + + TerminationKind getTerminationKind(); + gribozavr2 wrote: > I just realized that the rest of the syntax tree API does not use the `get~` > prefix. However, most of

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:247 + + TerminationKind getTerminationKind(); + I just realized that the rest of the syntax tree API does not use the `get~` prefix. However, most of Clang does, as far as

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-06 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:197 + MaybeTerminated, + Separated, +}; eduucaldas wrote: > gribozavr2 wrote: > > Add a "WithoutDelimiters" case as well? > I think we might want to treat

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas updated this revision to Diff 283536. eduucaldas marked 9 inline comments as done. eduucaldas added a comment. Answer comments non-delimited-lists need further discussion Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85295/new/

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-06 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:197 + MaybeTerminated, + Separated, +}; gribozavr2 wrote: > Add a "WithoutDelimiters" case as well? I think we might want to treat non-delimited-list in another way. as

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-05 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr2 added a comment. I feel uneasy about adding this code without tests. Could we maybe port the function parameter list to use this infrastructure, and then add tests that exercise `getElementsAsNodesAndDelimiters`? Comment at:

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-05 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas added a reviewer: gribozavr2. eduucaldas added inline comments. Comment at: clang/include/clang/Tooling/Syntax/Tree.h:223 + + // These can't be implemented with the information we have! + As this is a base List, we don't have the necessary

[PATCH] D85295: [SyntaxTree] Implement the List construct.

2020-08-05 Thread Eduardo Caldas via Phabricator via cfe-commits
eduucaldas created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. eduucaldas requested review of this revision. We defined a List construct to help with the implementation of list-like grammar rules. This is a first implementation of this API. Repository: