[clang] bd763e2 - [clang] Fix crash on visiting null nestedNameSpecifier.

2020-03-18 Thread Haojian Wu via cfe-commits 

Author: Haojian Wu
Date: 2020-03-18T09:15:02+01:00
New Revision: bd763e2cf7c1d84bab95064cc5cbe542b227b025

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

LOG: [clang] Fix crash on visiting null nestedNameSpecifier.

Summary: Fix https://github.com/clangd/clangd/issues/293

Reviewers: sammccall

Subscribers: ilya-biryukov, kadircet, usaxena95, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/lib/Sema/SemaTemplate.cpp
clang/test/Parser/cxx-template-decl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 1ba66d4a976a..f95b16c1cbae 100755
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -5928,7 +5928,9 @@ bool UnnamedLocalNoLinkageFinder::VisitDependentNameType(
 
 bool UnnamedLocalNoLinkageFinder::VisitDependentTemplateSpecializationType(
  const DependentTemplateSpecializationType* T) 
{
-  return VisitNestedNameSpecifier(T->getQualifier());
+  if (auto *Q = T->getQualifier())
+return VisitNestedNameSpecifier(Q);
+  return false;
 }
 
 bool UnnamedLocalNoLinkageFinder::VisitPackExpansionType(
@@ -5982,6 +5984,7 @@ bool UnnamedLocalNoLinkageFinder::VisitTagDecl(const 
TagDecl *Tag) {
 
 bool UnnamedLocalNoLinkageFinder::VisitNestedNameSpecifier(
 NestedNameSpecifier *NNS) {
+  assert(NNS);
   if (NNS->getPrefix() && VisitNestedNameSpecifier(NNS->getPrefix()))
 return true;
 

diff  --git a/clang/test/Parser/cxx-template-decl.cpp 
b/clang/test/Parser/cxx-template-decl.cpp
index 3d7a3dc14f4c..0d52ad8fb50f 100644
--- a/clang/test/Parser/cxx-template-decl.cpp
+++ b/clang/test/Parser/cxx-template-decl.cpp
@@ -273,3 +273,9 @@ namespace AnnotateAfterInvalidTemplateId {
 namespace PR45063 {
   template> struct X {}; // expected-error 
{{undeclared identifier 'a'}}
 }
+
+namespace NoCrashOnEmptyNestedNameSpecifier {
+  template ::template arg_t<0>> // 
expected-error {{no template named 'ABC'}}
+  void foo(FnT) {}
+}



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

[PATCH] D76320: [clang] Fix crash on visiting null nestedNameSpecifier.

2020-03-18 Thread Haojian Wu via Phabricator via cfe-commits 
hokein updated this revision to Diff 251006.
hokein marked an inline comment as done.
hokein added a comment.

address review comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76320

Files:
  clang/lib/Sema/SemaTemplate.cpp
  clang/test/Parser/cxx-template-decl.cpp


Index: clang/test/Parser/cxx-template-decl.cpp
===
--- clang/test/Parser/cxx-template-decl.cpp
+++ clang/test/Parser/cxx-template-decl.cpp
@@ -273,3 +273,9 @@
 namespace PR45063 {
   template> struct X {}; // expected-error 
{{undeclared identifier 'a'}}
 }
+
+namespace NoCrashOnEmptyNestedNameSpecifier {
+  template ::template arg_t<0>> // 
expected-error {{no template named 'ABC'}}
+  void foo(FnT) {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -5928,7 +5928,9 @@
 
 bool UnnamedLocalNoLinkageFinder::VisitDependentTemplateSpecializationType(
  const DependentTemplateSpecializationType* T) 
{
-  return VisitNestedNameSpecifier(T->getQualifier());
+  if (auto *Q = T->getQualifier())
+return VisitNestedNameSpecifier(Q);
+  return false;
 }
 
 bool UnnamedLocalNoLinkageFinder::VisitPackExpansionType(
@@ -5982,6 +5984,7 @@
 
 bool UnnamedLocalNoLinkageFinder::VisitNestedNameSpecifier(
 NestedNameSpecifier *NNS) {
+  assert(NNS);
   if (NNS->getPrefix() && VisitNestedNameSpecifier(NNS->getPrefix()))
 return true;
 


Index: clang/test/Parser/cxx-template-decl.cpp
===
--- clang/test/Parser/cxx-template-decl.cpp
+++ clang/test/Parser/cxx-template-decl.cpp
@@ -273,3 +273,9 @@
 namespace PR45063 {
   template> struct X {}; // expected-error {{undeclared identifier 'a'}}
 }
+
+namespace NoCrashOnEmptyNestedNameSpecifier {
+  template ::template arg_t<0>> // expected-error {{no template named 'ABC'}}
+  void foo(FnT) {}
+}
Index: clang/lib/Sema/SemaTemplate.cpp
===
--- clang/lib/Sema/SemaTemplate.cpp
+++ clang/lib/Sema/SemaTemplate.cpp
@@ -5928,7 +5928,9 @@
 
 bool UnnamedLocalNoLinkageFinder::VisitDependentTemplateSpecializationType(
  const DependentTemplateSpecializationType* T) {
-  return VisitNestedNameSpecifier(T->getQualifier());
+  if (auto *Q = T->getQualifier())
+return VisitNestedNameSpecifier(Q);
+  return false;
 }
 
 bool UnnamedLocalNoLinkageFinder::VisitPackExpansionType(
@@ -5982,6 +5984,7 @@
 
 bool UnnamedLocalNoLinkageFinder::VisitNestedNameSpecifier(
 NestedNameSpecifier *NNS) {
+  assert(NNS);
   if (NNS->getPrefix() && VisitNestedNameSpecifier(NNS->getPrefix()))
 return true;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76320: [clang] Fix crash on visiting null nestedNameSpecifier.

2020-03-18 Thread Haojian Wu via Phabricator via cfe-commits 
hokein marked 2 inline comments as done.
hokein added inline comments.



Comment at: clang/lib/Sema/SemaTemplate.cpp:5926
const DependentNameType* T) 
{
-  return VisitNestedNameSpecifier(T->getQualifier());
+  if (auto *Q = T->getQualifier())
+return VisitNestedNameSpecifier(Q);

sammccall wrote:
> Do you have an idea of how we're getting here with a null qualifier?
> AIUI DependentNameType is a type that's a name with a dependent qualifier. If 
> the qualifier doesn't exist, it's not dependent. Is this a recovery path, 
> where the qualifier doesn't exist because it contains an error? (Like the 
> ABC in your test where ABC can't be resolved).
> 
> (Docs do reference MSVC-compat cases where the qualifier need not be 
> dependent, and maybe need not exist, but neither the linked bug nor the test 
> use msvc-compat)
right, the crash only happens on `DependentTemplateSpecializationType`. I made 
this change initially when trying the fix the crash (they look quite 
suspicious).

It can be ran on a recovery path, but I just checked all call sides, qualifier 
can not be null for DependentNameType (even for the MSVC mode). Reverted the 
change here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76320



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

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

2020-03-18 Thread Alexey Sachkov via Phabricator via cfe-commits 
AlexeySachkov added inline comments.



Comment at: clang/include/clang/AST/Type.h:1827-1830
+if (Dependent)
+  Deps |= TypeDependence::Dependent | TypeDependence::Instantiation;
+if (InstantiationDependent)
+  Deps |= TypeDependence::Instantiation;

sammccall wrote:
> AlexeySachkov wrote:
> > @ilya-biryukov, Is this code snippet correct?
> > 
> > It seems to be, that it should look like:
> > ```
> > if (Dependent)
> >   Deps |= TypeDependence::Dependent;
> > if (InstantiationDependent)
> >   Deps |= TypeDependence::Dependent | TypeDependence::Instantiation;
> > ```
> I agree that seems clearer, but ISTM they are equivalent because a dependent 
> type is always instantiation-dependent (right?)
> 
> Are you seeing related problems?
> Are you seeing related problems?

I though I was seeing a related problem, but it turned out that I wasn't.

Looking at the code again with a clear head, I believe that everything is 
correct here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71920



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

[PATCH] D75579: Replace MCTargetOptionsCommandFlags.inc and CommandFlags.inc by runtime-registration

2020-03-18 Thread serge via Phabricator via cfe-commits 
serge-sans-paille added a comment.

@modocache : thanks for the heads up, I'm investigating the issue.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75579



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

[PATCH] D76229: [clang-tidy] Added PlacementNewStorageCheck

2020-03-18 Thread Karasev Nikita via Phabricator via cfe-commits 
f00kat added a comment.

In D76229#1926818 , @Charusso wrote:

> In D76229#1925363 , @f00kat wrote:
>
> > In D76229#1925268 , @aaron.ballman 
> > wrote:
> >
> > > Have you considered making this a static analyzer check as opposed to a 
> > > clang-tidy check? I think using dataflow analysis will really reduce the 
> > > false positives because it will be able to track the allocation and 
> > > alignment data across control flow.
> >
> >
> > I have never try to write static analyzer before. Okay, I will look at 
> > examples of how to work with dataflow.
>
>
> To getting started with the Analyzer please visit @NoQ's (tiny bit outdated) 
> booklet: https://github.com/haoNoQ/clang-analyzer-guide
>
> Advertisement:
>  If D69726  lands we will have an arsenal of 
> APIs for helping dynamic size based checkers.
>  If D73521  lands it should be easier to 
> getting started with the Analyzer, but now you could visit what is our 
> current API.


Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76229



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

<    1   2   3