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

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]>;

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 different "
+  "macro expansion contexts">, InGroup<CompoundTokenSplitByMacro>;
+def warn_compound_token_split_by_file : Warning<
+  "%sub{subst_compound_token_kind}0,1,2,3 appear in different 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 "

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

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 */
 

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);

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))

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 {

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,

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.
 //===----------------------------------------------------------------------===//

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

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 different 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 different 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 different macro expansion contexts}} expected-note {{')' token is here}}
+  return n;
+}
+
+[ // expected-warning-re {{{{^}}'[' tokens introducing attribute appear in different 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