Author: Richard Smith Date: 2020-08-28T13:35:50-07:00 New Revision: 0e00a95b4fad5e72851de012d3a0b2c2d01f8685
URL: https://github.com/llvm/llvm-project/commit/0e00a95b4fad5e72851de012d3a0b2c2d01f8685 DIFF: https://github.com/llvm/llvm-project/commit/0e00a95b4fad5e72851de012d3a0b2c2d01f8685.diff LOG: Add new warning for compound punctuation tokens that are split across macro expansions or split by whitespace. For example: #define FOO(x) (x) FOO({}); ... forms a statement-expression after macro expansion. This warning applies to '({' and '})' delimiting statement-expressions, '[[' and ']]' delimiting attributes, and '::*' introducing a pointer-to-member. The warning for forming these compound tokens across macro expansions (or across files!) is enabled by default; the warning for whitespace within the tokens is not, but is included in -Wall. Differential Revision: https://reviews.llvm.org/D86751 Added: clang/test/Parser/compound-token-split.cpp Modified: clang/include/clang/Basic/DiagnosticGroups.td clang/include/clang/Basic/DiagnosticParseKinds.td clang/include/clang/Parse/Parser.h clang/lib/Headers/altivec.h clang/lib/Parse/ParseDecl.cpp clang/lib/Parse/ParseDeclCXX.cpp clang/lib/Parse/ParseExpr.cpp clang/lib/Parse/ParseStmt.cpp clang/lib/Parse/Parser.cpp clang/test/Misc/warning-wall.c Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 10a5c90c960e..a79e057a5b33 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -45,6 +45,11 @@ def GNUCompoundLiteralInitializer : DiagGroup<"gnu-compound-literal-initializer" def BitFieldConstantConversion : DiagGroup<"bitfield-constant-conversion">; def BitFieldEnumConversion : DiagGroup<"bitfield-enum-conversion">; def BitFieldWidth : DiagGroup<"bitfield-width">; +def CompoundTokenSplitByMacro : DiagGroup<"compound-token-split-by-macro">; +def CompoundTokenSplitBySpace : DiagGroup<"compound-token-split-by-space">; +def CompoundTokenSplit : DiagGroup<"compound-token-split", + [CompoundTokenSplitByMacro, + CompoundTokenSplitBySpace]>; def CoroutineMissingUnhandledException : DiagGroup<"coroutine-missing-unhandled-exception">; def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException]>; @@ -943,7 +948,8 @@ def Consumed : DiagGroup<"consumed">; // Note that putting warnings in -Wall will not disable them by default. If a // warning should be active _only_ when -Wall is passed in, mark it as // DefaultIgnore in addition to putting it here. -def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool, MisleadingIndentation]>; +def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool, + MisleadingIndentation, CompoundTokenSplit]>; // Warnings that should be in clang-cl /w4. def : DiagGroup<"CL4", [All, Extra]>; diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td index 08b91de31993..42da8bbad74f 100644 --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/clang/include/clang/Basic/DiagnosticParseKinds.td @@ -62,6 +62,23 @@ def warn_misleading_indentation : Warning< def note_previous_statement : Note< "previous statement is here">; +def subst_compound_token_kind : TextSubstitution< + "%select{%1 and |}0%2 tokens " + "%select{introducing statement expression|terminating statement expression|" + "introducing attribute|terminating attribute|" + "forming pointer to member type}3">; +def warn_compound_token_split_by_macro : Warning< + "%sub{subst_compound_token_kind}0,1,2,3 appear in diff erent " + "macro expansion contexts">, InGroup<CompoundTokenSplitByMacro>; +def warn_compound_token_split_by_file : Warning< + "%sub{subst_compound_token_kind}0,1,2,3 appear in diff erent source files">, + InGroup<CompoundTokenSplit>; +def note_compound_token_split_second_token_here : Note< + "%select{|second }0%1 token is here">; +def warn_compound_token_split_by_whitespace : Warning< + "%sub{subst_compound_token_kind}0,1,2,3 are separated by whitespace">, + InGroup<CompoundTokenSplitBySpace>, DefaultIgnore; + def ext_thread_before : Extension<"'__thread' before '%0'">; def ext_keyword_as_ident : ExtWarn< "keyword '%0' will be made available as an identifier " diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h index 4068e6a444c9..37ca9e893329 100644 --- a/clang/include/clang/Parse/Parser.h +++ b/clang/include/clang/Parse/Parser.h @@ -1045,6 +1045,25 @@ class Parser : public CodeCompletionHandler { /// was successful. bool expectIdentifier(); + /// Kinds of compound pseudo-tokens formed by a sequence of two real tokens. + enum class CompoundToken { + /// A '(' '{' beginning a statement-expression. + StmtExprBegin, + /// A '}' ')' ending a statement-expression. + StmtExprEnd, + /// A '[' '[' beginning a C++11 or C2x attribute. + AttrBegin, + /// A ']' ']' ending a C++11 or C2x attribute. + AttrEnd, + /// A '::' '*' forming a C++ pointer-to-member declaration. + MemberPtr, + }; + + /// Check that a compound operator was written in a "sensible" way, and warn + /// if not. + void checkCompoundToken(SourceLocation FirstTokLoc, + tok::TokenKind FirstTokKind, CompoundToken Op); + public: //===--------------------------------------------------------------------===// // Scope manipulation diff --git a/clang/lib/Headers/altivec.h b/clang/lib/Headers/altivec.h index 5bc0e23792d4..927f25751664 100644 --- a/clang/lib/Headers/altivec.h +++ b/clang/lib/Headers/altivec.h @@ -3324,23 +3324,19 @@ static __inline__ void __attribute__((__always_inline__)) vec_dssall(void) { /* vec_dst */ #define vec_dst(__PTR, __CW, __STR) \ - __extension__( \ - { __builtin_altivec_dst((const void *)(__PTR), (__CW), (__STR)); }) + __builtin_altivec_dst((const void *)(__PTR), (__CW), (__STR)) /* vec_dstst */ #define vec_dstst(__PTR, __CW, __STR) \ - __extension__( \ - { __builtin_altivec_dstst((const void *)(__PTR), (__CW), (__STR)); }) + __builtin_altivec_dstst((const void *)(__PTR), (__CW), (__STR)) /* vec_dststt */ #define vec_dststt(__PTR, __CW, __STR) \ - __extension__( \ - { __builtin_altivec_dststt((const void *)(__PTR), (__CW), (__STR)); }) + __builtin_altivec_dststt((const void *)(__PTR), (__CW), (__STR)) /* vec_dstt */ #define vec_dstt(__PTR, __CW, __STR) \ - __extension__( \ - { __builtin_altivec_dstt((const void *)(__PTR), (__CW), (__STR)); }) + __builtin_altivec_dstt((const void *)(__PTR), (__CW), (__STR)) /* vec_eqv */ diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index 7b3a98edb372..38df6be17efe 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -5591,6 +5591,11 @@ void Parser::ParseDeclaratorInternal(Declarator &D, return; } + if (SS.isValid()) { + checkCompoundToken(SS.getEndLoc(), tok::coloncolon, + CompoundToken::MemberPtr); + } + SourceLocation StarLoc = ConsumeToken(); D.SetRangeEnd(StarLoc); DeclSpec DS(AttrFactory); diff --git a/clang/lib/Parse/ParseDeclCXX.cpp b/clang/lib/Parse/ParseDeclCXX.cpp index d2cc089c98eb..75bb78152e57 100644 --- a/clang/lib/Parse/ParseDeclCXX.cpp +++ b/clang/lib/Parse/ParseDeclCXX.cpp @@ -4142,9 +4142,11 @@ void Parser::ParseCXX11AttributeSpecifier(ParsedAttributes &attrs, assert(Tok.is(tok::l_square) && NextToken().is(tok::l_square) && "Not a double square bracket attribute list"); - Diag(Tok.getLocation(), diag::warn_cxx98_compat_attribute); + SourceLocation OpenLoc = Tok.getLocation(); + Diag(OpenLoc, diag::warn_cxx98_compat_attribute); ConsumeBracket(); + checkCompoundToken(OpenLoc, tok::l_square, CompoundToken::AttrBegin); ConsumeBracket(); SourceLocation CommonScopeLoc; @@ -4227,8 +4229,11 @@ void Parser::ParseCXX11AttributeSpecifier(ParsedAttributes &attrs, << AttrName; } + SourceLocation CloseLoc = Tok.getLocation(); if (ExpectAndConsume(tok::r_square)) SkipUntil(tok::r_square); + else if (Tok.is(tok::r_square)) + checkCompoundToken(CloseLoc, tok::r_square, CompoundToken::AttrEnd); if (endLoc) *endLoc = Tok.getLocation(); if (ExpectAndConsume(tok::r_square)) diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp index a6d64cbb7507..c31757ed848d 100644 --- a/clang/lib/Parse/ParseExpr.cpp +++ b/clang/lib/Parse/ParseExpr.cpp @@ -2840,6 +2840,8 @@ Parser::ParseParenExpression(ParenParseOption &ExprType, bool stopIfCastExpr, if (ExprType >= CompoundStmt && Tok.is(tok::l_brace)) { Diag(Tok, diag::ext_gnu_statement_expr); + checkCompoundToken(OpenLoc, tok::l_paren, CompoundToken::StmtExprBegin); + if (!getCurScope()->getFnParent() && !getCurScope()->getBlockParent()) { Result = ExprError(Diag(OpenLoc, diag::err_stmtexpr_file_scope)); } else { diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index 82b0a6562820..d017842e7754 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -1135,9 +1135,17 @@ StmtResult Parser::ParseCompoundStatementBody(bool isStmtExpr) { SourceLocation CloseLoc = Tok.getLocation(); // We broke out of the while loop because we found a '}' or EOF. - if (!T.consumeClose()) + if (!T.consumeClose()) { + // If this is the '})' of a statement expression, check that it's written + // in a sensible way. + if (isStmtExpr && Tok.is(tok::r_paren)) + checkCompoundToken(CloseLoc, tok::r_brace, CompoundToken::StmtExprEnd); + } else { // Recover by creating a compound statement with what we parsed so far, - // instead of dropping everything and returning StmtError(); + // instead of dropping everything and returning StmtError(). + } + + if (T.getCloseLocation().isValid()) CloseLoc = T.getCloseLocation(); return Actions.ActOnCompoundStmt(T.getOpenLocation(), CloseLoc, diff --git a/clang/lib/Parse/Parser.cpp b/clang/lib/Parse/Parser.cpp index 5e485eda831c..70e6e74ade89 100644 --- a/clang/lib/Parse/Parser.cpp +++ b/clang/lib/Parse/Parser.cpp @@ -227,6 +227,39 @@ bool Parser::expectIdentifier() { return true; } +void Parser::checkCompoundToken(SourceLocation FirstTokLoc, + tok::TokenKind FirstTokKind, CompoundToken Op) { + if (FirstTokLoc.isInvalid()) + return; + SourceLocation SecondTokLoc = Tok.getLocation(); + + // We expect both tokens to come from the same FileID. + FileID FirstID = PP.getSourceManager().getFileID(FirstTokLoc); + FileID SecondID = PP.getSourceManager().getFileID(SecondTokLoc); + if (FirstID != SecondID) { + Diag(FirstTokLoc, (FirstTokLoc.isMacroID() || SecondTokLoc.isMacroID()) + ? diag::warn_compound_token_split_by_macro + : diag::warn_compound_token_split_by_file) + << (FirstTokKind == Tok.getKind()) << FirstTokKind << Tok.getKind() + << static_cast<int>(Op) << SourceRange(FirstTokLoc); + Diag(SecondTokLoc, diag::note_compound_token_split_second_token_here) + << (FirstTokKind == Tok.getKind()) << Tok.getKind() + << SourceRange(SecondTokLoc); + return; + } + + // We expect the tokens to abut. + if (Tok.hasLeadingSpace()) { + SourceLocation SpaceLoc = PP.getLocForEndOfToken(FirstTokLoc); + if (SpaceLoc.isInvalid()) + SpaceLoc = FirstTokLoc; + Diag(SpaceLoc, diag::warn_compound_token_split_by_whitespace) + << (FirstTokKind == Tok.getKind()) << FirstTokKind << Tok.getKind() + << static_cast<int>(Op) << SourceRange(FirstTokLoc, SecondTokLoc); + return; + } +} + //===----------------------------------------------------------------------===// // Error recovery. //===----------------------------------------------------------------------===// diff --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c index c63d4beecff0..6e1134572a3c 100644 --- a/clang/test/Misc/warning-wall.c +++ b/clang/test/Misc/warning-wall.c @@ -94,6 +94,9 @@ CHECK-NEXT: -Wdangling-else CHECK-NEXT: -Wswitch CHECK-NEXT: -Wswitch-bool CHECK-NEXT: -Wmisleading-indentation +CHECK-NEXT: -Wcompound-token-split +CHECK-NEXT: -Wcompound-token-split-by-macro +CHECK-NEXT: -Wcompound-token-split-by-space CHECK-NOT:-W diff --git a/clang/test/Parser/compound-token-split.cpp b/clang/test/Parser/compound-token-split.cpp new file mode 100644 index 000000000000..2ec7955658de --- /dev/null +++ b/clang/test/Parser/compound-token-split.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 %s -verify +// RUN: %clang_cc1 %s -verify=expected,space -Wcompound-token-split +// RUN: %clang_cc1 %s -verify=expected,space -Wall + +#ifdef LSQUARE +[ // expected-note {{second '[' token is here}} +#else + +#define VAR(type, name, init) type name = (init) + +void f() { + VAR(int, x, {}); // #1 + // expected-warning@#1 {{'(' and '{' tokens introducing statement expression appear in diff erent macro expansion contexts}} + // expected-note-re@#1 {{{{^}}'{' token is here}} + // + // FIXME: It would be nice to suppress this when we already warned about the opening '({'. + // expected-warning@#1 {{'}' and ')' tokens terminating statement expression appear in diff erent macro expansion contexts}} + // expected-note-re@#1 {{{{^}}')' token is here}} + // + // expected-error@#1 {{cannot initialize a variable of type 'int' with an rvalue of type 'void'}} +} + +#define RPAREN ) + +int f2() { + int n = ({ 1; }RPAREN; // expected-warning {{'}' and ')' tokens terminating statement expression appear in diff erent macro expansion contexts}} expected-note {{')' token is here}} + return n; +} + +[ // expected-warning-re {{{{^}}'[' tokens introducing attribute appear in diff erent source files}} +#define LSQUARE +#include __FILE__ + noreturn ]] void g(); + +[[noreturn] ] void h(); // space-warning-re {{{{^}}']' tokens terminating attribute are separated by whitespace}} + +struct X {}; +int X:: *p; // space-warning {{'::' and '*' tokens forming pointer to member type are separated by whitespace}} + +#endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
