Do you have some data on the true/false positive rate for this warning? On Fri, Sep 23, 2016 at 6:12 AM Daniel Marjamäki < daniel.marjam...@evidente.se> wrote:
> danielmarjamaki created this revision. > danielmarjamaki added reviewers: dblaikie, rtrieu. > danielmarjamaki added a subscriber: cfe-commits. > danielmarjamaki set the repository for this revision to rL LLVM. > > This patch makes Clang warn about following code: > > a = (b * c >> 2); > > It might be a good idea to use parentheses to clarify the operator > precedence. > > > Repository: > rL LLVM > > https://reviews.llvm.org/D24861 > > Files: > lib/Sema/SemaExpr.cpp > test/Sema/parentheses.cpp > > Index: test/Sema/parentheses.cpp > =================================================================== > --- test/Sema/parentheses.cpp > +++ test/Sema/parentheses.cpp > @@ -95,6 +95,21 @@ > // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:"(" > // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:15}:")" > > + (void)(a >> b * c); // expected-warning {{operator '>>' has lower > precedence than '*'; '*' will be evaluated first}} \ > + expected-note {{place parentheses around the '*' > expression to silence this warning}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"(" > + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:20-[[@LINE-3]]:20}:")" > + > + (void)(a / b << c); // expected-warning {{operator '<<' has lower > precedence than '/'; '/' will be evaluated first}} \ > + expected-note {{place parentheses around the '/' > expression to silence this warning}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:"(" > + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:15}:")" > + > + (void)(a % b << c); // expected-warning {{operator '<<' has lower > precedence than '%'; '%' will be evaluated first}} \ > + expected-note {{place parentheses around the '%' > expression to silence this warning}} > + // CHECK: fix-it:"{{.*}}":{[[@LINE-2]]:10-[[@LINE-2]]:10}:"(" > + // CHECK: fix-it:"{{.*}}":{[[@LINE-3]]:15-[[@LINE-3]]:15}:")" > + > Stream() << b + c; > Stream() >> b + c; // expected-warning {{operator '>>' has lower > precedence than '+'; '+' will be evaluated first}} \ > expected-note {{place parentheses around the '+' > expression to silence this warning}} > Index: lib/Sema/SemaExpr.cpp > =================================================================== > --- lib/Sema/SemaExpr.cpp > +++ lib/Sema/SemaExpr.cpp > @@ -11246,10 +11246,10 @@ > } > } > > -static void DiagnoseAdditionInShift(Sema &S, SourceLocation OpLoc, > +static void DiagnoseAddOrMulInShift(Sema &S, SourceLocation OpLoc, > Expr *SubExpr, StringRef Shift) { > if (BinaryOperator *Bop = dyn_cast<BinaryOperator>(SubExpr)) { > - if (Bop->getOpcode() == BO_Add || Bop->getOpcode() == BO_Sub) { > + if (Bop->isAdditiveOp() || Bop->isMultiplicativeOp()) { > StringRef Op = Bop->getOpcodeStr(); > S.Diag(Bop->getOperatorLoc(), diag::warn_addition_in_bitshift) > << Bop->getSourceRange() << OpLoc << Shift << Op; > @@ -11313,8 +11313,8 @@ > if ((Opc == BO_Shl && > LHSExpr->getType()->isIntegralType(Self.getASTContext())) > || Opc == BO_Shr) { > StringRef Shift = BinaryOperator::getOpcodeStr(Opc); > - DiagnoseAdditionInShift(Self, OpLoc, LHSExpr, Shift); > - DiagnoseAdditionInShift(Self, OpLoc, RHSExpr, Shift); > + DiagnoseAddOrMulInShift(Self, OpLoc, LHSExpr, Shift); > + DiagnoseAddOrMulInShift(Self, OpLoc, RHSExpr, Shift); > } > > // Warn on overloaded shift operators and comparisons, such as: > > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits