Add -Wbitwise-conditional-parentheses to warn on mixing '|' and '&' with "?:"
Extend -Wparentheses to cover mixing bitwise-and and bitwise-or with the
conditional operator. There's two main cases seen with this:
unsigned bits1 = 0xf0 | cond ? 0x4 : 0x1;
unsigned bits2 = cond1 ? 0xf0 : 0x10 | cond2 ? 0x5 : 0x2;
// Intended order of evaluation:
unsigned bits1 = 0xf0 | (cond ? 0x4 : 0x1);
unsigned bits2 = (cond1 ? 0xf0 : 0x10) | (cond2 ? 0x5 : 0x2);
// Actual order of evaluation:
unsigned bits1 = (0xf0 | cond) ? 0x4 : 0x1;
unsigned bits2 = cond1 ? 0xf0 : ((0x10 | cond2) ? 0x5 : 0x2);
Differential Revision: https://reviews.llvm.org/D66043
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@375326 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/docs/ReleaseNotes.rst b/docs/ReleaseNotes.rst
index 41dea29..298d793 100644
--- a/docs/ReleaseNotes.rst
+++ b/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/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
index 3573ebd..9280595 100644
--- a/include/clang/Basic/DiagnosticGroups.td
+++ b/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/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 98fce04..d802a92 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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<Parentheses>;
+def warn_precedence_bitwise_conditional : Warning<
+ "operator '?:' has lower precedence than '%0'; '%0' will be evaluated first">,
+ InGroup<BitwiseConditionalParentheses>;
def note_precedence_conditional_first : Note<
"place parentheses around the '?:' expression to evaluate it first">;
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 2734b4d..e41cd5b 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/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/test/Sema/parentheses.c b/test/Sema/parentheses.c
index 0dabc11..047bcbf 100644
--- a/test/Sema/parentheses.c
+++ b/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
