[clang] 0e00a95 - Add new warning for compound punctuation tokens that are split across macro expansions or split by whitespace.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Fri Aug 28 13:36:10 PDT 2020
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
More information about the cfe-commits
mailing list