[Clang][Diagnostic] Add `-Wcomparison-op-parentheses` to warn on chained comparisons

[hazohelet]

Summary

This patch introduces a new warning flag -Wcomparison-op-parentheses in -Wparentheses to issue warnings with its fixit hint for comparison operators within another comparison operator.

This fixes https://github.com/llvm/llvm-project/issues/60256

[hazohelet]

Fix the new warning issued against libcxx test code.

[erichkeane]

clang/lib/Sema/SemaExpr.cpp

15539

I'd repalce the comment with something more like:

Emit a diagnostic for a case where a comparison operation is a direct sub-expression of another comparison operation.  Additionally, emit a fixit hint to suggest the inner comparison expression be wrapped in parentheses.

15548

I find myself wondering if we could provide a better 'suggested fix' here. Aaron is better with the diagnostics than I am, but I would think that someone doing:

a < b < c PROBABLY doesn't mean that, they probably mean: a < b && b < c.

Also, is the mistake the 'same' when they do something like a > b != c ? It would seem to me that mixing operators might make it something either more intentional/meaningful. ADDITIONALLY, in the case where they are booleans, these end up being overly noisy. The pattern of the == c (where 'c' is bool, or convertible to bool) is probably intentional.

I think the logic here needs to be more complicated than just "Comparison within Comparison", however I don't have a fully formed idea of when to diagnose.

@tahonermann : Do you perhaps have a good idea?

clang/test/Sema/comparison-op-parentheses.c

2

don't use macros to control conditional diagnostics like that. You use -verify=something.

[hazohelet]

clang/lib/Sema/SemaExpr.cpp

15548

I find myself wondering if we could provide a better 'suggested fix' here. Aaron is better with the diagnostics than I am, but I would think that someone doing:

a < b < c PROBABLY doesn't mean that, they probably mean: a < b && b < c.

Yes. We could provide a better fix-it hint.
My idea:

• In the case of chained relational operators (<, >, <=, >=), clang suggests adding &&.
• In other cases, clang suggests parentheses.

How about doing it this way? It's similar to how Rust handles chained comparisons.

Also, is the mistake the 'same' when they do something like a > b != c ? It would seem to me that mixing operators might make it something either more intentional/meaningful. ADDITIONALLY, in the case where they are booleans, these end up being overly noisy. The pattern of the == c (where 'c' is bool, or convertible to bool) is probably intentional.

I think the logic here needs to be more complicated than just "Comparison within Comparison", however I don't have a fully formed idea of when to diagnose.

I have a differing perspective on suppressing the warning for boolean and boolean-convertible values.
There are two possible programmer mistakes in chained comparisons.

1. a > b != c misunderstood as a > b && b != c
2. a > b != c misunderstood as a > (b != c)

While the latter is considered rare in this scenario, the former could be likely to happen due to other languages, such as Python handling chained comparisons in the former manner.

[erichkeane]

clang/lib/Sema/SemaExpr.cpp

15548

I'd be interested to see the fixit-hints for the first bit, also to see how others feel about it here.

IMO, a > b != c to mean (a > b) != c is a reasonably common pattern I suspect we won't want to be noisy on.

[dblaikie]

I'd be interested to see the fixit-hints for the first bit, also to see how others feel about it here.

My 2c is that -Wparentheses is already a very stylistic warning (even correct code, without knowing about this esoteric/specific suppression style of adding parens, is likely to be caught by this general class of diagnostic) so I'm pretty apprehensive about opening it up further - but data would be the decider. Running the proposed slices (different operators in different orders, etc) of warning over some broad codebases to get a sense of the false positive rate would be really helpful/necessary to decide which things to include and exclude.

[hazohelet]

Address comments from erichkeane:

• Fix comment wording
• Avoid using macro in test file

Implement my proposal for fix-it hint:

• In the case of chained relational operators (<, >, <=, >=), suggest adding &&.
• In other cases, suggest parentheses.

[erichkeane]

I'm reasonably happy here. I think the chained-comparisons getting the && suggestion is nice. However in each case, the diagnostic is REALLY poor. It isn't really clear to most folks (barely clear to me!) that '<' within '>' really means anything. I'd love if we could spend some time brainstorming some better diagnostic wording that makes it clear what the problem is.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

6654

Spit-balling wording ideas:

'%0' results in a boolean value which is then compared using '%1'; did you mean a three-way comparison instead?

comparing the boolean result of '%0' with '%1' does not perform a three-way comparison

Also, why are these diagnostics off by default? Do we have some idea as to the false positive rate?

clang/lib/Sema/SemaExpr.cpp

15548

Common? I did a code search and everything I'm seeing is either a false positive (a lot of template stuff) or within a comment: https://sourcegraph.com/search?q=context:global+%5BA-Za-z0-9_%5D%2B%5Cs*%28%5C%3D%5C%3D%7C%5C%21%5C%3D%7C%5C%3C%7C%5C%3E%7C%5C%3C%5C%3D%7C%5C%3E%5C%3D%29%5Cs*%5BA-Za-z0-9_%5D%2B%5Cs*%5B%5C%21%5C%3D%5D%5C%3D%5Cs*%5BA-Za-z0-9_%5D%2B+lang:c+lang:C%2B%2B+&patternType=regexp&sm=1&groupBy=repo

[hazohelet]

Also, why are these diagnostics off by default? Do we have some idea as to the false positive rate?

As for the false positive rate, I have checked for instances of this warning in the codebases for 'oneapi-src/oneTBB', 'rui314/mold', and 'microsoft/lightgbm', but did not find any new cases.
I also ran a test on 'tensorflow/tensorflow' using gcc '-Wparentheses' and found six lines of code that trigger the new diagnostic. They all relate to checking whether x and y have the same sign using x > 0 == y > 0 and alike. I tried to build with tensorflow using clang, but I stumbled upon some errors (for my poor knowledge of bazel configuration), so here I am using gcc.

I set the diagnostic disabled by default for compatibility with gcc. Considering the test against tensorflow above, it would be too noisy if we turned on the suggest-parentheses diagnostic by default (From my best guess, it would generate at least 18 new instances of warning on the tensorflow build for the six lines).
However, in my opinion, it is reasonable enough to have the diagnostic on chained relational operators enabled by default. The following is an excerpt from the 'Existing Code in C++' section of the proposal document of the introduction of chaining comparison for C++17.

Overall, what we found was:

• Zero instances of chained arithmetic comparisons that are correct today. That is, intentionally using the current standard behavior.
• Four instances of currently-erroneous arithmetic chaining, of the assert(0 <= ratio <= 1.0); variety. These are bugs that compile today but don’t do what the programmer intended, but with this proposal would change in meaning to become correct.
• Many instances of using successive comparison operators in DSLs that overloaded these operators to give meaning unrelated to comparisons.

Note that the 'chaining comparisons' in the document are

• all ==, such as a == b == c == d;
• all {<, <=}, such as a < b <= c < d; and
• all {>, >=} (e.g., a >= b > c > d).

URL: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0893r0.html

Although this paper is five years old now, I think we can conclude chaining relational operators are bug-prone and should be diagnosed by default.
Also, reading the document above, I think it would be reasonable to suggest adding '&&' in a == b == c case, too.

tensorflow/tensorflow newly-warned lines:

• https://github.com/tensorflow/tensorflow/blob/12eef948f12e47ab9ce736d8967c72fc4436db79/tensorflow/compiler/xla/overflow_util.h#L67
• https://github.com/tensorflow/tensorflow/blob/78034c6ee8f20932d7c43498326ea190966c5a19/tensorflow/core/kernels/cwise_ops.h#L366
• https://github.com/tensorflow/tensorflow/blob/78034c6ee8f20932d7c43498326ea190966c5a19/tensorflow/core/kernels/cwise_ops.h#L436
• https://github.com/tensorflow/tensorflow/blob/78034c6ee8f20932d7c43498326ea190966c5a19/tensorflow/core/kernels/cwise_ops.h#L457
• https://github.com/tensorflow/tensorflow/blob/9edf0d9eb621f18217c2a0cc5d9b67f53b40901e/tensorflow/compiler/xla/service/while_loop_analysis.cc#L364
• https://github.com/tensorflow/tensorflow/blob/9edf0d9eb621f18217c2a0cc5d9b67f53b40901e/tensorflow/compiler/xla/service/while_loop_analysis.cc#L377

[aaron.ballman]

In D142800#4104241, @hazohelet wrote:

Also, why are these diagnostics off by default? Do we have some idea as to the false positive rate?

As for the false positive rate, I have checked for instances of this warning in the codebases for 'oneapi-src/oneTBB', 'rui314/mold', and 'microsoft/lightgbm', but did not find any new cases.

Thank you for doing the extra testing! Sorry for the delayed response, this review fell off my radar for a bit.

I also ran a test on 'tensorflow/tensorflow' using gcc '-Wparentheses' and found six lines of code that trigger the new diagnostic. They all relate to checking whether x and y have the same sign using x > 0 == y > 0 and alike. I tried to build with tensorflow using clang, but I stumbled upon some errors (for my poor knowledge of bazel configuration), so here I am using gcc.

Thank you for reporting this, that's really good information.

I set the diagnostic disabled by default for compatibility with gcc. Considering the test against tensorflow above, it would be too noisy if we turned on the suggest-parentheses diagnostic by default (From my best guess, it would generate at least 18 new instances of warning on the tensorflow build for the six lines).
However, in my opinion, it is reasonable enough to have the diagnostic on chained relational operators enabled by default. The following is an excerpt from the 'Existing Code in C++' section of the proposal document of the introduction of chaining comparison for C++17.

Overall, what we found was:

• Zero instances of chained arithmetic comparisons that are correct today. That is, intentionally using the current standard behavior.
• Four instances of currently-erroneous arithmetic chaining, of the assert(0 <= ratio <= 1.0); variety. These are bugs that compile today but don’t do what the programmer intended, but with this proposal would change in meaning to become correct.
• Many instances of using successive comparison operators in DSLs that overloaded these operators to give meaning unrelated to comparisons.

Note that the 'chaining comparisons' in the document are

• all ==, such as a == b == c == d;
• all {<, <=}, such as a < b <= c < d; and
• all {>, >=} (e.g., a >= b > c > d).

URL: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0893r0.html

Although this paper is five years old now, I think we can conclude chaining relational operators are bug-prone and should be diagnosed by default.
Also, reading the document above, I think it would be reasonable to suggest adding '&&' in a == b == c case, too.

I tend to agree -- I was searching around sourcegraph (https://sourcegraph.com/search?q=context:global+lang:C+lang:C%2B%2B+%5BA-Za-z0-9_%5D%2B%5Cs*%28%3D%3D%7C%21%3D%7C%3C%3D%7C%3E%3D%29%5Cs*%5BA-Za-z0-9_%5D%2B%5Cs*%28%3D%3D%7C%21%3D%7C%3C%3D%7C%3E%3D%29&patternType=regexp&sm=1&groupBy=repo) and I can't find a whole lot of evidence for chained operators outside of comments.

[hazohelet]

Update the differential

• Revise warning messages based on ideas from @aaron.ballman.
• Introduce a new warning flag -Wchaining-comparisons that is enabled by default, to warn about chaining relationals or equal operators.
• Adjust existing test files to conform with the new warning settings.

[hazohelet]

In D142800#4165090, @aaron.ballman wrote:

In D142800#4104241, @hazohelet wrote:

Also, why are these diagnostics off by default? Do we have some idea as to the false positive rate?

As for the false positive rate, I have checked for instances of this warning in the codebases for 'oneapi-src/oneTBB', 'rui314/mold', and 'microsoft/lightgbm', but did not find any new cases.

Thank you for doing the extra testing! Sorry for the delayed response, this review fell off my radar for a bit.

I also ran a test on 'tensorflow/tensorflow' using gcc '-Wparentheses' and found six lines of code that trigger the new diagnostic. They all relate to checking whether x and y have the same sign using x > 0 == y > 0 and alike. I tried to build with tensorflow using clang, but I stumbled upon some errors (for my poor knowledge of bazel configuration), so here I am using gcc.

Thank you for reporting this, that's really good information.

I set the diagnostic disabled by default for compatibility with gcc. Considering the test against tensorflow above, it would be too noisy if we turned on the suggest-parentheses diagnostic by default (From my best guess, it would generate at least 18 new instances of warning on the tensorflow build for the six lines).
However, in my opinion, it is reasonable enough to have the diagnostic on chained relational operators enabled by default. The following is an excerpt from the 'Existing Code in C++' section of the proposal document of the introduction of chaining comparison for C++17.

Overall, what we found was:

• Zero instances of chained arithmetic comparisons that are correct today. That is, intentionally using the current standard behavior.
• Four instances of currently-erroneous arithmetic chaining, of the assert(0 <= ratio <= 1.0); variety. These are bugs that compile today but don’t do what the programmer intended, but with this proposal would change in meaning to become correct.
• Many instances of using successive comparison operators in DSLs that overloaded these operators to give meaning unrelated to comparisons.

Note that the 'chaining comparisons' in the document are

• all ==, such as a == b == c == d;
• all {<, <=}, such as a < b <= c < d; and
• all {>, >=} (e.g., a >= b > c > d).

URL: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p0893r0.html

Although this paper is five years old now, I think we can conclude chaining relational operators are bug-prone and should be diagnosed by default.
Also, reading the document above, I think it would be reasonable to suggest adding '&&' in a == b == c case, too.

I tend to agree -- I was searching around sourcegraph (https://sourcegraph.com/search?q=context:global+lang:C+lang:C%2B%2B+%5BA-Za-z0-9_%5D%2B%5Cs*%28%3D%3D%7C%21%3D%7C%3C%3D%7C%3E%3D%29%5Cs*%5BA-Za-z0-9_%5D%2B%5Cs*%28%3D%3D%7C%21%3D%7C%3C%3D%7C%3E%3D%29&patternType=regexp&sm=1&groupBy=repo) and I can't find a whole lot of evidence for chained operators outside of comments.

Thanks for the review and the sourcegraph search!
I added a new warning flag -Wchaining-comparisons that is enabled by default. I added this flag to -Wparentheses for gcc compatibility.

[hazohelet]

Rebase and Ping

[xgupta]

Thought to point out that there is a clang-tidy check in review doing the similar thing - https://reviews.llvm.org/D144429.

clang/docs/ReleaseNotes.rst

 Bug Fixes
 ---------
 - Fix crash on invalid code when looking up a destructor in a templated class
   inside a namespace. This fixes
   `Issue 59446 <https://github.com/llvm/llvm-project/issues/59446>`_.
 
 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
 -------------------------------------------------
 
 New Compiler Flags
 ------------------
 
 Deprecated Compiler Flags

clang/include/clang/Basic/DiagnosticGroups.td

 def : DiagGroup<"c++2a-compat-pedantic", [CXX20CompatPedantic]>;
 
 def ExitTimeDestructors : DiagGroup<"exit-time-destructors">;
 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 ComparisonOpParentheses: DiagGroup<"comparison-op-parentheses">;
 def LogicalOpParentheses: DiagGroup<"logical-op-parentheses">;
 def LogicalNotParentheses: DiagGroup<"logical-not-parentheses">;
 def ShiftOpParentheses: DiagGroup<"shift-op-parentheses">;
 def OverloadedShiftOpParentheses: DiagGroup<"overloaded-shift-op-parentheses">;
 def DanglingElse: DiagGroup<"dangling-else">;
 def DanglingField : DiagGroup<"dangling-field">;
 def DanglingInitializerList : DiagGroup<"dangling-initializer-list">;
 def DanglingGsl : DiagGroup<"dangling-gsl">;

 // in -Wparentheses because most users who use -Wparentheses explicitly
 // do not want these warnings.
 def ParenthesesOnEquality : DiagGroup<"parentheses-equality">;
 def Parentheses : DiagGroup<"parentheses",
                             [LogicalOpParentheses,
                              LogicalNotParentheses,
                              BitwiseConditionalParentheses,
                              BitwiseOpParentheses,
+                             ComparisonOpParentheses,
                              ShiftOpParentheses,
                              OverloadedShiftOpParentheses,
                              ParenthesesOnEquality,
                              DanglingElse]>;
 
 // -Wconversion has its own warnings, but we split a few out for
 // legacy reasons:
 //   - some people want just 64-to-32 warnings

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def note_logical_instead_of_bitwise_change_operator : Note<
   "use '%0' for a bitwise operation">;
 def note_logical_instead_of_bitwise_remove_constant : Note<
   "remove constant to silence this warning">;
 
 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;
 
 def warn_overloaded_shift_in_comparison :Warning<
   "overloaded operator %select{>>|<<}0 has higher precedence than "
   "comparison operator">,
   InGroup<OverloadedShiftOpParentheses>;
 def note_evaluate_comparison_first :Note<

clang/lib/Sema/SemaExpr.cpp

SuggestParentheses(S, OCE->getOperatorLoc(),
                      S.PDiag(diag::note_precedence_silence)
                          << (Kind == OO_LessLess ? "<<" : ">>"),
                      OCE->getSourceRange());
   SuggestParentheses(
       S, OpLoc, S.PDiag(diag::note_evaluate_comparison_first),
       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,
                                     SourceLocation OpLoc, Expr *LHSExpr,
                                     Expr *RHSExpr){
   // Diagnose "arg1 'bitwise' arg2 'eq' arg3".
   if (BinaryOperator::isBitwiseOp(Opc))
     DiagnoseBitwisePrecedence(Self, Opc, OpLoc, LHSExpr, RHSExpr);
static void DiagnoseBinOpPrecedence(Sema &Self, BinaryOperatorKind Opc,
 
   if ((Opc == BO_Shl && LHSExpr->getType()->isIntegralType(Self.getASTContext()))
       || Opc == BO_Shr) {
     StringRef Shift = BinaryOperator::getOpcodeStr(Opc);
     DiagnoseAdditionInShift(Self, OpLoc, LHSExpr, Shift);
     DiagnoseAdditionInShift(Self, OpLoc, RHSExpr, Shift);
   }
 
+  if (BinaryOperator::isComparisonOp(Opc)) {
     // Warn on overloaded shift operators and comparisons, such as:
     // cout << 5 == 4;
-  if (BinaryOperator::isComparisonOp(Opc))
     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.
 ExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc,
                             tok::TokenKind Kind,
                             Expr *LHSExpr, Expr *RHSExpr) {
   BinaryOperatorKind Opc = ConvertTokenKindToBinaryOpcode(Kind);
   assert(LHSExpr && "ActOnBinOp(): missing left expression");

clang/test/Misc/warning-wall.c

 CHECK-NEXT:    -Wcast-of-sel-type
 CHECK-NEXT:    -Wextern-c-compat
 CHECK-NEXT:    -Wuser-defined-warnings
 CHECK-NEXT:  -Wparentheses
 CHECK-NEXT:    -Wlogical-op-parentheses
 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
 CHECK-NEXT:    -Wdangling-else
 CHECK-NEXT:  -Wswitch
 CHECK-NEXT:  -Wswitch-bool
 CHECK-NEXT:  -Wmisleading-indentation
 
 
 CHECK-NOT:-W

clang/test/Sema/comparison-op-parentheses.c

+// 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);
+  (void)((a<b) > (c > a));
+
+}