[PATCH] D24861: [Sema] extend Wshift-op-parentheses so it warns for multiplicative operators
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 23 12:03:10 PDT 2016
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.marjamaki at 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:
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160923/192119df/attachment.html>
More information about the cfe-commits
mailing list