sammccall added inline comments.
================
Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp:12
+//
+// shouldVisitTemplateInstantiations() controls traversal of AST nodes that
+// were created by instantiation, rather than by written code. This means:
----------------
hokein wrote:
> I think this is a very useful documentation about the RAV API, instead of
> staying in the test file, I would suggest moving it to the RAV.h file.
Good idea. I suppose it belongs on shouldVisitTemplateInstantiations().
================
Comment at: clang/unittests/Tooling/RecursiveASTVisitorTests/Templates.cpp:18
+// - Explicit instantiations are a tricky case:
+// - the "declaration part" (template args, function params etc) was
written
+// and should always be traversed.
----------------
hokein wrote:
> Conceptually, an instantiations create a specific {Class,
> Var}TemplateSpecializationDecl, functionDecl.
>
> ```
> template <class T> void foo(T) { }
> template void foo<char>(); // Line 2
> ```
>
> For the statement on line 2 (called `explicit instantiation definition` I
> think)
> - there is no corresponding node in the clang AST, this is a missing feature
> in clang
> - it creates a FunctionDecl, that is `void foo(char) { }`,
>
> does the "declaration part" in the comment refer to the declaration part of
> the `explicit instantiation definition` (I think so), or the declaration part
> of the FunctionDecl `void foo(char) {}`? The same question to the definition,
> my understanding from the comment is the function body of the `void foo(char)
> {}`, which is `{}`.
>
> We describe the expected behavior, but there are some bugs or unimplemented
> features in clang, which creates discrepancies. I think this is confusing and
> hard to follow, not sure how to do here (encoding a bunch of FIXMEs here is
> not good, not mentioning them at all is not good neither, can we mention
> only critical ones?)
>
>
>
>
> Conceptually, an instantiations create a specific {Class,
> Var}TemplateSpecializationDecl, functionDecl.
>
> ```
> template <class T> void foo(T) { }
> template void foo<char>(); // Line 2
> ```
>
> For the statement on line 2 (called `explicit instantiation definition` I
> think)
> - there is no corresponding node in the clang AST, this is a missing feature
> in clang
> - it creates a FunctionDecl, that is `void foo(char) { }`,
This is a plausible explanation (for functions and variables, but not classes).
I don't think it's the best model - I'd prefer to say that the instantiated
decl *corresponds* to the explicit instantiation definition.
In this example there are three decls: the template, the templated decl, and
the instantiation.
For functions these are {FunctionTemplateDecl, FunctionDecl, FunctionDecl}.
For variables these are {VarTemplateDecl, VarDecl,
VarTemplateSpecializationDecl}
For classes these are {ClassTemplateDecl, CXXRecordDecl,
ClassTemplateSpecializationDecl}.
(Sometimes in -dump-ast they appear multiple times, but there are only 3
distinct pointers).
There are inconsistencies here:
- function specializations don't have a distinct node type. I'm not sure this
is terribly significant. (I suspect it's related to the fact they can't be
partially specialized).
- class specializations have locations that point to the explicit
instantiation where possible (primary location, template parameter locations
etc), and the template body otherwise. Variable and function specializations
point at the template always.
- in `-dump-ast` output it's inconsistent whether specializations are shown at
the top level, under the template, or both. But I don't think there's any
significant meaning behind this.
The second inconsistency (locations) is the closest to answering the question
"do they correspond"?
Possible answers:
- specializations should correspond to the explicit instantiations, so
function + var are deficient in not reporting the locations
- specializations should not correspond, and class specializations are buggy
in reporting explicit instantiation locations
- class specializations should correspond to the explicit instantiations, but
others not (this seems silly)
I think #1 is the most plausible here, especially given that the
specializations do know (getTemplateSpecializationKind()) if they were produced
from an explicit instantiation.
> does the "declaration part" in the comment refer to the declaration part of
> the `explicit instantiation definition` (I think so), or the declaration part
> of the FunctionDecl `void foo(char) {}`? The same question to the definition,
> my understanding from the comment is the function body of the `void foo(char)
> {}`, which is `{}`.
This comment is talking about attributing AST nodes to source code, so it's
talking about the FunctionDecl. By "declaration part" I mean the
FunctionTypeLoc etc. I'll clarify this.
> We describe the expected behavior, but there are some bugs or unimplemented
> features in clang, which creates discrepancies. I think this is confusing and
> hard to follow, not sure how to do here (encoding a bunch of FIXMEs here is
> not good, not mentioning them at all is not good neither, can we mention
> only critical ones?)
I think we should say "there are some bugs, see FIXMEs below".
The concepts are complicated and I think trying to mix them in with the
implementation limitations will make a mess.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D120498/new/
https://reviews.llvm.org/D120498
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
