[PATCH] D81168: Add support for DeclRefExpr in SyntaxTree, by generating IdExpressions

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.


================
Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:187
 
+/// A sequence of these specifiers make a `nested-name-specifier`
+class NameSpecifier final : public Tree {
----------------
Could you provide examples or a grammar quote in the comment?


================
Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:207
+
+/// Models an `unqualified-id`, e.g. in `std::vector<int>::size` it is `size`.
+/// C++ [expr.prim.id.unqual]
----------------
"the `size` in `std::vector<int>::size`"


================
Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:214
+    return N->kind() == NodeKind::UnqualifiedId;
+  }
+};
----------------
Could you add TODOs about adding accessors?


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:700
+TEST_P(SyntaxTreeTest, UnqualifiedId) {
+  if (!GetParam().isCXX11OrLater()) {
+    return;
----------------
Could you split off the parts of this test that don't depend on C++11?


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:983
+
+TEST_P(SyntaxTreeTest, QualifiedIdWithTemplate) {
+  if (!GetParam().isCXX()) {
----------------
"WithTemplateKeyword"


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1101
+
+TEST_P(SyntaxTreeTest, DecltypeQualifiedId) {
+  if (!GetParam().isCXX11OrLater()) {
----------------
"QualifiedIdDecltype" for consistency with other tests.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1109
+    static void f(){}
+  };
+void test(S s) {
----------------
Indent -2?


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:1112
+  decltype(s)::   // decltype-specifier
+  f();
+}
----------------
Could you indent the second line?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81168