vabridgers created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
vabridgers requested review of this revision.
This changes add a new warning named -Wcompare-op-parentheses that's
part of the -Wparentheses diagnostic group. This diagnostic produces a
warning when a pattern like 'x<=y<=z' is found. When this pattern is not
qualified by parentheses, it's equivalent to '(x<=y ? 1 : 0) <= z',
which is a different interpretation from that of ordinary mathematical
notation.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D85097
Files:
clang/docs/DiagnosticsReference.rst
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExpr.cpp
clang/test/Misc/warning-wall.c
clang/test/Sema/warn-compare-op-parentheses.c
Index: clang/test/Sema/warn-compare-op-parentheses.c
===================================================================
--- /dev/null
+++ clang/test/Sema/warn-compare-op-parentheses.c
@@ -0,0 +1,129 @@
+// RUN: %clang_cc1 -fsyntax-only -Wcompare-op-parentheses -verify %s
+
+int case1(int p1, int p2, int p3) {
+ if (p1 < p2 < p3)
+ return 1;
+ return 0;
+ // expected-warning@-3 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-4 {{place parentheses around the '<' expression to silence this warning}}
+}
+
+int case2(int p1, int p2, int p3) {
+ // no warning
+ if ((p1 < p2) && (p2 < p3))
+ return 1;
+ return 0;
+}
+
+int case3(int p1, int p2, int p3) {
+ // no warning
+ if ((p1 < p2) && (p2))
+ return 1;
+ return 0;
+}
+
+int case4(int p1, int p2, int p3) {
+ // no warning
+ if ((p1) && (p3 < p2))
+ return 1;
+ return 0;
+}
+
+int case5(int p1, int p2, int p3) {
+ while (p1 < p3 < p2)
+ return 1;
+ return 0;
+ // expected-warning@-3 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-4 {{place parentheses around the '<' expression to silence this warning}}
+}
+
+int case6(int p1, int p2, int p3) {
+ // should not warn
+ while (p1 && p3 < p2)
+ return 1;
+ return 0;
+}
+
+int case7(int p1, int p2, int p3) {
+ // should not warn
+ while ((p1 < p3) < p2)
+ return 1;
+ return 0;
+}
+
+int case8(int p1, int p2, int p3) {
+ int ret = p1 < p2 < p3;
+ // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-2 {{place parentheses around the '<' expression to silence this warning}}
+ return ret;
+}
+
+int case9(int p1, int p2, int p3) {
+ int ret = (p1 < p2) < p3 < p1;
+ // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-2 {{place parentheses around the '<' expression to silence this warning}}
+ return ret;
+}
+
+int case10(int p1, int p2, int p3) {
+ if (p1 <= p2 < p3)
+ return 1;
+ return 0;
+ // expected-warning@-3 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-4 {{place parentheses around the '<=' expression to silence this warning}}
+}
+
+int case11(int p1, int p2, int p3) {
+ if (p1 < p2 <= p3)
+ return 1;
+ return 0;
+ // expected-warning@-3 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-4 {{place parentheses around the '<' expression to silence this warning}}
+}
+
+int case12(int p1, int p2, int p3) {
+ if (p1 <= p2 <= p3)
+ return 1;
+ return 0;
+ // expected-warning@-3 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-4 {{place parentheses around the '<=' expression to silence this warning}}
+}
+
+int case13(int p1, int p2, int p3) {
+ if (p1 > p2 < p3)
+ return 1;
+ return 0;
+ // expected-warning@-3 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-4 {{place parentheses around the '>' expression to silence this warning}}
+}
+
+int case14(int p1, int p2, int p3) {
+ if (p1 > p2 > p3)
+ return 1;
+ return 0;
+ // expected-warning@-3 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-4 {{place parentheses around the '>' expression to silence this warning}}
+}
+
+int case15(int p1, int p2, int p3) {
+ if (p1 >= p2 > p3)
+ return 1;
+ return 0;
+ // expected-warning@-3 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-4 {{place parentheses around the '>=' expression to silence this warning}}
+}
+
+int case16(int p1, int p2, int p3) {
+ if (p1 >= p2 >= p3)
+ return 1;
+ return 0;
+ // expected-warning@-3 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-4 {{place parentheses around the '>=' expression to silence this warning}}
+}
+
+int case17(int p1, int p2, int p3) {
+ // should not warn
+ if (p1 >= p2 || p3)
+ return 1;
+ return 0;
+}
Index: clang/test/Misc/warning-wall.c
===================================================================
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -83,6 +83,7 @@
CHECK-NEXT: -Wextern-c-compat
CHECK-NEXT: -Wuser-defined-warnings
CHECK-NEXT: -Wparentheses
+CHECK-NEXT: -Wcompare-op-parentheses
CHECK-NEXT: -Wlogical-op-parentheses
CHECK-NEXT: -Wlogical-not-parentheses
CHECK-NEXT: -Wbitwise-conditional-parentheses
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -14002,6 +14002,17 @@
ParensRange);
}
+static void EmitDiagnosticForAmbiguousComparison(Sema &Self,
+ SourceLocation OpLoc,
+ BinaryOperator *Bop) {
+ Self.Diag(Bop->getOperatorLoc(), diag::warn_comparison_parentheses)
+ << Bop->getSourceRange() << OpLoc;
+ SuggestParentheses(Self, Bop->getOperatorLoc(),
+ Self.PDiag(diag::note_precedence_silence)
+ << Bop->getOpcodeStr(),
+ Bop->getSourceRange());
+}
+
/// It accepts a '&&' expr that is inside a '||' one.
/// Emit a diagnostic together with a fixit hint that wraps the '&&' expression
/// in parentheses.
@@ -14033,6 +14044,27 @@
E->EvaluateAsBooleanCondition(Res, S.getASTContext()) && !Res;
}
+static bool isComparisonOpSamePrecedence(BinaryOperatorKind Opc) {
+ switch (Opc) {
+ case BO_GT:
+ case BO_GE:
+ case BO_LT:
+ case BO_LE:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static void DiagnoseAmbiguousComparison(Sema &S, SourceLocation OpLoc,
+ Expr *LHSExpr, Expr *RHSExpr) {
+ if (BinaryOperator *Bop = dyn_cast<BinaryOperator>(LHSExpr)) {
+ if (isComparisonOpSamePrecedence(Bop->getOpcode())) {
+ return EmitDiagnosticForAmbiguousComparison(S, OpLoc, Bop);
+ }
+ }
+}
+
/// Look for '&&' in the left hand of a '||' expr.
static void DiagnoseLogicalAndInLogicalOrLHS(Sema &S, SourceLocation OpLoc,
Expr *LHSExpr, Expr *RHSExpr) {
@@ -14162,6 +14194,9 @@
// cout << 5 == 4;
if (BinaryOperator::isComparisonOp(Opc))
DiagnoseShiftCompare(Self, OpLoc, LHSExpr, RHSExpr);
+
+ if (isComparisonOpSamePrecedence(Opc))
+ DiagnoseAmbiguousComparison(Self, OpLoc, LHSExpr, RHSExpr);
}
// Binary Operators. 'Tok' is the token for the operator.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6130,6 +6130,10 @@
def warn_bitwise_op_in_bitwise_op : Warning<
"'%0' within '%1'">, InGroup<BitwiseOpParentheses>, DefaultIgnore;
+def warn_comparison_parentheses : Warning<
+ "comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'">,
+ InGroup<CompareOpParentheses>, DefaultIgnore;
+
def warn_logical_and_in_logical_or : Warning<
"'&&' within '||'">, InGroup<LogicalOpParentheses>, DefaultIgnore;
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -341,6 +341,7 @@
def GlobalConstructors : DiagGroup<"global-constructors">;
def BitwiseConditionalParentheses: DiagGroup<"bitwise-conditional-parentheses">;
def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">;
+def CompareOpParentheses: DiagGroup<"compare-op-parentheses">;
def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">;
def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">;
def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
@@ -785,7 +786,8 @@
// do not want these warnings.
def ParenthesesOnEquality : DiagGroup<"parentheses-equality">;
def Parentheses : DiagGroup<"parentheses",
- [LogicalOpParentheses,
+ [CompareOpParentheses,
+ LogicalOpParentheses,
LogicalNotParentheses,
BitwiseConditionalParentheses,
BitwiseOpParentheses,
Index: clang/docs/DiagnosticsReference.rst
===================================================================
--- clang/docs/DiagnosticsReference.rst
+++ clang/docs/DiagnosticsReference.rst
@@ -2850,6 +2850,15 @@
+---------------------------------------------------------------------------+
+-Wcompare-no-parentheses
+--------------------------------
+**Diagnostic text:**
+
++----------------------------------------------------------------------------------------------------------+
+|:warning:`warning:` |nbsp| :diagtext:`comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'`|
++----------------------------------------------------------------------------------------------------------+
+
+
-Wcomplex-component-init
------------------------
**Diagnostic text:**
@@ -9873,7 +9882,7 @@
-------------
Some of the diagnostics controlled by this flag are enabled by default.
-Also controls `-Wbitwise-conditional-parentheses`_, `-Wbitwise-op-parentheses`_, `-Wdangling-else`_, `-Wlogical-not-parentheses`_, `-Wlogical-op-parentheses`_, `-Woverloaded-shift-op-parentheses`_, `-Wparentheses-equality`_, `-Wshift-op-parentheses`_.
+Also controls `-Wbitwise-conditional-parentheses`_, `-Wbitwise-op-parentheses`_, `-Wcompare-no-parentheses`, `-Wdangling-else`_, `-Wlogical-not-parentheses`_, `-Wlogical-op-parentheses`_, `-Woverloaded-shift-op-parentheses`_, `-Wparentheses-equality`_, `-Wshift-op-parentheses`_.
**Diagnostic text:**
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
