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

[Jordan Rose]

I notice you used a diagnostic format specifier to handle '+' vs. '-'. Can the "silence this warning" notes be combined for all the *-op-parentheses warnings?


On Oct 4, 2012, at 17:41 , David Blaikie <dblaikie at gmail.com> wrote:

> 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}}
> +}
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits