eduucaldas added a reviewer: gribozavr2.
eduucaldas added a comment.
This is quite low priority, compared to the other branches of work, but it was
almost ready work, so I decided to polish it and send it to review now.
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:238-273
+ class ElementAndDelimiterIterator
+ : public llvm::iterator_facade_base<ElementAndDelimiterIterator,
+ std::forward_iterator_tag,
+ ElementAndDelimiter<Node>> {
+ public:
+ ElementAndDelimiterIterator(llvm::Optional<ElementAndDelimiter<Node>> ED)
+ : EDI(ED) {}
----------------
Should we hide some of this? Most of the member functions are a couple of
lines, so I decided not to. What is your opinion?
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:275-276
+
+ ElementAndDelimiterIterator getBegin();
+ ElementAndDelimiterIterator getEnd();
+
----------------
I chose to leave it as `getBegin` and `getEnd` instead of
`getElementsAndDelimitersBegin`. I did this because the return type of those
methods already tells us that we're getting an `ElementAndDelimiterIterator`.
What do you think?
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:312-319
+
+private:
+ // Auxiliary methods for implementing `ElementAndDelimiterIterator`.
+ static ElementAndDelimiter<Node> getWithDelimiter(Node *Element);
+ static llvm::Optional<ElementAndDelimiter<Node>>
+ getElementAndDelimiterAfterDelimiter(Leaf *Delimiter);
+ static llvm::Optional<ElementAndDelimiter<Node>>
----------------
These are all private static methods, should we hide them from the header?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88106/new/
https://reviews.llvm.org/D88106
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
