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

[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 many of the member functions stop making sense. 
`getElementsAsNodesAndDelimiters` 
`getDelimiterTokenKind`
`getTerminationKind`


================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:229
+  /// elements to empty or one-element lists.
+  clang::tok::TokenKind getDelimiterTokenKind();
+
----------------
gribozavr2 wrote:
> Should we change this function to return optional to allow representing lists that don't have any delimiters?
Cf, previous comment on non-delimited-lists


================
Comment at: clang/lib/Tooling/Syntax/Tree.cpp:277
+  if (!firstChild()) {
+    assert(canBeEmpty());
+    return {};
----------------
gribozavr2 wrote:
> This assert is only correct for valid code. When a syntax tree represents invalid code, any list can be empty.
Yeah, of course, we had already discussed that verifying those things in the casa of valid code would be the task of the verifier.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85295/new/

https://reviews.llvm.org/D85295