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

2020-04-22 Thread Haojian Wu via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG7d1ee639cb9e: [clangd] Fix a crash for accessing a null 
template decl returned by… (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78626

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


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1286,6 +1286,20 @@
 "1: targets = {}\n"
 "2: targets = {T}\n"
   },
+  // unknown template name should not crash.
+  {R"cpp(
+template  typename T>
+struct Base {};
+namespace foo {
+template 
+struct $1^Derive : $2^Base<$3^T::template $4^Unknown> {};
+}
+  )cpp",
+  "0: targets = {foo::Derive::T}, decl\n"
+  "1: targets = {foo::Derive}, decl\n"
+  "2: targets = {Base}\n"
+  "3: targets = {foo::Derive::T}\n"
+  "4: targets = {}, qualifier = 'T::'\n"},
 };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -860,15 +860,17 @@
   // TemplateArgumentLoc is the only way to get locations for references to
   // template template parameters.
   bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
+llvm::SmallVector 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:


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1286,6 +1286,20 @@
 "1: targets = {}\n"
 "2: targets = {T}\n"
   },
+  // unknown template name should not crash.
+  {R"cpp(
+template  typename T>
+struct Base {};
+namespace foo {
+template 
+struct $1^Derive : $2^Base<$3^T::template $4^Unknown> {};
+}
+  )cpp",
+  "0: targets = {foo::Derive::T}, decl\n"
+  "1: targets = {foo::Derive}, decl\n"
+  "2: targets = {Base}\n"
+  "3: targets = {foo::Derive::T}\n"
+  "4: targets = {}, qualifier = 'T::'\n"},
 };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -860,15 +860,17 @@
   // TemplateArgumentLoc is the only way to get locations for references to
   // template template parameters.
   bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
+llvm::SmallVector 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:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-04-22 Thread Haojian Wu via Phabricator via cfe-commits 
hokein marked an inline comment as done.
hokein added inline comments.



Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:1293
+struct Base {};
+namespace foo {
+template 

kadircet wrote:
> is namespace required?
yes, the test expects to look for `foo` namespace or `foo` function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78626



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-04-22 Thread Kadir Cetinkaya via Phabricator via cfe-commits 
kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

thanks, lgtm!




Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:1293
+struct Base {};
+namespace foo {
+template 

is namespace required?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D78626



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2020-04-22 Thread Haojian Wu via Phabricator via cfe-commits 
hokein created this revision.
hokein added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman, jkorous, MaskRay, ilya-biryukov.
Herald added a project: clang.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D78626

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


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1286,6 +1286,20 @@
 "1: targets = {}\n"
 "2: targets = {T}\n"
   },
+  // unknown template name should not crash.
+  {R"cpp(
+template  typename T>
+struct Base {};
+namespace foo {
+template 
+struct $1^Derive : $2^Base<$3^T::template $4^Unknown> {};
+}
+  )cpp",
+  "0: targets = {foo::Derive::T}, decl\n"
+  "1: targets = {foo::Derive}, decl\n"
+  "2: targets = {Base}\n"
+  "3: targets = {foo::Derive::T}\n"
+  "4: targets = {}, qualifier = 'T::'\n"},
 };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -860,15 +860,17 @@
   // TemplateArgumentLoc is the only way to get locations for references to
   // template template parameters.
   bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
+llvm::SmallVector 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:


Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1286,6 +1286,20 @@
 "1: targets = {}\n"
 "2: targets = {T}\n"
   },
+  // unknown template name should not crash.
+  {R"cpp(
+template  typename T>
+struct Base {};
+namespace foo {
+template 
+struct $1^Derive : $2^Base<$3^T::template $4^Unknown> {};
+}
+  )cpp",
+  "0: targets = {foo::Derive::T}, decl\n"
+  "1: targets = {foo::Derive}, decl\n"
+  "2: targets = {Base}\n"
+  "3: targets = {foo::Derive::T}\n"
+  "4: targets = {}, qualifier = 'T::'\n"},
 };
 
   for (const auto  : Cases) {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -860,15 +860,17 @@
   // TemplateArgumentLoc is the only way to get locations for references to
   // template template parameters.
   bool TraverseTemplateArgumentLoc(TemplateArgumentLoc A) {
+llvm::SmallVector 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:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits