gribozavr2 added inline comments.
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:211
+ return element == Other.element && delimiter == Other.delimiter;
+ }
};
----------------
Please also define `operator!=` even if it is not used yet.
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:225
+ /// elements and delimiters are represented as null pointers. Below we give
+ /// examples of how this iteration would look like:
///
----------------
"how something looks" or "what something looks like"
================
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) {}
----------------
eduucaldas wrote:
> 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?
I think it is fine as is.
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:269
+ return EDI == Other.EDI;
+ }
+
----------------
Please also define `operator!=`.
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:283
+ return ElementsAndDelimitersRange(getBegin(), getEnd());
+ }
+
----------------
I'd imagine derived classes would have iterators that produce strongly-typed
elements, right?
If so, these methods getBegin()/getEnd()/getRange() should have the word "Node"
in them.
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:314
+private:
+ // Auxiliary methods for implementing `ElementAndDelimiterIterator`.
+ static ElementAndDelimiter<Node> getWithDelimiter(Node *Element);
----------------
People can use "find usages" to find what uses these methods. Such comments
often go out of date.
================
Comment at: clang/lib/Tooling/Syntax/Tree.cpp:350
+ auto *Next = Element->getNextSibling();
+ // a b, x => End of list, this was the last ElementAndDelimiter.
+ if (!Next)
----------------
I have a hard time understand what a b x and the comma stand for in these
comments.
================
Comment at: clang/lib/Tooling/Syntax/Tree.cpp:391-396
+ auto Children = std::vector<syntax::List::ElementAndDelimiter<Node>>();
+ for (auto C : getRange()) {
+ Children.push_back(C);
}
+
+ return Children;
----------------
================
Comment at: clang/lib/Tooling/Syntax/Tree.cpp:405
return Children;
}
----------------
Ditto?
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
