On Fri, Oct 19, 2012 at 11:30 AM, David Blaikie <[email protected]> wrote: > On Thu, Oct 18, 2012 at 6:16 PM, Matt Beaumont-Gay <[email protected]> > wrote: >> A little belated bikeshed-painting... >> >> On Thu, Oct 4, 2012 at 5:41 PM, David Blaikie <[email protected]> wrote: >>> 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>; >> >> I have a hard time understanding what problem this warning is trying >> to explain. Maybe we could phrase it like our other shift operator >> precedence warning: >> operator '?:' has lower precedence than '<<'; '<<' will be evaluated >> first [-Werror,-Wparentheses] > > Fair point, improved as suggested in r166296. (pity we can't reuse the > text of a warning because it's associated with a particular flag)
Thanks! > > I got the wording from the '||'+'&&' and '|'+'&' warning above this > one. Should we change these too? SGTM, but I don't have particularly strong feelings about it. > >>> +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) { >> >> Also, out of curiosity, why not warn on multiply/divide/mod? > > It's just what GCC supports. We could evaluate whether > multiply/divide/mod are sufficiently error prone as to be worth > diagnosing. Given past observation of people not understanding the precedence of <<, I'd say it's worth checking out. _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
