vabridgers updated this revision to Diff 283780.
vabridgers added a comment.
use source from expression in fixit
s/seperate/separate/
address some chained comparison ambiguities outside of original scope of changes
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85097/new/
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,263 @@
+// RUN: %clang_cc1 -fsyntax-only -Wcompare-op-parentheses -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wparentheses -verify %s
+
+int tests(int p1, int p2, int p3, int p4, int p5, int p6) {
+ int b = 0;
+ b = 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 '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '<=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '<=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = 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 '>' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '>' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '>=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '>=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = 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 '>' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = 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 '>' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = 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 '>=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '<=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = 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 '>=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = 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 '<=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = p1 < p2 < p3 < p4;
+ // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ // expected-warning@-5 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-6 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-7 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-8 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = (p1 < p2) < p3 < p4;
+ // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = p1 < (p2 < p3) < p4;
+ // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = p1 < p2 < (p3 < p4);
+ // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = (p1 < p2 < p3) < p4;
+ // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = p1 < (p2 < p3 < p4);
+ // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = p1 < p2 <= p3 < p4;
+ // expected-warning@-1 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ // expected-warning@-5 {{comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'}}
+ // expected-note@-6 {{place parentheses around the '<=' comparison to silence this warning}}
+ // expected-note@-7 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-8 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = (p1 < p2) < p3; // no warning
+ b = p1 < (p2 < p3); // no warning
+ b = (p1 < p2) <= p3; // no warning
+ b = p1 < (p2 <= p3); // no warning
+ b = (p1 <= p2) <= p3; // no warning
+ b = p1 <= (p2 <= p3); // no warning
+ b = (p1 <= p2) < p3; // no warning
+ b = p1 <= (p2 < p3); // no warning
+ b = (p1 > p2) > p3; // no warning
+ b = p1 > (p2 > p3); // no warning
+ b = (p1 > p2) >= p3; // no warning
+ b = p1 > (p2 >= p3); // no warning
+ b = (p1 >= p2) >= p3; // no warning
+ b = p1 >= (p2 >= p3); // no warning
+ b = (p1 >= p2) > p3; // no warning
+ b = p1 >= (p2 > p3); // no warning
+ b = (p1 > p2) < p3; // no warning
+ b = p1 > (p2 < p3); // no warning
+ b = (p1 < p2) > p3; // no warning
+ b = p1 < (p2 > p3); // no warning
+ b = (p1 > p2) <= p3; // no warning
+ b = p1 > (p2 <= p3); // no warning
+ b = (p1 < p2) >= p3; // no warning
+ b = p1 < (p2 >= p3); // no warning
+ b = (p1 >= p2) < p3; // no warning
+ b = p1 >= (p2 < p3); // no warning
+ b = (p1 <= p2) > p3; // no warning
+ b = p1 <= (p2 > p3); // no warning
+ b = (p1 >= p2) <= p3; // no warning
+ b = p1 >= (p2 <= p3); // no warning
+ b = (p1 <= p2) >= p3; // no warning
+ b = p1 <= (p2 >= p3); // no warning
+
+ b = (p1 < p2) < (p3 < p4); // no warning
+ b = p1 < ((p2 < p3) < p4); // no warning
+ b = ((p1 < p2) < p3) < p4; // no warning
+
+ b = (p1 < p2) <= (p3 < p4); // no warning
+
+ b = (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 '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = ((p1 < p2) && (p2 < p3)); // no warning
+ b = ((p1 < p2) && (p2)); // no warning
+ b = ((p1) && (p3 < p2)); // no warning
+ b = (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 '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = (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 '<=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = (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 '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+ b = (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 '<=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = (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 '>' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = (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 '>' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = (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 '>=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = (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 '>=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '>=' comparison to silence this warning}}
+ // expected-note@-4 {{separate the expression into two clauses to give it the intended mathematical meaning}}
+
+ b = (p1 >= p2 || p3); // no warning
+ b = (p1 && p3 < p2); // no warning
+ b = ((p1 < p2) < p3); // no warning
+ b = (p1 < (p2 < p3)); // no warning
+
+ b = (0 <= (p1 < p2));
+ // expected-warning@-1 {{result of comparison of constant 0 with boolean expression is always true}}
+
+ // FIXME - These do not currently trigger any warnings
+ b = (p1 == p2 < p3); // maybe intentional, but definitely deserving of a warning
+ // expected-warning@-1 {{chained comparisons may need parentheses for clarity}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '==' comparison to silence this warning}}
+ b = (p1 == p2 == p3); // definitely buggy and deserving of a warning
+ // expected-warning@-1 {{chained comparisons may need parentheses for clarity}}
+ // expected-note@-2 {{place parentheses around the '==' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '==' comparison to silence this warning}}
+ b = (p1 != p2 != p3); // definitely buggy and deserving of a warning
+ // expected-warning@-1 {{chained comparisons may need parentheses for clarity}}
+ // expected-note@-2 {{place parentheses around the '!=' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '!=' comparison to silence this warning}}
+ b = (p1 < p2 == p3 < p4); // maybe intentional, but definitely deserving of a warning
+ // expected-warning@-1 {{chained comparisons may need parentheses for clarity}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '==' comparison to silence this warning}}
+ b = (p1 == p2 < p3 == p4); // definitely buggy and deserving of a warning
+ // expected-warning@-1 {{chained comparisons may need parentheses for clarity}}
+ // expected-note@-2 {{place parentheses around the '<' comparison to silence this warning}}
+ // expected-note@-3 {{place parentheses around the '==' comparison to silence this warning}}
+ // expected-warning@-4 {{chained comparisons may need parentheses for clarity}}
+ // expected-note@-5 {{place parentheses around the '==' comparison to silence this warning}}
+ // expected-note@-6 {{place parentheses around the '==' comparison to silence this warning}}
+
+ return b;
+}
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,83 @@
ParensRange);
}
+// Emit diagnostics for ambiguous compares, loosely x<=y<=z, or
+// more precisely...
+//
+// Accepts (x ['<'|'<='|'>'|'>='] y ['<'|'<='|'>'|'>='] z), suggests:
+// 1) ((x ['<'|'<='|'>'|'>='] y) ['<'|'<='|'>'|'>='] z), or
+// 2) (x ['<'|'<='|'>'|'>='] (y ['<'|'<='|'>'|'>='] z)). or
+// 3) (x ['<'|'<='|'>'|'>='] y) && (y ['<'|'<='|'>'|'>='] z)
+static void EmitDiagnosticForAmbiguousRelationOp(Sema &Self,
+ BinaryOperatorKind Opc,
+ SourceLocation OpLoc,
+ Expr *LHSExpr, Expr *RHSExpr) {
+ BinaryOperator *LHSBO = dyn_cast<BinaryOperator>(LHSExpr);
+
+ SourceRange DiagRange = SourceRange(LHSExpr->getBeginLoc(), OpLoc);
+
+ Self.Diag(OpLoc, diag::warn_relation_parentheses) << DiagRange << OpLoc;
+ SuggestParentheses(Self, LHSExpr->getBeginLoc(),
+ Self.PDiag(diag::note_compare_with_parens)
+ << LHSBO->getOpcodeStr(),
+ SourceRange(LHSExpr->getBeginLoc(), LHSExpr->getEndLoc()));
+ SuggestParentheses(
+ Self, OpLoc,
+ Self.PDiag(diag::note_compare_with_parens)
+ << BinaryOperator::getOpcodeStr(Opc),
+ SourceRange(LHSBO->getRHS()->getBeginLoc(), RHSExpr->getEndLoc()));
+
+ std::string LHSLOpStr = "xyz"; // = LHSBO->getRHS()->getName();//"xyz";
+
+ Expr *lhs = LHSBO->getRHS();
+ if (DeclRefExpr *DRE =
+ dyn_cast<DeclRefExpr>(lhs->IgnoreImplicit()->IgnoreParens())) {
+ if (VarDecl *VD = dyn_cast<VarDecl>(DRE->getDecl())) {
+ LHSLOpStr = VD->getQualifiedNameAsString();
+ }
+ }
+
+ Self.Diag(LHSExpr->getBeginLoc(), diag::note_compare_separate_sides)
+ << FixItHint::CreateInsertion(LHSExpr->getBeginLoc(), "(")
+ << FixItHint::CreateInsertion(LHSExpr->getEndLoc(), ") && ")
+ << FixItHint::CreateInsertion(LHSBO->getRHS()->getBeginLoc(),
+ "( " + LHSLOpStr)
+ << FixItHint::CreateInsertion(RHSExpr->getEndLoc(), ")");
+}
+
+static void EmitDiagnosticForAmbiguousCompareOp(Sema &Self,
+ BinaryOperatorKind Opc,
+ SourceLocation OpLoc,
+ Expr *LHSExpr, Expr *RHSExpr) {
+ BinaryOperator *LHSBO = dyn_cast<BinaryOperator>(LHSExpr);
+ BinaryOperator *RHSBO = dyn_cast<BinaryOperator>(RHSExpr);
+
+ bool isLeftCompare = LHSBO && LHSBO->isComparisonOp();
+ bool isRightCompare = RHSBO && RHSBO->isComparisonOp();
+
+ SourceRange DiagRange = isLeftCompare
+ ? SourceRange(LHSExpr->getBeginLoc(), OpLoc)
+ : SourceRange(OpLoc, RHSExpr->getEndLoc());
+
+ StringRef OpStr =
+ isLeftCompare ? LHSBO->getOpcodeStr() : RHSBO->getOpcodeStr();
+ SourceRange ParensRange =
+ isLeftCompare
+ ? SourceRange(LHSBO->getRHS()->getBeginLoc(), RHSExpr->getEndLoc())
+ : SourceRange(LHSExpr->getBeginLoc(), RHSBO->getLHS()->getEndLoc());
+
+ Self.Diag(OpLoc, diag::warn_compare_parentheses) << DiagRange << OpLoc;
+ // see line 13980 ... for reference
+ SuggestParentheses(Self, OpLoc,
+ Self.PDiag(diag::note_compare_with_parens) << OpStr,
+ DiagRange);
+
+ SuggestParentheses(Self, OpLoc,
+ Self.PDiag(diag::note_compare_with_parens)
+ << BinaryOperator::getOpcodeStr(Opc),
+ DiagRange);
+}
+
/// 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 +14110,24 @@
E->EvaluateAsBooleanCondition(Res, S.getASTContext()) && !Res;
}
+// Finds ambiguous compares, loosely x<=y<=z, and emits diagnostics if found.
+static void DiagnoseAmbiguousComparison(Sema &S, BinaryOperatorKind Opc,
+ SourceLocation OpLoc, Expr *LHSExpr,
+ Expr *RHSExpr) {
+ BinaryOperator *LBop = dyn_cast<BinaryOperator>(LHSExpr);
+ BinaryOperator *RBop = dyn_cast<BinaryOperator>(RHSExpr);
+ bool isRelationOp =
+ (LBop && LBop->isRelationalOp()) || (RBop && RBop->isRelationalOp());
+ bool isCompareOp =
+ (LBop && LBop->isComparisonOp()) || (RBop && RBop->isComparisonOp());
+ if (isRelationOp && BinaryOperator::isRelationalOp(Opc)) {
+ return EmitDiagnosticForAmbiguousRelationOp(S, Opc, OpLoc, LHSExpr,
+ RHSExpr);
+ } else if (isCompareOp) {
+ return EmitDiagnosticForAmbiguousCompareOp(S, Opc, OpLoc, LHSExpr, RHSExpr);
+ }
+}
+
/// Look for '&&' in the left hand of a '||' expr.
static void DiagnoseLogicalAndInLogicalOrLHS(Sema &S, SourceLocation OpLoc,
Expr *LHSExpr, Expr *RHSExpr) {
@@ -14160,8 +14255,15 @@
// Warn on overloaded shift operators and comparisons, such as:
// cout << 5 == 4;
- if (BinaryOperator::isComparisonOp(Opc))
+ if (BinaryOperator::isComparisonOp(Opc)) {
DiagnoseShiftCompare(Self, OpLoc, LHSExpr, RHSExpr);
+ // Warn on ambiguous compares, loosely x<=y<=z
+ DiagnoseAmbiguousComparison(Self, Opc, OpLoc, LHSExpr, RHSExpr);
+ }
+
+ // Warn on ambiguous compares, loosely x<=y<=z
+ // if (BinaryOperator::isRelationalOp(Opc))
+ // DiagnoseAmbiguousComparison(Self, Opc, 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,18 @@
def warn_bitwise_op_in_bitwise_op : Warning<
"'%0' within '%1'">, InGroup<BitwiseOpParentheses>, DefaultIgnore;
+def warn_compare_parentheses : Warning<
+ "chained comparisons may need parentheses for clarity">,
+ InGroup<CompareOpParentheses>, DefaultIgnore;
+
+def warn_relation_parentheses : Warning<
+ "chained comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'">,
+ InGroup<CompareOpParentheses>, DefaultIgnore;
+def note_compare_with_parens : Note<
+ "place parentheses around the '%0' comparison to silence this warning">;
+def note_compare_separate_sides : Note<
+ "separate the expression into two clauses to give it the intended mathematical meaning">;
+
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">;
@@ -786,7 +787,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-op-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-op-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
