[PATCH] D45482: [clangd] Match AST and Index label for template Symbols

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Nice, I'd been wondering about that...



================
Comment at: clangd/index/SymbolCollector.cpp:36
+      return *T;
+    return ND;
+  }
----------------
uber-nit: these three return statements are a bit confusing to me. Maybe omit them and if/elseif, so the default case falls through to the bottom.


================
Comment at: clangd/index/SymbolCollector.cpp:331
+  // We call getTemplateOrThis, since this is what clang's code completion gets
+  // from the lookup in an actual run.
+  CodeCompletionResult SymbolCompletion(&getTemplateOrThis(ND), 0);
----------------
"an actual run" confused me here. Maybe "We use the primary template, as clang does during code completion"?


================
Comment at: unittests/clangd/FileIndexTests.cpp:218
+      EXPECT_EQ(Sym.CompletionLabel, "vector<class Ty>");
+      EXPECT_EQ(Sym.CompletionSnippetInsertText, "vector<${1:class Ty}>");
+      SeenVector = true;
----------------
If snippets are off, we'll get "vector", not "vector<>", right?

(Probably no need to test this explicitly, but I just want to be sure)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D45482