[PATCH] D13643: [Sema] Warn on ternary comparison

[Matěj Grabovský via cfe-commits]

mgrabovsky created this revision.
mgrabovsky added a subscriber: cfe-commits.

This change adds a Sema diagnostic for comparisons like `a < b < c`,
which usually do not behave as intended by the author.

Fixes bug 20082.

http://reviews.llvm.org/D13643

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/bool-compare.c

Index: test/Sema/bool-compare.c
===================================================================
--- test/Sema/bool-compare.c
+++ test/Sema/bool-compare.c
@@ -85,7 +85,9 @@
   if ((a<y) != -1) {}// expected-warning {{comparison of constant -1 with boolean expression is always true}}
 
   if ((a<y) == z) {} // no warning
-  if (a>y<z)      {} // no warning
+  if (a>y<z)      {} // expected-warning {{ternary comparisons do not work as expected}} \
+                     // expected-note {{to achieve the expected behavior, turn this expression into a conjunction of two comparisons}} \
+                     // expected-note {{place parentheses around either of the comparisons to silence this warning}}
   if ((a<y) > z)  {} // no warning
   if((a<y)>(z<y)) {} // no warning
   if((a<y)==(z<y)){} // no warning
@@ -144,10 +146,12 @@
   if (z !=(a<y))  {} // no warning
   if (-1 !=(a<y)) {} // expected-warning {{comparison of constant -1 with boolean expression is always true}}
 
-  if (z ==(a<y))  {}    // no warning
-  if (z<a>y)      {}        // no warning
-  if (z > (a<y))  {}    // no warning
-  if((z<y)>(a<y)) {}   // no warning
+  if (z ==(a<y))  {}  // no warning
+  if (z<a>y)      {}  // expected-warning {{ternary comparisons do not work as expected}} \
+                      // expected-note {{to achieve the expected behavior, turn this expression into a conjunction of two comparisons}} \
+                      // expected-note {{place parentheses around either of the comparisons to silence this warning}}
+  if (z > (a<y))  {}  // no warning
+  if((z<y)>(a<y)) {}  // no warning
   if((z<y)==(a<y)){}  // no warning
   if((z<y)!=(a<y)){}  // no warning
   if((y==z)<(z==x)){}  // no warning
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -6693,6 +6693,22 @@
         << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange());
 }
 
+/// Diagnose attempts at ternary comparison, e.g., 1 < x < 2
+static void DiagnoseTernaryComparison(Sema &S, BinaryOperator *E) {
+  BinaryOperator *LHS = dyn_cast<BinaryOperator>(E->getLHS());
+  if (!LHS || !LHS->isComparisonOp())
+    return;
+
+  SourceLocation Loc = E->getSourceRange().getBegin();
+
+  S.DiagRuntimeBehavior(Loc, E,
+    S.PDiag(diag::warn_ternary_comparison) << E->getSourceRange());
+  S.DiagRuntimeBehavior(Loc, E,
+    S.PDiag(diag::note_ternary_comparison_to_conjunction));
+  S.DiagRuntimeBehavior(Loc, E,
+    S.PDiag(diag::note_ternary_comparison_silence));
+}
+
 /// Analyze the operands of the given comparison.  Implements the
 /// fallback case from AnalyzeComparison.
 static void AnalyzeImpConvsInComparison(Sema &S, BinaryOperator *E) {
@@ -6720,7 +6736,9 @@
   Expr *RHS = E->getRHS()->IgnoreParenImpCasts();
   
   bool IsComparisonConstant = false;
-  
+
+  DiagnoseTernaryComparison(S, E);
+
   // Check whether an integer constant comparison results in a value
   // of 'true' or 'false'.
   if (T->isIntegralType(S.Context)) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5862,6 +5862,14 @@
 def note_condition_assign_silence : Note<
   "place parentheses around the assignment to silence this warning">;
 
+def warn_ternary_comparison : Warning<"ternary comparisons do not work "
+  "as expected">,
+  InGroup<Parentheses>;
+def note_ternary_comparison_to_conjunction : Note<"to achieve the expected behavior, "
+  "turn this expression into a conjunction of two comparisons">;
+def note_ternary_comparison_silence : Note<"place parentheses around either "
+  "of the comparisons to silence this warning">;
+
 def warn_equality_with_extra_parens : Warning<"equality comparison with "
   "extraneous parentheses">, InGroup<ParenthesesOnEquality>;
 def note_equality_comparison_to_assign : Note<


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D13643.37078.patch
Type: text/x-patch
Size: 3998 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151012/94a7c4a0/attachment-0001.bin>