[PATCH] D90161: [SyntaxTree] Provide iterators for Lists

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(I started to review this, but now the current diff has only your changes against the first diff, rather than against master - can you upload a new diff?)



================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:231
+  /// `ElementAndDelimiter` acts as one.
+  template <typename ElementType> class ElementAndDelimiterIterator {
+  public:
----------------
this name is bulky and hard to read, leads to more unwieldy names later (e.g. `getElementAndDelimiterBeforeBegin` which despite its length is unclear that it's even an iterator).

Would there be actual confusion in calling this `ElementIterator`, and providing the associated delimiters in addition to the elements?


================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:282
+    /// Returns what you would expect from `It->element`.
+    ElementType *getElement() {
+      auto *ElementOrDelimiter = getElementOrDelimiter();
----------------
nit: const (and getDelimiter)

iterators, like pointers, don't propagate const to their pointees.

(This rarely matters, but it sometimes comes up with template goop)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90161