diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -61,6 +61,9 @@ operation and a constant. The group also has the new warning which diagnoses when a bitwise-or with a non-negative value is converted to a bool, since that bool will always be true. +- -Wbitwise-conditional-parentheses will warn on operator precedence issues + when mixing bitwise-and (&) and bitwise-or (|) operator with the + conditional operator (?:). Non-comprehensive list of changes in this release ------------------------------------------------- diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -296,6 +296,7 @@ def FlexibleArrayExtensions : DiagGroup<"flexible-array-extensions">; def FourByteMultiChar : DiagGroup<"four-char-constants">; def GlobalConstructors : DiagGroup<"global-constructors">; +def BitwiseConditionalParentheses: DiagGroup<"bitwise-conditional-parentheses">; def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">; def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">; def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">; @@ -737,6 +738,7 @@ def Parentheses : DiagGroup<"parentheses", [LogicalOpParentheses, LogicalNotParentheses, + BitwiseConditionalParentheses, BitwiseOpParentheses, ShiftOpParentheses, OverloadedShiftOpParentheses, diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -5745,6 +5745,9 @@ def warn_precedence_conditional : Warning< "operator '?:' has lower precedence than '%0'; '%0' will be evaluated first">, InGroup; +def warn_precedence_bitwise_conditional : Warning< + "operator '?:' has lower precedence than '%0'; '%0' will be evaluated first">, + InGroup; def note_precedence_conditional_first : Note< "place parentheses around the '?:' expression to evaluate it first">; diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -7620,7 +7620,12 @@ static bool IsArithmeticOp(BinaryOperatorKind Opc) { return BinaryOperator::isAdditiveOp(Opc) || BinaryOperator::isMultiplicativeOp(Opc) || - BinaryOperator::isShiftOp(Opc); + BinaryOperator::isShiftOp(Opc) || Opc == BO_And || Opc == BO_Or; + // This only checks for bitwise-or and bitwise-and, but not bitwise-xor and + // not any of the logical operators. Bitwise-xor is commonly used as a + // logical-xor because there is no logical-xor operator. The logical + // operators, including uses of xor, have a high false positive rate for + // precedence warnings. } /// IsArithmeticBinaryExpr - Returns true if E is an arithmetic binary @@ -7710,7 +7715,11 @@ // The condition is an arithmetic binary expression, with a right- // hand side that looks boolean, so warn. - Self.Diag(OpLoc, diag::warn_precedence_conditional) + unsigned DiagID = BinaryOperator::isBitwiseOp(CondOpcode) + ? diag::warn_precedence_bitwise_conditional + : diag::warn_precedence_conditional; + + Self.Diag(OpLoc, DiagID) << Condition->getSourceRange() << BinaryOperator::getOpcodeStr(CondOpcode); diff --git a/clang/test/Sema/parentheses.c b/clang/test/Sema/parentheses.c --- a/clang/test/Sema/parentheses.c +++ b/clang/test/Sema/parentheses.c @@ -93,6 +93,28 @@ (void)(x + y > 0 ? 1 : 2); // no warning (void)(x + (y > 0) ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '+'}} expected-note 2{{place parentheses}} + + (void)(b ? 0xf0 : 0x10 | b ? 0x5 : 0x2); // expected-warning {{operator '?:' has lower precedence than '|'}} expected-note 2{{place parentheses}} + + (void)((b ? 0xf0 : 0x10) | (b ? 0x5 : 0x2)); // no warning, has parentheses + (void)(b ? 0xf0 : (0x10 | b) ? 0x5 : 0x2); // no warning, has parentheses + + (void)(x | b ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '|'}} expected-note 2{{place parentheses}} + (void)(x & b ? 1 : 2); // expected-warning {{operator '?:' has lower precedence than '&'}} expected-note 2{{place parentheses}} + + (void)((x | b) ? 1 : 2); // no warning, has parentheses + (void)(x | (b ? 1 : 2)); // no warning, has parentheses + (void)((x & b) ? 1 : 2); // no warning, has parentheses + (void)(x & (b ? 1 : 2)); // no warning, has parentheses + + // Only warn on uses of the bitwise operators, and not the logical operators. + // The bitwise operators are more likely to be bugs while the logical + // operators are more likely to be used correctly. Since there is no + // explicit logical-xor operator, the bitwise-xor is commonly used instead. + // For this warning, treat the bitwise-xor as if it were a logical operator. + (void)(x ^ b ? 1 : 2); // no warning, ^ is often used as logical xor + (void)(x || b ? 1 : 2); // no warning, logical operator + (void)(x && b ? 1 : 2); // no warning, logical operator } // RUN: not %clang_cc1 -fsyntax-only -Wparentheses -Werror -fdiagnostics-show-option %s 2>&1 | FileCheck %s -check-prefix=CHECK-FLAG