[PATCH] D65510: [clangd] Fix implicit template instatiations appearing as topLevelDecls.

[Ilya Biryukov via Phabricator via cfe-commits]

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 "_Bool".
----------------
Could you wrap the result with `<` and `>`, add `,` as a separator?
To make sure the output format for functions is not different for other decls


================
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:91
+      for (const auto Arg : Args->asArray()) {
+        SpecializationArgs += Arg.getAsType().getAsString(Policy);
+      }
----------------
NIT: remove braces


================
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:97
+  if (const NamedDecl *ND = dyn_cast<NamedDecl>(arg)) {
+    return printTemplateSpecializationArgs(*ND) == ArgName;
+  }
----------------
NIT: remove braces


================
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:186
+                        AllOf(DeclNamed("foo"), WithTemplateArgs("")),
+                        AllOf(DeclNamed("i"), WithTemplateArgs("")),
+                        AllOf(DeclNamed("d"), WithTemplateArgs("")),
----------------
NIT: do not match `WithTemplateArgs` for non-template names (like `i` and `d` here)?


================
Comment at: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp:188
+                        AllOf(DeclNamed("d"), WithTemplateArgs("")),
+                        AllOf(DeclNamed("foo"), WithTemplateArgs("<>")),
+                        AllOf(DeclNamed("foo"), WithTemplateArgs("<bool>"))}));
----------------
NIT: add `// FIXME: this should be '<T*>', not '<>'`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65510