hazohelet updated this revision to Diff 492986.
hazohelet added a comment.
Fix the new warning issued against libcxx test code.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142800/new/
https://reviews.llvm.org/D142800
Files:
clang/docs/ReleaseNotes.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/comparison-op-parentheses.c
libcxx/test/support/test_comparisons.h
Index: libcxx/test/support/test_comparisons.h
===================================================================
--- libcxx/test/support/test_comparisons.h
+++ libcxx/test/support/test_comparisons.h
@@ -162,7 +162,7 @@
bool less = order == Order::less;
bool greater = order == Order::greater;
- return (t1 <=> t2 == order) && testComparisonsComplete(t1, t2, equal, less, greater);
+ return ((t1 <=> t2) == order) && testComparisonsComplete(t1, t2, equal, less, greater);
}
template <class T, class Param>
Index: clang/test/Sema/comparison-op-parentheses.c
===================================================================
--- /dev/null
+++ clang/test/Sema/comparison-op-parentheses.c
@@ -0,0 +1,94 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -DSILENCE
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wcomparison-op-parentheses
+// RUN: %clang_cc1 -fsyntax-only -verify %s -Wparentheses
+// RUN: %clang_cc1 -fsyntax-only -fdiagnostics-parseable-fixits %s -Wcomparison-op-parentheses 2>&1 | FileCheck %s
+
+#ifdef SILENCE
+// expected-no-diagnostics
+#endif
+
+void comparison_op_parentheses(int a, int b, int c) {
+ (void)(a ==
+ b > c);
+#ifndef SILENCE
+ // expected-warning@-2 {{'>' within '=='}}
+ // expected-note@-3 {{place parentheses around the '>' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:19-[[@LINE-6]]:19}:")"
+
+ (void)(a ==b == c);
+#ifndef SILENCE
+ // expected-warning@-2 {{'==' within '=='}}
+ // expected-note@-3 {{place parentheses around the '==' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")"
+
+ (void)(a !=b == c);
+#ifndef SILENCE
+ // expected-warning@-2 {{'!=' within '=='}}
+ // expected-note@-3 {{place parentheses around the '!=' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:15-[[@LINE-6]]:15}:")"
+
+ (void)(a !=
+ b < c);
+#ifndef SILENCE
+ // expected-warning@-2 {{'<' within '!='}}
+ // expected-note@-3 {{place parentheses around the '<' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:14-[[@LINE-5]]:14}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:19-[[@LINE-6]]:19}:")"
+
+ (void)(a>=b >= c);
+#ifndef SILENCE
+ // expected-warning@-2 {{'>=' within '>='}}
+ // expected-note@-3 {{place parentheses around the '>=' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:14-[[@LINE-6]]:14}:")"
+
+ (void)(a >b >= c);
+#ifndef SILENCE
+ // expected-warning@-2 {{'>' within '>='}}
+ // expected-note@-3 {{place parentheses around the '>' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:14-[[@LINE-6]]:14}:")"
+
+ (void)(a >b > c);
+#ifndef SILENCE
+ // expected-warning@-2 {{'>' within '>'}}
+ // expected-note@-3 {{place parentheses around the '>' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:14-[[@LINE-6]]:14}:")"
+
+
+
+ (void)(a<b <= c);
+#ifndef SILENCE
+ // expected-warning@-2 {{'<' within '<='}}
+ // expected-note@-3 {{place parentheses around the '<' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:13-[[@LINE-6]]:13}:")"
+
+ (void)(a<b > c);
+#ifndef SILENCE
+ // expected-warning@-2 {{'<' within '>'}}
+ // expected-note@-3 {{place parentheses around the '<' expression to silence this warning}}
+#endif
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-5]]:10-[[@LINE-5]]:10}:"("
+ // CHECK: fix-it:"{{.*}}":{[[@LINE-6]]:13-[[@LINE-6]]:13}:")"
+
+ (void)(a != (b > c));
+ (void)(a == (b > c));
+ (void)((a>b) >= c);
+ (void)((a<b) <= c);
+ (void)((a<b) > c);
+ (void)(a != b && a > c);
+ (void)((a<b) > c);
+}
Index: clang/test/Misc/warning-wall.c
===================================================================
--- clang/test/Misc/warning-wall.c
+++ clang/test/Misc/warning-wall.c
@@ -92,6 +92,7 @@
CHECK-NEXT: -Wlogical-not-parentheses
CHECK-NEXT: -Wbitwise-conditional-parentheses
CHECK-NEXT: -Wbitwise-op-parentheses
+CHECK-NEXT: -Wcomparison-op-parentheses
CHECK-NEXT: -Wshift-op-parentheses
CHECK-NEXT: -Woverloaded-shift-op-parentheses
CHECK-NEXT: -Wparentheses-equality
Index: clang/lib/Sema/SemaExpr.cpp
===================================================================
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -15534,6 +15534,37 @@
SourceRange(OCE->getArg(1)->getBeginLoc(), RHSExpr->getEndLoc()));
}
+/// It accepts a comparison expr that is inside a comparison one.
+/// Emit a diagnostic together with a fixit hint that wraps the inner comparison
+/// expression in parentheses.
+static void EmitDiagnosticForCompOpInCompOp(Sema &Self, SourceLocation OpLoc,
+ BinaryOperatorKind ParentOpc,
+ BinaryOperator *Bop) {
+ assert(Bop->isComparisonOp());
+ Self.Diag(Bop->getOperatorLoc(), diag::warn_comp_op_in_comp_op)
+ << Bop->getOpcodeStr() << BinaryOperator::getOpcodeStr(ParentOpc)
+ << Bop->getSourceRange() << OpLoc;
+
+ SuggestParentheses(Self, Bop->getOperatorLoc(),
+ Self.PDiag(diag::note_precedence_silence)
+ << Bop->getOpcodeStr(),
+ Bop->getSourceRange());
+}
+
+static void DiagnoseCompOpInCompOp(Sema &Self, SourceLocation OpLoc,
+ BinaryOperatorKind ParentOpc, Expr *LHSExpr,
+ Expr *RHSExpr) {
+ assert(BinaryOperator::isComparisonOp(ParentOpc));
+
+ BinaryOperator *LHSBO = dyn_cast<BinaryOperator>(LHSExpr);
+ BinaryOperator *RHSBO = dyn_cast<BinaryOperator>(RHSExpr);
+
+ if (LHSBO && LHSBO->isComparisonOp())
+ EmitDiagnosticForCompOpInCompOp(Self, OpLoc, ParentOpc, LHSBO);
+ if (RHSBO && RHSBO->isComparisonOp())
+ EmitDiagnosticForCompOpInCompOp(Self, OpLoc, ParentOpc, RHSBO);
+}
+
/// DiagnoseBinOpPrecedence - Emit warnings for expressions with tricky
/// precedence.
static void DiagnoseBinOpPrecedence(Sema &Self, BinaryOperatorKind Opc,
@@ -15564,10 +15595,14 @@
DiagnoseAdditionInShift(Self, OpLoc, RHSExpr, Shift);
}
- // Warn on overloaded shift operators and comparisons, such as:
- // cout << 5 == 4;
- if (BinaryOperator::isComparisonOp(Opc))
+ if (BinaryOperator::isComparisonOp(Opc)) {
+ // Warn on overloaded shift operators and comparisons, such as:
+ // cout << 5 == 4;
DiagnoseShiftCompare(Self, OpLoc, LHSExpr, RHSExpr);
+
+ // warnings for chaining comparisons like `a > b > c` and `a == b < c`
+ DiagnoseCompOpInCompOp(Self, OpLoc, Opc, 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
@@ -6650,6 +6650,9 @@
def warn_bitwise_op_in_bitwise_op : Warning<
"'%0' within '%1'">, InGroup<BitwiseOpParentheses>, DefaultIgnore;
+def warn_comp_op_in_comp_op : Warning<
+ "'%0' within '%1'">, InGroup<ComparisonOpParentheses>, 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
@@ -405,6 +405,7 @@
def GlobalConstructors : DiagGroup<"global-constructors">;
def BitwiseConditionalParentheses: DiagGroup<"bitwise-conditional-parentheses">;
def BitwiseOpParentheses: DiagGroup<"bitwise-op-parentheses">;
+def ComparisonOpParentheses: DiagGroup<"comparison-op-parentheses">;
def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">;
def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">;
def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
@@ -896,6 +897,7 @@
LogicalNotParentheses,
BitwiseConditionalParentheses,
BitwiseOpParentheses,
+ ComparisonOpParentheses,
ShiftOpParentheses,
OverloadedShiftOpParentheses,
ParenthesesOnEquality,
Index: clang/docs/ReleaseNotes.rst
===================================================================
--- clang/docs/ReleaseNotes.rst
+++ clang/docs/ReleaseNotes.rst
@@ -60,6 +60,8 @@
Improvements to Clang's diagnostics
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Clang now warns on chained comparisons like ``x > y > z`` and ``x == y > z``. This fixes
+ `Issue 60256 <https://github.com/llvm/llvm-project/issues/60256>`_
Non-comprehensive list of changes in this release
-------------------------------------------------
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
