[PATCH] D88106: [SyntaxTree] Provide iterator-like functions for Lists

[Eduardo Caldas via Phabricator via cfe-commits]

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