eduucaldas abandoned this revision.
eduucaldas added inline comments.
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:252
+ Kind = IteratorKind::NotSentinel;
+ if (isElement(Begin))
+ Current = getWithDelimiter(cast<ElementType>(Begin));
----------------
Since `NodeRole` is forward declared, we cannot access `NodeRole::ListElement`
within the header, this is my work-around.
Another possibility would be to not forward declare and put `NodeRole`
definition here. I think this is a bad choice because most roles are linked to
syntax.
I declare `isElement` as a static private member function of `List` from lack
of better place, I accept suggestions.
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:390
+
+ ElementAndDelimiterIterator<Node> getBeforeBeginNode();
+ ElementAndDelimiterIterator<Node> getBeginNode();
----------------
I have a hard time finding a name that is expressive enough and concise enough.
The current one seems to imply that we're getting a node...
Alternatives:
* `getBeforeBeginWithElementAsNode`
* `getBeforeBeginWithNodes`
* `getBeforeBeginBase`
* `getBeforeBeginNodeAndDelimiter`
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:241-242
/// "a; b; c" <=> [("a" , ";"), ("b" , ";" ), ("c" , null)]
+ template <typename ElementType>
+ class ElementAndDelimiterIterator
+ : public llvm::iterator_facade_base<
----------------
eduucaldas wrote:
> Since we're gonna provide strongly-typed iterators, I make the Iterator a
> class template.
>
> I keep the base functions as they were, `getElementAndDelimiterAfter...`, but
> I cast their result using `castToElementType`.
>
> Another option would be to make the base functions also templated and not
> perform any casting.
>
> We'll use `castToElementType` to provide the strongly-typed iterators as well.
Ignore comment above
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88106/new/
https://reviews.llvm.org/D88106
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
