[clang-tools-extra] r368261 - [clangd] Fix implicit template instatiations appearing as topLevelDecls.

Johan Vikstrom via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 8 00:21:06 PDT 2019


Author: jvikstrom
Date: Thu Aug  8 00:21:06 2019
New Revision: 368261

URL: http://llvm.org/viewvc/llvm-project?rev=368261&view=rev
Log:
[clangd] Fix implicit template instatiations appearing as topLevelDecls.

Summary: The parser gives implicit template instantiations to the action's HandleTopLevelDecls callback. This makes semantic highlighting highlight these templated functions multiple times. Fixed by filtering on if a Decl is an implicit function/variable/class instantiation. Also added a testcase to semantic highlighting on this.

Reviewers: hokein, ilya-biryukov

Subscribers: MaskRay, jkorous, arphaman, kadircet, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D65510

Modified:
    clang-tools-extra/trunk/clangd/AST.cpp
    clang-tools-extra/trunk/clangd/AST.h
    clang-tools-extra/trunk/clangd/ClangdUnit.cpp
    clang-tools-extra/trunk/clangd/CodeComplete.cpp
    clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp
    clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp

Modified: clang-tools-extra/trunk/clangd/AST.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.cpp?rev=368261&r1=368260&r2=368261&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/AST.cpp (original)
+++ clang-tools-extra/trunk/clangd/AST.cpp Thu Aug  8 00:21:06 2019
@@ -15,6 +15,7 @@
 #include "clang/AST/TemplateBase.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/Specifiers.h"
 #include "clang/Index/USRGeneration.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/Support/Casting.h"
@@ -41,13 +42,57 @@ getTemplateSpecializationArgLocs(const N
   // contain TemplateArgumentLoc information.
   return llvm::None;
 }
+
+template <class T>
+bool isTemplateSpecializationKind(const NamedDecl *D,
+                                  TemplateSpecializationKind Kind) {
+  if (const auto *TD = dyn_cast<T>(D))
+    return TD->getTemplateSpecializationKind() == Kind;
+  return false;
+}
+
+bool isTemplateSpecializationKind(const NamedDecl *D,
+                                  TemplateSpecializationKind Kind) {
+  return isTemplateSpecializationKind<FunctionDecl>(D, Kind) ||
+         isTemplateSpecializationKind<CXXRecordDecl>(D, Kind) ||
+         isTemplateSpecializationKind<VarDecl>(D, Kind);
+}
+
 } // namespace
 
+bool isImplicitTemplateInstantiation(const NamedDecl *D) {
+  return isTemplateSpecializationKind(D, TSK_ImplicitInstantiation);
+}
+
+bool isExplicitTemplateSpecialization(const NamedDecl *D) {
+  return isTemplateSpecializationKind(D, TSK_ExplicitSpecialization);
+}
+
 bool isImplementationDetail(const Decl *D) {
   return !isSpelledInSource(D->getLocation(),
                             D->getASTContext().getSourceManager());
 }
 
+// Returns true if the complete name of decl \p D is spelled in the source code.
+// This is not the case for:
+//   * symbols formed via macro concatenation, the spelling location will
+//     be "<scratch space>"
+//   * symbols controlled and defined by a compile command-line option
+//     `-DName=foo`, the spelling location will be "<command line>".
+bool isSpelledInSourceCode(const Decl *D) {
+  const auto &SM = D->getASTContext().getSourceManager();
+  auto Loc = D->getLocation();
+  // FIXME: Revisit the strategy, the heuristic is limitted when handling
+  // macros, we should use the location where the whole definition occurs.
+  if (Loc.isMacroID()) {
+    std::string PrintLoc = SM.getSpellingLoc(Loc).printToString(SM);
+    if (llvm::StringRef(PrintLoc).startswith("<scratch") ||
+        llvm::StringRef(PrintLoc).startswith("<command line>"))
+      return false;
+  }
+  return true;
+}
+
 SourceLocation findName(const clang::Decl *D) {
   return D->getLocation();
 }

Modified: clang-tools-extra/trunk/clangd/AST.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/AST.h?rev=368261&r1=368260&r2=368261&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/AST.h (original)
+++ clang-tools-extra/trunk/clangd/AST.h Thu Aug  8 00:21:06 2019
@@ -80,9 +80,23 @@ std::string printType(const QualType QT,
 /// take in to account using directives etc
 /// Example: shortenNamespace("ns1::MyClass<ns1::OtherClass>", "ns1")
 ///    --> "MyClass<ns1::OtherClass>"
-std::string  shortenNamespace(const llvm::StringRef OriginalName,
-                              const llvm::StringRef CurrentNamespace);
+std::string shortenNamespace(const llvm::StringRef OriginalName,
+                             const llvm::StringRef CurrentNamespace);
 
+/// Indicates if \p D is a template instantiation implicitly generated by the
+/// compiler, e.g.
+///     template <class T> struct vector {};
+///     vector<int> v; // 'vector<int>' is an implicit instantiation
+bool isImplicitTemplateInstantiation(const NamedDecl *D);
+/// Indicates if \p D is an explicit template specialization, e.g.
+///   template <class T> struct vector {};
+///   template <> struct vector<bool> {}; // <-- explicit specialization
+///
+/// Note that explicit instantiations are NOT explicit specializations, albeit
+/// they look similar.
+///   template struct vector<bool>; // <-- explicit instantiation, NOT an
+///   explicit specialization.
+bool isExplicitTemplateSpecialization(const NamedDecl *D);
 
 } // namespace clangd
 } // namespace clang

Modified: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdUnit.cpp?rev=368261&r1=368260&r2=368261&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp Thu Aug  8 00:21:06 2019
@@ -9,6 +9,7 @@
 #include "ClangdUnit.h"
 #include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
 #include "../clang-tidy/ClangTidyModuleRegistry.h"
+#include "AST.h"
 #include "Compiler.h"
 #include "Diagnostics.h"
 #include "Headers.h"
@@ -19,6 +20,7 @@
 #include "index/CanonicalIncludes.h"
 #include "index/Index.h"
 #include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/TokenKinds.h"
@@ -70,6 +72,9 @@ public:
       auto &SM = D->getASTContext().getSourceManager();
       if (!isInsideMainFile(D->getLocation(), SM))
         continue;
+      if (const NamedDecl *ND = dyn_cast<NamedDecl>(D))
+        if (isImplicitTemplateInstantiation(ND))
+          continue;
 
       // ObjCMethodDecl are not actually top-level decls.
       if (isa<ObjCMethodDecl>(D))

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=368261&r1=368260&r2=368261&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Aug  8 00:21:06 2019
@@ -1674,13 +1674,6 @@ private:
   }
 };
 
-template <class T> bool isExplicitTemplateSpecialization(const NamedDecl &ND) {
-  if (const auto *TD = dyn_cast<T>(&ND))
-    if (TD->getTemplateSpecializationKind() == TSK_ExplicitSpecialization)
-      return true;
-  return false;
-}
-
 } // namespace
 
 clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
@@ -1783,9 +1776,7 @@ bool isIndexedForCodeCompletion(const Na
   };
   // We only complete symbol's name, which is the same as the name of the
   // *primary* template in case of template specializations.
-  if (isExplicitTemplateSpecialization<FunctionDecl>(ND) ||
-      isExplicitTemplateSpecialization<CXXRecordDecl>(ND) ||
-      isExplicitTemplateSpecialization<VarDecl>(ND))
+  if (isExplicitTemplateSpecialization(&ND))
     return false;
 
   if (InTopLevelScope(ND))

Modified: clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp?rev=368261&r1=368260&r2=368261&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp Thu Aug  8 00:21:06 2019
@@ -6,13 +6,16 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include "AST.h"
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "SourceCode.h"
 #include "TestTU.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/TokenKinds.h"
 #include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock-matchers.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -21,6 +24,8 @@ namespace clangd {
 namespace {
 
 using ::testing::ElementsAre;
+using ::testing::ElementsAreArray;
+using ::testing::AllOf;
 
 TEST(ClangdUnitTest, GetBeginningOfIdentifier) {
   std::string Preamble = R"cpp(
@@ -72,6 +77,31 @@ MATCHER_P(DeclNamed, Name, "") {
   return false;
 }
 
+// Matches if the Decl has template args equal to ArgName. If the decl is a
+// NamedDecl and ArgName is an empty string it also matches.
+MATCHER_P(WithTemplateArgs, ArgName, "") {
+  if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(arg)) {
+    if (const auto *Args = FD->getTemplateSpecializationArgs()) {
+      std::string SpecializationArgs;
+      // Without the PrintingPolicy "bool" will be printed as "_Bool".
+      LangOptions LO;
+      PrintingPolicy Policy(LO);
+      Policy.adjustForCPlusPlus();
+      for (const auto Arg : Args->asArray()) {
+        if (SpecializationArgs.size() > 0)
+          SpecializationArgs += ",";
+        SpecializationArgs += Arg.getAsType().getAsString(Policy);
+      }
+      if (Args->size() == 0)
+        return ArgName == SpecializationArgs;
+      return ArgName == "<" + SpecializationArgs + ">";
+    }
+  }
+  if (const NamedDecl *ND = dyn_cast<NamedDecl>(arg))
+    return printTemplateSpecializationArgs(*ND) == ArgName;
+  return false;
+}
+
 TEST(ClangdUnitTest, TopLevelDecls) {
   TestTU TU;
   TU.HeaderCode = R"(
@@ -103,6 +133,65 @@ TEST(ClangdUnitTest, DoesNotGetIncludedT
   EXPECT_THAT(AST.getLocalTopLevelDecls(), ElementsAre(DeclNamed("main")));
 }
 
+TEST(ClangdUnitTest, DoesNotGetImplicitTemplateTopDecls) {
+  TestTU TU;
+  TU.Code = R"cpp(
+    template<typename T>
+    void f(T) {}
+    void s() {
+      f(10UL);
+    }
+  )cpp";
+
+  auto AST = TU.build();
+  EXPECT_THAT(AST.getLocalTopLevelDecls(),
+              ElementsAre(DeclNamed("f"), DeclNamed("s")));
+}
+
+TEST(ClangdUnitTest,
+     GetsExplicitInstantiationAndSpecializationTemplateTopDecls) {
+  TestTU TU;
+  TU.Code = R"cpp(
+    template <typename T>
+    void f(T) {}
+    template<>
+    void f(bool);
+    template void f(double);
+
+    template <class T>
+    struct V {};
+    template<class T>
+    struct V<T*> {};
+    template <>
+    struct V<bool> {};
+
+    template<class T>
+    T foo = T(10);
+    int i = foo<int>;
+    double d = foo<double>;
+
+    template <class T>
+    int foo<T*> = 0;
+    template <>
+    int foo<bool> = 0;
+  )cpp";
+
+  auto AST = TU.build();
+  EXPECT_THAT(
+      AST.getLocalTopLevelDecls(),
+      ElementsAreArray({AllOf(DeclNamed("f"), WithTemplateArgs("")),
+                        AllOf(DeclNamed("f"), WithTemplateArgs("<bool>")),
+                        AllOf(DeclNamed("f"), WithTemplateArgs("<double>")),
+                        AllOf(DeclNamed("V"), WithTemplateArgs("")),
+                        AllOf(DeclNamed("V"), WithTemplateArgs("<T *>")),
+                        AllOf(DeclNamed("V"), WithTemplateArgs("<bool>")),
+                        AllOf(DeclNamed("foo"), WithTemplateArgs("")),
+                        AllOf(DeclNamed("i"), WithTemplateArgs("")),
+                        AllOf(DeclNamed("d"), WithTemplateArgs("")),
+                        AllOf(DeclNamed("foo"), WithTemplateArgs("<>")),
+                        AllOf(DeclNamed("foo"), WithTemplateArgs("<bool>"))}));
+}
+
 TEST(ClangdUnitTest, TokensAfterPreamble) {
   TestTU TU;
   TU.AdditionalFiles["foo.h"] = R"(

Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=368261&r1=368260&r2=368261&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Thu Aug  8 00:21:06 2019
@@ -249,6 +249,12 @@ TEST(SemanticHighlighting, GetsCorrectTo
 
       template<typename $TemplateParameter[[T]]>
       void $Function[[foo]]($TemplateParameter[[T]] ...);
+    )cpp",
+    R"cpp(
+      template <class $TemplateParameter[[T]]>
+      struct $Class[[Tmpl]] {$TemplateParameter[[T]] $Field[[x]] = 0;};
+      extern template struct $Class[[Tmpl]]<float>;
+      template struct $Class[[Tmpl]]<double>;
     )cpp"};
   for (const auto &TestCase : TestCases) {
     checkHighlightings(TestCase);




More information about the cfe-commits mailing list