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 <[email protected]> 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 > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
