diff --git a/clang/docs/DiagnosticsReference.rst b/clang/docs/DiagnosticsReference.rst --- a/clang/docs/DiagnosticsReference.rst +++ b/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:** 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 @@ -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, 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 @@ -6130,6 +6130,10 @@ def warn_bitwise_op_in_bitwise_op : Warning< "'%0' within '%1'">, InGroup, DefaultIgnore; +def warn_comparison_parentheses : Warning< + "comparisons like 'x<=y<=z' are interpreted as '(x<=y ? 1 : 0) <= z'">, + InGroup, DefaultIgnore; + def warn_logical_and_in_logical_or : Warning< "'&&' within '||'">, InGroup, DefaultIgnore; 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 @@ -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(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. diff --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c --- a/clang/test/Misc/warning-wall.c +++ b/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 diff --git a/clang/test/Sema/warn-compare-op-parentheses.c b/clang/test/Sema/warn-compare-op-parentheses.c new file mode 100644 --- /dev/null +++ b/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; +}