gribozavr2 added inline comments.
================ Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:190 + return N->kind() == NodeKind::NameSpecifier; + } +}; ---------------- Should there be getters for various parts of the specifier? ================ Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:203 + +/// An identifier expression, e.g. `n::S::a`. Modeled according to the grammar +class IdExpression final : public Expression { ---------------- Could you add more details? Feel free to even quote the relevant grammar bits. ================ Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:211 + + syntax::Leaf *unqualifiedId(); + syntax::NestedNameSpecifier *qualifier(); ---------------- unqualified-id in the grammar can be more than one token. ================ Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:212 + syntax::Leaf *unqualifiedId(); + syntax::NestedNameSpecifier *qualifier(); +}; ---------------- Could you swap the order of getters to match the source order? ================ Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:618 + Builder.foldNode(Builder.getRange(it.getLocalSourceRange()), NS, nullptr); + Builder.markChild(NS, syntax::NodeRole::Unknown); + } ---------------- Do we need to mark the role if it is unknown? ================ Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:751 +void test(int b) { + ::a::b::S::f(); +} ---------------- Could you add more tests even if they fail today due to missing implementation? `a::b::operator-(a::b::S())` `a::b::S2<int>::f()` etc. Basically cover the whole grammar in tests, if possible. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81168/new/ https://reviews.llvm.org/D81168 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits