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

Haojian Wu via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 1 12:04:46 PDT 2020


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 at 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 <cfe-commits-bounces at lists.llvm.org> On Behalf Of
> Haojian Wu via cfe-commits
> Sent: Monday, March 30, 2020 5:57
> To: cfe-commits at 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<TypeDependence>(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 = static_cast<int>(bar() + 1);
>
>  // FIXME: store initializer even when 'auto' could not be deduced.
>  // Expressions with errors currently do not keep initializers around.
> -// CHECK:     `-VarDecl {{.*}} invalid e 'auto'
> +// CHECK: -VarDecl {{.*}} invalid e 'auto'
>  auto e = bar();
> +
> +// Error type should result in an invalid decl.
> +// CHECK: -VarDecl {{.*}} invalid f 'decltype(<recovery-expr>(bar))'
> +decltype(bar()) f;
>
> diff  --git a/clang/test/Sema/invalid-member.cpp
> b/clang/test/Sema/invalid-member.cpp
> index 5475157e936e..544979980fc9 100644
> --- a/clang/test/Sema/invalid-member.cpp
> +++ b/clang/test/Sema/invalid-member.cpp
> @@ -1,7 +1,15 @@
> -// RUN: %clang_cc1 -verify -fsyntax-only %s -void foo(); // expected-note
> {{requires 0 arguments}}
> +// RUN: %clang_cc1 -verify -fsyntax-only -fno-recovery-ast %s // RUN:
> +%clang_cc1 -verify -fsyntax-only -frecovery-ast %s
> +
> +void foo(); // expected-note 2{{requires 0 arguments}}
>  class X {
>    decltype(foo(42)) invalid; // expected-error {{no matching function}}
> };  // Should be able to evaluate sizeof without crashing.
>  static_assert(sizeof(X) == 1, "No valid members");
> +
> +class Y {
> +  typeof(foo(42)) invalid; // expected-error {{no matching function}}
> +}; // Should be able to evaluate sizeof without crashing.
> +static_assert(sizeof(Y) == 1, "No valid members");
>
> diff  --git a/clang/unittests/Sema/CodeCompleteTest.cpp
> b/clang/unittests/Sema/CodeCompleteTest.cpp
> index a9441a679cac..d8b303d77bb9 100644
> --- a/clang/unittests/Sema/CodeCompleteTest.cpp
> +++ b/clang/unittests/Sema/CodeCompleteTest.cpp
> @@ -486,7 +486,10 @@ TEST(PreferredTypeTest, NoCrashOnInvalidTypes) {
>    StringRef Code = R"cpp(
>      auto x = decltype(&1)(^);
>      auto y = new decltype(&1)(^);
> +    // GNU decimal type extension is not supported in clang.
> +    auto z = new _Decimal128(^);
>    )cpp";
>    EXPECT_THAT(collectPreferredTypes(Code), Each("NULL TYPE"));  }
> +
>  } // namespace
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20200401/d8bd4b3b/attachment.html>


More information about the cfe-commits mailing list