Re: [clang] 6f428e0 - [AST] Fix crashes on decltype(recovery-expr).

2020-04-01 Thread Haojian Wu via cfe-commits
Hello Douglas,

Thanks for the report. I don't think it is intentional, it is certainly a
regression (I'm surprised the patch would lead to such behavior), I will
take a look tomorrow.

On Wed, Apr 1, 2020 at 8:28 PM Yung, Douglas via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Hi Haojian,
>
> I noticed that after your change, the compiler is now giving an error when
> trying to create a vector of >= 1024 elements when previously it worked,
> and gcc has no problem with the same code. Is that intentional? I have put
> the details in PR45387, can you take a look?
>
> Douglas Yung
>
> -Original Message-
> From: cfe-commits  On Behalf Of
> Haojian Wu via cfe-commits
> Sent: Monday, March 30, 2020 5:57
> To: cfe-commits@lists.llvm.org
> Subject: [clang] 6f428e0 - [AST] Fix crashes on decltype(recovery-expr).
>
>
> Author: Haojian Wu
> Date: 2020-03-30T14:56:33+02:00
> New Revision: 6f428e09fbe8ce7e3510ae024031a5fc19653483
>
> URL:
> https://github.com/llvm/llvm-project/commit/6f428e09fbe8ce7e3510ae024031a5fc19653483
> DIFF:
> https://github.com/llvm/llvm-project/commit/6f428e09fbe8ce7e3510ae024031a5fc19653483.diff
>
> LOG: [AST] Fix crashes on decltype(recovery-expr).
>
> Summary: We mark these decls as invalid.
>
> Reviewers: sammccall
>
> Subscribers: cfe-commits
>
> Tags: #clang
>
> Differential Revision: https://reviews.llvm.org/D77037
>
> Added:
>
>
> Modified:
> clang/include/clang/AST/DependenceFlags.h
> clang/include/clang/AST/Type.h
> clang/lib/Parse/ParseExprCXX.cpp
> clang/lib/Sema/SemaType.cpp
> clang/test/AST/ast-dump-expr-errors.cpp
> clang/test/Sema/invalid-member.cpp
> clang/unittests/Sema/CodeCompleteTest.cpp
>
> Removed:
>
>
>
>
> 
> diff  --git a/clang/include/clang/AST/DependenceFlags.h
> b/clang/include/clang/AST/DependenceFlags.h
> index 75c9aa1656b8..0b24bae6df9b 100644
> --- a/clang/include/clang/AST/DependenceFlags.h
> +++ b/clang/include/clang/AST/DependenceFlags.h
> @@ -50,14 +50,16 @@ struct TypeDependenceScope {
>  /// Whether this type is a variably-modified type (C99 6.7.5).
>  VariablyModified = 8,
>
> -// FIXME: add Error bit.
> +/// Whether this type references an error, e.g.
> decltype(err-expression)
> +/// yields an error type.
> +Error = 16,
>
>  None = 0,
> -All = 15,
> +All = 31,
>
>  DependentInstantiation = Dependent | Instantiation,
>
> -LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/VariablyModified)
> +LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Error)
>};
>  };
>  using TypeDependence = TypeDependenceScope::TypeDependence;
> @@ -147,6 +149,7 @@ class Dependence {
>  return translate(V, UnexpandedPack, TypeDependence::UnexpandedPack) |
> translate(V, Instantiation, TypeDependence::Instantiation) |
> translate(V, Dependent, TypeDependence::Dependent) |
> +   translate(V, Error, TypeDependence::Error) |
> translate(V, VariablyModified,
> TypeDependence::VariablyModified);
>}
>
>
> diff  --git a/clang/include/clang/AST/Type.h
> b/clang/include/clang/AST/Type.h index 248fbcfba98e..5d2c035ea0fe 100644
> --- a/clang/include/clang/AST/Type.h
> +++ b/clang/include/clang/AST/Type.h
> @@ -2139,6 +2139,11 @@ class alignas(8) Type : public
> ExtQualsTypeCommonBase {
>  return static_cast(TypeBits.Dependence);
>}
>
> +  /// Whether this type is an error type.
> +  bool containsErrors() const {
> +return getDependence() & TypeDependence::Error;  }
> +
>/// Whether this type is a dependent type, meaning that its definition
>/// somehow depends on a template parameter (C++ [temp.dep.type]).
>bool isDependentType() const {
>
> diff  --git a/clang/lib/Parse/ParseExprCXX.cpp
> b/clang/lib/Parse/ParseExprCXX.cpp
> index 761fad9456be..4389c8777c6d 100644
> --- a/clang/lib/Parse/ParseExprCXX.cpp
> +++ b/clang/lib/Parse/ParseExprCXX.cpp
> @@ -3105,10 +3105,14 @@ Parser::ParseCXXNewExpression(bool UseGlobal,
> SourceLocation Start) {
>auto RunSignatureHelp = [&]() {
>  ParsedType TypeRep =
>  Actions.ActOnTypeName(getCurScope(), DeclaratorInfo).get();
> -assert(TypeRep && "invalid types should be handled before");
> -QualType PreferredType = Actions.ProduceConstructorSignatureHelp(
> -getCurScope(), TypeRep.get()->getCanonicalTypeInternal(),
> -DeclaratorInfo.getEndLoc(), ConstructorArgs,
> ConstructorLParen);
> +QualType PreferredType;
> +// ActOnTypeName might adjust DeclaratorInfo and return a null
> type even
> +// the passing DeclaratorInfo is valid, e.g. running
> SignatureHelp on
> +// `new decltype(invalid) (^)`.
> +if (TypeRep)
> +  PreferredType = Actions.ProduceConstructorSignatureHelp(
> +  getCurScope(), TypeRep.get()->getCanonicalTypeInternal(),
> +  DeclaratorInfo.getEndLoc(), ConstructorArgs,

RE: [clang] 6f428e0 - [AST] Fix crashes on decltype(recovery-expr).

2020-04-01 Thread Yung, Douglas via cfe-commits
Hi Haojian,

I noticed that after your change, the compiler is now giving an error when 
trying to create a vector of >= 1024 elements when previously it worked, and 
gcc has no problem with the same code. Is that intentional? I have put the 
details in PR45387, can you take a look?

Douglas Yung

-Original Message-
From: cfe-commits  On Behalf Of Haojian Wu 
via cfe-commits
Sent: Monday, March 30, 2020 5:57
To: cfe-commits@lists.llvm.org
Subject: [clang] 6f428e0 - [AST] Fix crashes on decltype(recovery-expr).


Author: Haojian Wu
Date: 2020-03-30T14:56:33+02:00
New Revision: 6f428e09fbe8ce7e3510ae024031a5fc19653483

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

LOG: [AST] Fix crashes on decltype(recovery-expr).

Summary: We mark these decls as invalid.

Reviewers: sammccall

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/include/clang/AST/DependenceFlags.h
clang/include/clang/AST/Type.h
clang/lib/Parse/ParseExprCXX.cpp
clang/lib/Sema/SemaType.cpp
clang/test/AST/ast-dump-expr-errors.cpp
clang/test/Sema/invalid-member.cpp
clang/unittests/Sema/CodeCompleteTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/DependenceFlags.h 
b/clang/include/clang/AST/DependenceFlags.h
index 75c9aa1656b8..0b24bae6df9b 100644
--- a/clang/include/clang/AST/DependenceFlags.h
+++ b/clang/include/clang/AST/DependenceFlags.h
@@ -50,14 +50,16 @@ struct TypeDependenceScope {
 /// Whether this type is a variably-modified type (C99 6.7.5).
 VariablyModified = 8,
 
-// FIXME: add Error bit.
+/// Whether this type references an error, e.g. decltype(err-expression)
+/// yields an error type.
+Error = 16,
 
 None = 0,
-All = 15,
+All = 31,
 
 DependentInstantiation = Dependent | Instantiation,
 
-LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/VariablyModified)
+LLVM_MARK_AS_BITMASK_ENUM(/*LargestValue=*/Error)
   };
 };
 using TypeDependence = TypeDependenceScope::TypeDependence;
@@ -147,6 +149,7 @@ class Dependence {
 return translate(V, UnexpandedPack, TypeDependence::UnexpandedPack) |
translate(V, Instantiation, TypeDependence::Instantiation) |
translate(V, Dependent, TypeDependence::Dependent) |
+   translate(V, Error, TypeDependence::Error) |
translate(V, VariablyModified, TypeDependence::VariablyModified);
   }
 

diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h 
index 248fbcfba98e..5d2c035ea0fe 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -2139,6 +2139,11 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
 return static_cast(TypeBits.Dependence);
   }
 
+  /// Whether this type is an error type.
+  bool containsErrors() const {
+return getDependence() & TypeDependence::Error;  }
+
   /// Whether this type is a dependent type, meaning that its definition
   /// somehow depends on a template parameter (C++ [temp.dep.type]).
   bool isDependentType() const {

diff  --git a/clang/lib/Parse/ParseExprCXX.cpp 
b/clang/lib/Parse/ParseExprCXX.cpp
index 761fad9456be..4389c8777c6d 100644
--- a/clang/lib/Parse/ParseExprCXX.cpp
+++ b/clang/lib/Parse/ParseExprCXX.cpp
@@ -3105,10 +3105,14 @@ Parser::ParseCXXNewExpression(bool UseGlobal, 
SourceLocation Start) {
   auto RunSignatureHelp = [&]() {
 ParsedType TypeRep =
 Actions.ActOnTypeName(getCurScope(), DeclaratorInfo).get();
-assert(TypeRep && "invalid types should be handled before");
-QualType PreferredType = Actions.ProduceConstructorSignatureHelp(
-getCurScope(), TypeRep.get()->getCanonicalTypeInternal(),
-DeclaratorInfo.getEndLoc(), ConstructorArgs, ConstructorLParen);
+QualType PreferredType;
+// ActOnTypeName might adjust DeclaratorInfo and return a null type 
even
+// the passing DeclaratorInfo is valid, e.g. running SignatureHelp on
+// `new decltype(invalid) (^)`.
+if (TypeRep)
+  PreferredType = Actions.ProduceConstructorSignatureHelp(
+  getCurScope(), TypeRep.get()->getCanonicalTypeInternal(),
+  DeclaratorInfo.getEndLoc(), ConstructorArgs, 
+ ConstructorLParen);
 CalledSignatureHelp = true;
 return PreferredType;
   };

diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 
55ce028fb8c2..e128ebf31270 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -1678,6 +1678,12 @@ static QualType 
ConvertDeclSpecToType(TypeProcessingState ) {
 break;
   }
 
+  // FIXME: we want resulting declarations to be marked invalid, but 
+ claiming  // the type is invalid is