From 4d1b7e9820ee9c87541619ce4dd41e92dc43cd9c Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 29 Jun 2020 14:12:14 -0700 Subject: [PATCH] Fix a few cases that were incorrectly parsed as unary-expressions instead of postfix-expressions, and improve error recovery for postfix operators after unary-expressions. This covers nullptr, __null, and some calls to type traits with special parsing rules. We would previously not parse a postfix-expression suffix for these expressions, so would reject expressions such as __is_trivial(int)["foo"]. For the case where a postfix-expression suffix is *not* permitted after a unary-expression (for example, after a new-expression or sizeof expression), produce a diagnostic if one appears there anyway. That's always ill-formed, but previously produced very bad diagnostics. --- clang/include/clang/Basic/DiagnosticParseKinds.td | 2 + clang/lib/Parse/ParseExpr.cpp | 108 +++++++++++++++++----- clang/test/Parser/expressions.cpp | 38 ++++++++ 3 files changed, 126 insertions(+), 22 deletions(-) create mode 100644 clang/test/Parser/expressions.cpp diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 5d57cfd..f5b32a6 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -194,6 +194,8 @@ def err_function_declared_typedef : Error< def err_at_defs_cxx : Error<"@defs is not supported in Objective-C++">; def err_at_in_class : Error<"unexpected '@' in member specification">; def err_unexpected_semi : Error<"unexpected ';' before %0">; +def err_postfix_after_unary_requires_parens : Error< + "expression cannot be followed by a postfix %0 operator; add parentheses">; def err_unparenthesized_non_primary_expr_in_requires_clause : Error< "parentheses are required around this expression in a requires clause">; def note_unparenthesized_non_primary_expr_in_requires_clause : Note< diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp index bd00320..bfc465f8 100644 --- a/clang/lib/Parse/ParseExpr.cpp +++ b/clang/lib/Parse/ParseExpr.cpp @@ -920,6 +920,11 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, auto SavedType = PreferredType; NotCastExpr = false; + // Are postfix-expression suffix operators permitted after this + // cast-expression? If not, and we find some, we'll parse them anyway and + // diagnose them. + bool AllowSuffix = true; + // This handles all of cast-expression, unary-expression, postfix-expression, // and primary-expression. We handle them together like this for efficiency // and to simplify handling of an expression starting with a '(' token: which @@ -929,8 +934,7 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, // If the parsed tokens consist of a primary-expression, the cases below // break out of the switch; at the end we call ParsePostfixExpressionSuffix // to handle the postfix expression suffixes. Cases that cannot be followed - // by postfix exprs should return without invoking - // ParsePostfixExpressionSuffix. + // by postfix exprs should set AllowSuffix to false. switch (SavedKind) { case tok::l_paren: { // If this expression is limited to being a unary-expression, the paren can @@ -953,8 +957,11 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, Res = ParseParenExpression(ParenExprType, false/*stopIfCastExr*/, isTypeCast == IsTypeCast, CastTy, RParenLoc); + // FIXME: What should we do if a vector literal is followed by a + // postfix-expression suffix? Usually postfix operators are permitted on + // literals. if (isVectorLiteral) - return Res; + return Res; switch (ParenExprType) { case SimpleExpr: break; // Nothing else to do. @@ -992,11 +999,13 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, case tok::kw___objc_yes: case tok::kw___objc_no: - return ParseObjCBoolLiteral(); + Res = ParseObjCBoolLiteral(); + break; case tok::kw_nullptr: Diag(Tok, diag::warn_cxx98_compat_nullptr); - return Actions.ActOnCXXNullPtrLiteral(ConsumeToken()); + Res = Actions.ActOnCXXNullPtrLiteral(ConsumeToken()); + break; case tok::annot_uneval_primary_expr: case tok::annot_primary_expr: @@ -1295,7 +1304,8 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, Res = ParseGenericSelectionExpression(); break; case tok::kw___builtin_available: - return ParseAvailabilityCheckExpr(Tok.getLocation()); + Res = ParseAvailabilityCheckExpr(Tok.getLocation()); + break; case tok::kw___builtin_va_arg: case tok::kw___builtin_offsetof: case tok::kw___builtin_choose_expr: @@ -1307,9 +1317,11 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, case tok::kw___builtin_LINE: if (NotPrimaryExpression) *NotPrimaryExpression = true; + // This parses the complete suffix; we can return early. return ParseBuiltinPrimaryExpression(); case tok::kw___null: - return Actions.ActOnGNUNullExpr(ConsumeToken()); + Res = Actions.ActOnGNUNullExpr(ConsumeToken()); + break; case tok::plusplus: // unary-expression: '++' unary-expression [C99] case tok::minusminus: { // unary-expression: '--' unary-expression [C99] @@ -1421,7 +1433,9 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, case tok::kw___builtin_omp_required_simd_align: if (NotPrimaryExpression) *NotPrimaryExpression = true; - return ParseUnaryExprOrTypeTraitExpression(); + AllowSuffix = false; + Res = ParseUnaryExprOrTypeTraitExpression(); + break; case tok::ampamp: { // unary-expression: '&&' identifier if (NotPrimaryExpression) *NotPrimaryExpression = true; @@ -1437,7 +1451,8 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, Tok.getLocation()); Res = Actions.ActOnAddrLabel(AmpAmpLoc, Tok.getLocation(), LD); ConsumeToken(); - return Res; + AllowSuffix = false; + break; } case tok::kw_const_cast: case tok::kw_dynamic_cast: @@ -1632,12 +1647,16 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, if (Tok.is(tok::kw_new)) { if (NotPrimaryExpression) *NotPrimaryExpression = true; - return ParseCXXNewExpression(true, CCLoc); + Res = ParseCXXNewExpression(true, CCLoc); + AllowSuffix = false; + break; } if (Tok.is(tok::kw_delete)) { if (NotPrimaryExpression) *NotPrimaryExpression = true; - return ParseCXXDeleteExpression(true, CCLoc); + Res = ParseCXXDeleteExpression(true, CCLoc); + AllowSuffix = false; + break; } // This is not a type name or scope specifier, it is an invalid expression. @@ -1648,15 +1667,21 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, case tok::kw_new: // [C++] new-expression if (NotPrimaryExpression) *NotPrimaryExpression = true; - return ParseCXXNewExpression(false, Tok.getLocation()); + Res = ParseCXXNewExpression(false, Tok.getLocation()); + AllowSuffix = false; + break; case tok::kw_delete: // [C++] delete-expression if (NotPrimaryExpression) *NotPrimaryExpression = true; - return ParseCXXDeleteExpression(false, Tok.getLocation()); + Res = ParseCXXDeleteExpression(false, Tok.getLocation()); + AllowSuffix = false; + break; case tok::kw_requires: // [C++2a] requires-expression - return ParseRequiresExpression(); + Res = ParseRequiresExpression(); + AllowSuffix = false; + break; case tok::kw_noexcept: { // [C++0x] 'noexcept' '(' expression ')' if (NotPrimaryExpression) @@ -1672,32 +1697,36 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, // which is an unevaluated operand, can throw an exception. EnterExpressionEvaluationContext Unevaluated( Actions, Sema::ExpressionEvaluationContext::Unevaluated); - ExprResult Result = ParseExpression(); + Res = ParseExpression(); T.consumeClose(); - if (!Result.isInvalid()) - Result = Actions.ActOnNoexceptExpr(KeyLoc, T.getOpenLocation(), - Result.get(), T.getCloseLocation()); - return Result; + if (!Res.isInvalid()) + Res = Actions.ActOnNoexceptExpr(KeyLoc, T.getOpenLocation(), Res.get(), + T.getCloseLocation()); + AllowSuffix = false; + break; } #define TYPE_TRAIT(N,Spelling,K) \ case tok::kw_##Spelling: #include "clang/Basic/TokenKinds.def" - return ParseTypeTrait(); + Res = ParseTypeTrait(); + break; case tok::kw___array_rank: case tok::kw___array_extent: if (NotPrimaryExpression) *NotPrimaryExpression = true; - return ParseArrayTypeTrait(); + Res = ParseArrayTypeTrait(); + break; case tok::kw___is_lvalue_expr: case tok::kw___is_rvalue_expr: if (NotPrimaryExpression) *NotPrimaryExpression = true; - return ParseExpressionTrait(); + Res = ParseExpressionTrait(); + break; case tok::at: { if (NotPrimaryExpression) @@ -1754,6 +1783,41 @@ ExprResult Parser::ParseCastExpression(CastParseKind ParseKind, // parsed. return Res; + if (!AllowSuffix) { + // FIXME: Don't parse a primary-expression suffix if we encountered a parse + // error already. + if (Res.isInvalid()) + return Res; + + switch (Tok.getKind()) { + case tok::l_square: + case tok::l_paren: + case tok::plusplus: + case tok::minusminus: + // "expected ';'" or similar is probably the right diagnostic here. Let + // the caller decide what to do. + if (Tok.isAtStartOfLine()) + return Res; + + LLVM_FALLTHROUGH; + case tok::period: + case tok::arrow: + break; + + default: + return Res; + } + + // This was a unary-expression for which a postfix-expression suffix is + // not permitted by the grammar (eg, a sizeof expression or + // new-expression or similar). Diagnose but parse the suffix anyway. + Diag(Tok.getLocation(), diag::err_postfix_after_unary_requires_parens) + << Tok.getKind() << Res.get()->getSourceRange() + << FixItHint::CreateInsertion(Res.get()->getBeginLoc(), "(") + << FixItHint::CreateInsertion(PP.getLocForEndOfToken(PrevTokLocation), + ")"); + } + // These can be followed by postfix-expr pieces. PreferredType = SavedType; Res = ParsePostfixExpressionSuffix(Res); diff --git a/clang/test/Parser/expressions.cpp b/clang/test/Parser/expressions.cpp new file mode 100644 index 0000000..6d3123b --- /dev/null +++ b/clang/test/Parser/expressions.cpp @@ -0,0 +1,38 @@ +// RUN: %clang_cc1 -std=c++2a -verify %s + +void *operator new(__SIZE_TYPE__, void*); + +// Check that we give a good diagnostic for an attempt to use a postfix +// operator after a unary-expression. +namespace postfix_after_unary { + struct A { int n; }; + int &a = new A->n; // expected-error {{expression cannot be followed by a postfix '->' operator; add parentheses}} + + struct B { B(int); int operator()(int); }; + int n = new (0) (B) (int()) (int()); // expected-error {{cannot be followed by a postfix '(}} expected-error {{not a function or function pointer}} + + char x = sizeof(int)["hello"]; // expected-error {{cannot be followed by a postfix '[}} + char y = alignof(int)["hello"]; // expected-error {{cannot be followed by a postfix '[}} + char z = noexcept(0)["hello"]; // expected-error {{cannot be followed by a postfix '[}} + char w = requires { x == x; }["ny"]; // expected-error {{cannot be followed by a postfix '[}} + + int f() { + label: + return &&label->n; // expected-error {{cannot be followed by a postfix}} expected-error {{not a structure or union}} + } + + char k = sizeof(int) // expected-error {{expected ';'}} + [[noreturn]] void g(); +} + +// Check that we do parse postfix-expression suffixes after some more unusual +// kinds of postfix-expressions (null literals and builtin calls). +namespace unusual_primary_exprs { + int a = nullptr["foo"]; // expected-error {{array subscript is not an integer}} + int b = __builtin_COLUMN()["sufficiently long string constant"]; + int c = __builtin_available(*)["ny"]; + static_assert(__null["foo"] == 'f'); // FIXME: Warn about converting __null to integer in array subscripting. + static_assert(__is_standard_layout(int)["ny"] == 'y'); + static_assert(__array_rank(int[1][2])["0123"] == '2'); + static_assert(__is_lvalue_expr(a)["ny"] == 'y'); +} -- 2.7.4