Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -5232,6 +5232,9 @@ def warn_addition_in_bitshift : Warning< "operator '%0' has lower precedence than '%1'; " "'%1' will be evaluated first">, InGroup; +def warn_multiplication_in_bitshift : Warning< + "operator '%0' has lower precedence than '%1'; " + "'%1' will be evaluated first">, InGroup>, DefaultIgnore; def warn_self_assignment : Warning< "explicitly assigning value of variable of type %0 to itself">, Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -11263,18 +11263,26 @@ } } -static void DiagnoseAdditionInShift(Sema &S, SourceLocation OpLoc, - Expr *SubExpr, StringRef Shift) { - if (BinaryOperator *Bop = dyn_cast(SubExpr)) { - if (Bop->getOpcode() == BO_Add || Bop->getOpcode() == BO_Sub) { - StringRef Op = Bop->getOpcodeStr(); - S.Diag(Bop->getOperatorLoc(), diag::warn_addition_in_bitshift) - << Bop->getSourceRange() << OpLoc << Shift << Op; - SuggestParentheses(S, Bop->getOperatorLoc(), - S.PDiag(diag::note_precedence_silence) << Op, - Bop->getSourceRange()); - } - } +static void DiagnoseAddOrMulInShift(Sema &S, SourceLocation OpLoc, + Expr *SubExpr, StringRef Shift, + bool ShiftLeftLHS) { + BinaryOperator *Bop = dyn_cast(SubExpr); + if (!Bop) + return; + if (!Bop->isAdditiveOp() && !Bop->isMultiplicativeOp()) + return; + // In many cases, execution order does not matter for 'A*B<getOpcode() == BO_Mul) + return; + + StringRef Op = Bop->getOpcodeStr(); + S.Diag(Bop->getOperatorLoc(), Bop->isAdditiveOp() + ? diag::warn_addition_in_bitshift + : diag::warn_multiplication_in_bitshift) + << Bop->getSourceRange() << OpLoc << Shift << Op; + SuggestParentheses(S, Bop->getOperatorLoc(), + S.PDiag(diag::note_precedence_silence) << Op, + Bop->getSourceRange()); } static void DiagnoseShiftCompare(Sema &S, SourceLocation OpLoc, @@ -11330,8 +11338,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, Opc == BO_Shl); + DiagnoseAddOrMulInShift(Self, OpLoc, RHSExpr, Shift, false); } // Warn on overloaded shift operators and comparisons, such as: Index: test/Sema/parentheses.cpp =================================================================== --- test/Sema/parentheses.cpp +++ test/Sema/parentheses.cpp @@ -1,5 +1,5 @@ -// RUN: %clang_cc1 -Wparentheses -fsyntax-only -verify %s -// RUN: %clang_cc1 -Wparentheses -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s +// RUN: %clang_cc1 -Wparentheses -Wshift-op-parentheses-mul -fsyntax-only -verify %s +// RUN: %clang_cc1 -Wparentheses -Wshift-op-parentheses-mul -fsyntax-only -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s bool someConditionFunc(); @@ -95,6 +95,23 @@ // 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}:")" + + (void)(a * b << c); // no warning, often the execution order does not matter. + 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}}