Skip to content

Commit

Permalink
[clangd] Fix a crash for accessing a null template decl returned by f…
Browse files Browse the repository at this point in the history
…indExplicitReferences.

Summary: Fixes clangd/clangd#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 7d1ee63)
Includes some test-only changes from f651c40
to support the cherry-picked tests.
Test tweaked slightly as it exhibits a separate bug that was fixed on master.
  • Loading branch information
hokein authored and sam-mccall committed Jun 10, 2020
1 parent 0530e2a commit c900824
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 9 deletions.
10 changes: 6 additions & 4 deletions clang-tools-extra/clangd/FindTarget.cpp
Expand Up @@ -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:
Expand Down
36 changes: 31 additions & 5 deletions clang-tools-extra/clangd/unittests/FindTargetTests.cpp
Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit c900824

Please sign in to comment.