This revision was automatically updated to reflect the committed changes.
jvikstrom marked 4 inline comments as done.
Closed by commit rL368261: [clangd] Fix implicit template instatiations
appearing as topLevelDecls. (authored by jvikstrom, committed by ).
Herald added a project: LLVM.
Herald add
ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM, but please print the function template arguments uniformly before landing
this (to avoid different forms of outputs inside the same test).
Repository:
rG LLVM Github Mon
jvikstrom marked an inline comment as done.
jvikstrom added inline comments.
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:188
+AllOf(DeclNamed("d"), WithTemplateArgs("")),
+AllOf(DeclNamed("foo"), WithTemplateA
jvikstrom marked an inline comment as done.
jvikstrom added inline comments.
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:144
+ EXPECT_THAT(AST.getLocalTopLevelDecls(),
+ ElementsAre(DeclNamed("f"), DeclNamed("f"), DeclNamed("f"),
+
ilya-biryukov added inline comments.
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:85
+if (const auto *Args = FD->getTemplateSpecializationArgs()) {
+ std::string SpecializationArgs;
+ // Without the PrintingPolicy "bool" will be printed as "_Bo
jvikstrom updated this revision to Diff 213878.
jvikstrom added a comment.
Changed tests to also look at template parameters.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65510/new/
https://reviews.llvm.org/D65510
Files:
clang-tools-extra/clang
ilya-biryukov added inline comments.
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:144
+ EXPECT_THAT(AST.getLocalTopLevelDecls(),
+ ElementsAre(DeclNamed("f"), DeclNamed("f"), DeclNamed("f"),
+ DeclNamed("V"), DeclNamed(
jvikstrom updated this revision to Diff 213808.
jvikstrom marked 4 inline comments as done.
jvikstrom added a comment.
Added more ways to specialize/instantiate templates to test.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65510/new/
https://rev
jvikstrom added inline comments.
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:144
+ EXPECT_THAT(AST.getLocalTopLevelDecls(),
+ ElementsAre(DeclNamed("f"), DeclNamed("f"), DeclNamed("f"),
+ DeclNamed("V"), DeclNamed("V")
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.
One important comment about somehow distinguishing multiple decls with the same
name.
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:110
+template
+void f(T) {}
+void
jvikstrom marked 2 inline comments as done.
jvikstrom added inline comments.
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:110
+template
+void f(T) {}
+void s() {
ilya-biryukov wrote:
> Could you also check that:
>
> 1. explicit
jvikstrom updated this revision to Diff 213608.
jvikstrom marked an inline comment as done.
jvikstrom added a comment.
Added tests for making sure explicit specializations, explicit instantiations,
partial instantiations, explicit declarations and template variables are in
topLevelDecls. Also ad
ilya-biryukov added inline comments.
Comment at: clang-tools-extra/clangd/AST.h:86
+bool isImplicitTemplateInstantiation(const NamedDecl *D);
+bool isExplicitTemplateSpecialization(const NamedDecl *D);
Could you add a small comment with an example?
```
/// Indi
jvikstrom updated this revision to Diff 213388.
jvikstrom marked an inline comment as done.
jvikstrom added a comment.
Move isImplicitTemplateInstantiation and isExplicitTemplateSpecialization, also
share implementation.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https:/
ilya-biryukov added inline comments.
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68
+ if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+ return false;
ilya-biryukov wrote:
> jvikstrom wrote:
ilya-biryukov added inline comments.
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68
+ if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+ return false;
jvikstrom wrote:
> ilya-biryukov wrote:
jvikstrom updated this revision to Diff 213331.
jvikstrom marked an inline comment as done.
jvikstrom added a comment.
Formatted.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65510/new/
https://reviews.llvm.org/D65510
Files:
clang-tools-extra/c
hokein added a comment.
the change looks good to me, leave the final approval to Ilya as he knows more
context.
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:116
+ auto AST = TU.build();
+ EXPECT_THAT(AST.getLocalTopLevelDecls(), ElementsAre(DeclNamed("f
jvikstrom marked 2 inline comments as done.
jvikstrom added inline comments.
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68
+ if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+ return false;
jvikstrom updated this revision to Diff 213041.
jvikstrom added a comment.
Moved test to ClangdUnitTests.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65510/new/
https://reviews.llvm.org/D65510
Files:
clang-tools-extra/clangd/AST.h
clang-tool
ilya-biryukov added inline comments.
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68
+ if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+ return false;
hokein wrote:
> ilya-biryukov wrote:
>
hokein added inline comments.
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68
+ if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+ return false;
ilya-biryukov wrote:
> We also want to skip `T
ilya-biryukov added inline comments.
Comment at: clang-tools-extra/clangd/ClangdUnit.cpp:68
+ if (const auto *TD = dyn_cast(D))
+return TD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation;
+ return false;
We also want to skip `TSK_ExplicitInsta
jvikstrom created this revision.
jvikstrom added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Herald added a project: clang.
The parser gives implicit template instantiations to the action's
HandleTopLevelDecls callback. This makes
24 matches
Mail list logo
