[PATCH] D84348: WIP: Add complete id-expression support to syntax trees

[Eduardo Caldas via Phabricator via cfe-commits]

eduucaldas added a reviewer: gribozavr2.
eduucaldas marked 6 inline comments as done.
eduucaldas added inline comments.


================
Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:101-106
+  UnknownNameSpecifier,
+  GlobalNameSpecifier,
+  NamespaceNameSpecifier,
+  TypeNameSpecifier,
+  IdentifierNameSpecifier,
+  TypeWithTemplateNameSpecifier
----------------
All of this is ephemeral, we are gonna have just one Name Specifier with a PointerUnion.


================
Comment at: clang/include/clang/Tooling/Syntax/Nodes.h:206-276
+class NameSpecifier : public Tree {
 public:
-  NameSpecifier() : Tree(NodeKind::NameSpecifier) {}
+  NameSpecifier(NodeKind K) : Tree(K) {}
   static bool classof(const Node *N) {
-    return N->kind() == NodeKind::NameSpecifier;
+    return N->kind() == NodeKind::GlobalNameSpecifier ||
+           N->kind() == NodeKind::TypeNameSpecifier ||
+           N->kind() == NodeKind::NamespaceNameSpecifier ||
----------------
(bis) All of this is temporary, we are gonna have just one Name Specifier with a PointerUnion.


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:768-769
-    // Get `UnqualifiedId` from `DeclRefExpr`.
-    // FIXME: Extract this logic so that it can be used by `MemberExpr`,
-    // and other semantic constructs, now it is tied to `DeclRefExpr`.
-    if (!S->hasExplicitTemplateArgs()) {
----------------
We extracted the logic into `getUnqualifiedSourceRange`


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:831-850
+  // FIXME: Same logic as DeclRefExpr. How to DRY?
+  SourceRange getUnqualifiedIdSourceRange(DependentScopeDeclRefExpr *S) {
+    if (S->hasExplicitTemplateArgs())
+      return SourceRange(S->getNameInfo().getBeginLoc(), S->getRAngleLoc());
+    return S->getNameInfo().getSourceRange();
+  }
+
----------------
This is the same logic as `DeclRefExpr`! Exacly the same code!
`DependentScopeDeclRefExpr` is important because it enables `T::template S<U>::`


================
Comment at: clang/lib/Tooling/Syntax/Nodes.cpp:119-130
+  case NodeKind::GlobalNameSpecifier:
+    return OS << "GlobalNameSpecifier";
+  case NodeKind::NamespaceNameSpecifier:
+    return OS << "NamespaceNameSpecifier";
+  case NodeKind::TypeNameSpecifier:
+    return OS << "TypeNameSpecifier";
+  case NodeKind::UnknownNameSpecifier:
----------------
(bis) All of this is temporary, we are gonna have just one Name Specifier with a PointerUnion.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:958-964
-struct X {
-  template<int> static void f();
-  template<int>
-  struct Y {
-    static void f();
-  };
-};
----------------
I noticed that we don't need that as everything is dependent on the template anyways


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84348