[PATCH] D72334: [Syntax] Build nodes for template declarations.

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko added a comment.

Let's continue the review at https://reviews.llvm.org/D76346.



================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:190
+  // Set role for the node that may or may not be delayed. Node must span
+  // exactly \p Range.
+  void markMaybeDelayedChild(llvm::ArrayRef<syntax::Token> Range, NodeRole R);
----------------
gribozavr2 wrote:
> Three slashes for docs.
Done.


================
Comment at: clang/lib/Tooling/Syntax/BuildTree.cpp:930
+    Builder.markChildToken(TemplateKW, syntax::NodeRole::IntroducerKeyword);
+    Builder.markMaybeDelayedChild(
+        TemplatedDeclaration,
----------------
gribozavr2 wrote:
> Why is this range maybe-delayed?
E.g because `template <class T> struct cls {};` is doesn't need delaying, but template <class T> int var = 10;` does (SimpleDeclaration is processed before Declarator). I'll try to come up with a more fitting design after I submit this and https://reviews.llvm.org/D72446.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:738
+  U foo();
+};
+    )cpp",
----------------
gribozavr2 wrote:
> Could you also add an out-of-line definition of X::foo? It will have two template parameter lists, which is a special case.
Example:

template <class T>
 struct X {
   template <class U>
   U foo();
 };

**template <class T>**
template <class U>
U X<T>::foo() {}

Discussed offline, RAV doesn't visit TemplateParameters and therefore  we're not creating a node for the first template parameter (in bold). We could overcome this in the BuildTree.cpp, but more principled solution would be to teach RAV to visit TemplateParameters. In a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72334