[cfe-commits] r165283 - in /cfe/trunk: include/clang/Basic/DiagnosticGroups.td include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/Sema/parentheses.cpp

[David Blaikie]

Author: dblaikie
Date: Thu Oct  4 19:41:03 2012
New Revision: 165283

URL: http://llvm.org/viewvc/llvm-project?rev=165283&view=rev
Log:
Implement -Wshift-op-parentheses for: a << b + c

This appears to be consistent with GCC's implementation of the same warning
under -Wparentheses. Suppressing a << b + c for cases where 'a' is a user
defined type for compatibility with C++ stream IO. Otherwise suggest
parentheses around the addition or subtraction subexpression.

(this came up when MSVC was complaining (incorrectly, so far as I can tell)
about a perceived violation of this within the LLVM codebase, PR14001)

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Sema/parentheses.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=165283&r1=165282&r2=165283&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Thu Oct  4 19:41:03 2012
@@ -119,6 +119,7 @@
 def : DiagGroup<"idiomatic-parentheses">;
 def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">;
 def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">;
+def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
 def DanglingElse: DiagGroup<"dangling-else">;
 def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">;
 def : DiagGroup<"import">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=165283&r1=165282&r2=165283&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Oct  4 19:41:03 2012
@@ -3863,6 +3863,11 @@
 def note_logical_and_in_logical_or_silence : Note<
   "place parentheses around the '&&' expression to silence this warning">;
 
+def warn_addition_in_bitshift : Warning<
+  "'%0' within '%1'">, InGroup<ShiftOpParentheses>;
+def note_addition_in_bitshift_silence : Note<
+  "place parentheses around the '%0' expression to silence this warning">;
+
 def warn_self_assignment : Warning<
   "explicitly assigning a variable of type %0 to itself">,
   InGroup<SelfAssignment>, DefaultIgnore;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=165283&r1=165282&r2=165283&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Oct  4 19:41:03 2012
@@ -8570,6 +8570,20 @@
   }
 }
 
+static void DiagnoseAdditionInShift(Sema &S, SourceLocation OpLoc,
+                                    Expr *SubExpr, StringRef shift) {
+  if (BinaryOperator *Bop = dyn_cast<BinaryOperator>(SubExpr)) {
+    if (Bop->getOpcode() == BO_Add || Bop->getOpcode() == BO_Sub) {
+      StringRef op = Bop->getOpcode() == BO_Add ? "+" : "-";
+      S.Diag(Bop->getOperatorLoc(), diag::warn_addition_in_bitshift)
+          << Bop->getSourceRange() << OpLoc << op << shift;
+      SuggestParentheses(S, Bop->getOperatorLoc(),
+          S.PDiag(diag::note_addition_in_bitshift_silence) << op,
+          Bop->getSourceRange());
+    }
+  }
+}
+
 /// DiagnoseBinOpPrecedence - Emit warnings for expressions with tricky
 /// precedence.
 static void DiagnoseBinOpPrecedence(Sema &Self, BinaryOperatorKind Opc,
@@ -8591,6 +8605,13 @@
     DiagnoseLogicalAndInLogicalOrLHS(Self, OpLoc, LHSExpr, RHSExpr);
     DiagnoseLogicalAndInLogicalOrRHS(Self, OpLoc, LHSExpr, RHSExpr);
   }
+
+  if ((Opc == BO_Shl && LHSExpr->getType()->isIntegralType(Self.getASTContext()))
+      || Opc == BO_Shr) {
+    StringRef shift = Opc == BO_Shl ? "<<" : ">>";
+    DiagnoseAdditionInShift(Self, OpLoc, LHSExpr, shift);
+    DiagnoseAdditionInShift(Self, OpLoc, RHSExpr, shift);
+  }
 }
 
 // Binary Operators.  'Tok' is the token for the operator.

Modified: cfe/trunk/test/Sema/parentheses.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/parentheses.cpp?rev=165283&r1=165282&r2=165283&view=diff
==============================================================================
--- cfe/trunk/test/Sema/parentheses.cpp (original)
+++ cfe/trunk/test/Sema/parentheses.cpp Thu Oct  4 19:41:03 2012
@@ -22,6 +22,8 @@
   operator int();
   Stream &operator<<(int);
   Stream &operator<<(const char*);
+  Stream &operator>>(int);
+  Stream &operator>>(const char*);
 };
 
 void f(Stream& s, bool b) {
@@ -45,3 +47,13 @@
   // Don't crash on unusual member call expressions.
   (void)((s->*m_ptr)() ? "foo" : "bar");
 }
+
+void test(int a, int b, int c) {
+  (void)(a >> b + c); // expected-warning {{'+' within '>>'}} \
+                         expected-note {{place parentheses around the '+' expression to silence this warning}}
+  (void)(a - b << c); // expected-warning {{'-' within '<<'}} \
+                         expected-note {{place parentheses around the '-' expression to silence this warning}}
+  Stream() << b + c;
+  Stream() >> b + c; // expected-warning {{'+' within '>>'}} \
+                        expected-note {{place parentheses around the '+' expression to silence this warning}}
+}