[llvm-branch-commits] [clang-tools-extra] c900824 - [clangd] Fix a crash for accessing a null template decl returned by findExplicitReferences.

[Sam McCall via llvm-branch-commits]

Author: Haojian Wu
Date: 2020-06-10T16:07:41+02:00
New Revision: c90082432021360fae9f838502479b9113854de4

URL: https://github.com/llvm/llvm-project/commit/c90082432021360fae9f838502479b9113854de4
DIFF: https://github.com/llvm/llvm-project/commit/c90082432021360fae9f838502479b9113854de4.diff

LOG: [clangd] Fix a crash for accessing a null template decl returned by findExplicitReferences.

Summary: Fixes https://github.com/clangd/clangd/issues/347.

Reviewers: kadircet

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, usaxena95, cfe-commits

Tags: #clang

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

(cherry picked from commit 7d1ee639cb9efea364bec90afe4d1161ec624a7f)
Includes some test-only changes from f651c402a221a20f3bc6ea43f70b29326a357010
to support the cherry-picked tests.
Test tweaked slightly as it exhibits a separate bug that was fixed on master.

Added: 
    

Modified: 
    clang-tools-extra/clangd/FindTarget.cpp
    clang-tools-extra/clangd/unittests/FindTargetTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp
index 5912464b0ed0..7a4f651b3620 100644
--- a/clang-tools-extra/clangd/FindTarget.cpp
+++ b/clang-tools-extra/clangd/FindTarget.cpp
@@ -759,15 +759,17 @@ class ExplicitReferenceCollector
   // TemplateArgumentLoc is the only way to get locations for references to
   // template template parameters.
   bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
+    llvm::SmallVector<const NamedDecl *, 1> Targets;
     switch (A.getArgument().getKind()) {
     case TemplateArgument::Template:
     case TemplateArgument::TemplateExpansion:
+      if (const auto *D = A.getArgument()
+                              .getAsTemplateOrTemplatePattern()
+                              .getAsTemplateDecl())
+        Targets.push_back(D);
       reportReference(ReferenceLoc{A.getTemplateQualifierLoc(),
                                    A.getTemplateNameLoc(),
-                                   /*IsDecl=*/false,
-                                   {A.getArgument()
-                                        .getAsTemplateOrTemplatePattern()
-                                        .getAsTemplateDecl()}},
+                                   /*IsDecl=*/false, Targets},
                       DynTypedNode::create(A.getArgument()));
       break;
     case TemplateArgument::Declaration:

diff  --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
index 9c1020b7a189..ae16c608fd33 100644
--- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -589,12 +589,21 @@ class FindExplicitReferencesTest : public ::testing::Test {
     auto *TestDecl = &findDecl(AST, "foo");
     if (auto *T = llvm::dyn_cast<FunctionTemplateDecl>(TestDecl))
       TestDecl = T->getTemplatedDecl();
-    auto &Func = llvm::cast<FunctionDecl>(*TestDecl);
 
     std::vector<ReferenceLoc> Refs;
-    findExplicitReferences(Func.getBody(), [&Refs](ReferenceLoc R) {
-      Refs.push_back(std::move(R));
-    });
+    if (const auto *Func = llvm::dyn_cast<FunctionDecl>(TestDecl))
+      findExplicitReferences(Func->getBody(), [&Refs](ReferenceLoc R) {
+        Refs.push_back(std::move(R));
+      });
+    else if (const auto *NS = llvm::dyn_cast<NamespaceDecl>(TestDecl))
+      findExplicitReferences(NS, [&Refs, &NS](ReferenceLoc R) {
+        // Avoid adding the namespace foo decl to the results.
+        if (R.Targets.size() == 1 && R.Targets.front() == NS)
+          return;
+        Refs.push_back(std::move(R));
+      });
+    else
+      ADD_FAILURE() << "Failed to find ::foo decl for test";
 
     auto &SM = AST.getSourceManager();
     llvm::sort(Refs, [&](const ReferenceLoc &L, const ReferenceLoc &R) {
@@ -984,7 +993,24 @@ TEST_F(FindExplicitReferencesTest, All) {
               }
             )cpp",
            "0: targets = {Test}\n"
-           "1: targets = {a}, decl\n"}};
+           "1: targets = {a}, decl\n"},
+      // unknown template name should not crash.
+      // duplicate $1$2 is fixed on master.
+      {R"cpp(
+        template <template <typename> typename T>
+        struct Base {};
+        namespace foo {
+        template <typename $0^T>
+        struct $1^$2^Derive : $3^Base<$4^T::template $5^Unknown> {};
+        }
+      )cpp",
+      "0: targets = {foo::Derive::T}, decl\n"
+      "1: targets = {foo::Derive}, decl\n"
+      "2: targets = {foo::Derive}, decl\n"
+      "3: targets = {Base}\n"
+      "4: targets = {foo::Derive::T}\n"
+      "5: targets = {}, qualifier = 'T::'\n"},
+    };
 
   for (const auto &C : Cases) {
     llvm::StringRef ExpectedCode = C.first;