Re: [clang] 6f428e0 - [AST] Fix crashes on decltype(recovery-expr).
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; > +// A
RE: [clang] 6f428e0 - [AST] Fix crashes on decltype(recovery-expr).
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 &state) { break; } + // FIXME: we want resulting declarations to be marked i
[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 &state) { break; } + // FIXME: we want resulting declarations to be marked invalid, but claiming + // the type is invalid is too strong - e.g. it causes ActOnTypeName to return + // a null type. + if (Result->containsErrors()) +declarator.setInvalidType(); + if (S.getLangOpts().OpenCL && S.checkOpenCLDisabledTypeDeclSpec(DS, Result)) declarator.setInvalidType(true); diff --git a/clang/test/AST/ast-dump-expr-errors.cpp b/clang/test/AST/ast-dump-expr-errors.cpp index e623fad04f4c..9334b73a4354 100644 --- a/clang/test/AST/ast-dump-expr-errors.cpp +++ b/clang/test/AST/ast-dump-expr-errors.cpp @@ -42,5 +42,9 @@ int d = stat