gribozavr2 added inline comments.
================ Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:295 syntax::NestedNameSpecifier *qualifier(); - // TODO after expose `id-expression` from `DependentScopeDeclRefExpr`: // Add accessor for `template_opt`. + syntax::Leaf *templateKeyword(); ---------------- You can remove this todo now. ================ Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:748 + bool isDecltype(const Type &T) { + return T.getTypeClass() == Type::TypeClass::Decltype; + } ---------------- Please use `isa<DecltypeType>(T)` and inline this expression into its only user. ================ Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:753 + return T.getTypeClass() == Type::TypeClass::TemplateSpecialization || + T.getTypeClass() == Type::TypeClass::DependentTemplateSpecialization; + } ---------------- Ditto, please use `isa` and inline into the only user. ================ Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:803 + // The 'template' keyword is always present in dependent template + // specializations + SR.setBegin(DependentTL.getTemplateKeywordLoc()); ---------------- ... except in the case of incorrect code. Feel free to just add a TODO. ================ Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:819 + auto *NS = BuildNameSpecifier(*NNS); + if (!syntax::GlobalNameSpecifier::classof(NS)) + Builder.foldNode(Builder.getRange(getLocalSourceRange(it)), NS, ---------------- Please use `isa<GlobalNameSpecifier>(NS)`. ================ Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:821 + Builder.foldNode(Builder.getRange(getLocalSourceRange(it)), NS, + nullptr); + Builder.markChild(NS, syntax::NodeRole::NestedNameSpecifier_specifier); ---------------- Could you add an overload for `foldNode` that takes a `NestedNameSpecifierLoc` but ignores it, just like we have an overload of `foldNode` that takes a `TypeLoc` but ignores it? ================ Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:834 + // FIXME: I feel like this could be upstreamed. + SourceRange getUnqualifiedIdSourceRange(DeclRefExpr *S) { + if (S->hasExplicitTemplateArgs()) ---------------- WDYM by "upstream"? ================ Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:844 + if (auto TKL = S->getTemplateKeywordLoc(); TKL.isValid()) + Builder.markChildToken(TKL, syntax::NodeRole::TemplateKeyword); ---------------- Please don't use C++17, Clang uses C++14 now. ================ Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:873 template<typename T> - static T f(){} + struct TS { + static void f(); ---------------- TS => ST (struct template?) ================ Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:879 +template<typename T> +struct TS { + struct S { ---------------- ST Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84348/new/ https://reviews.llvm.org/D84348 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits