From b796ddf8a372d34c5d24107f59ead4cfbe850297 Mon Sep 17 00:00:00 2001
From: David Bolvansky <david.bolvansky@gmail.com>
Date: Sun, 22 Sep 2019 22:00:48 +0000
Subject: [PATCH] [Diagnostics] Warn if ?: with integer constants always
 evaluates to true

Extracted from D63082. GCC has this warning under -Wint-in-bool-context, but as noted in the D63082's review, we should put it under TautologicalConstantCompare.



git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@372531 91177308-0d34-0410-b5e6-96231b3b80d8
---
 include/clang/Basic/DiagnosticSemaKinds.td    |  4 ++
 lib/Sema/SemaChecking.cpp                     | 41 +++++++++++++++++++
 test/Sema/warn-integer-constants-in-ternary.c | 32 +++++++++++++++
 3 files changed, 77 insertions(+)
 create mode 100644 test/Sema/warn-integer-constants-in-ternary.c

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index ccf2a60dad..965cccc480 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6137,6 +6137,10 @@ def warn_out_of_range_compare : Warning<
   InGroup<TautologicalOutOfRangeCompare>;
 def warn_tautological_bool_compare : Warning<warn_out_of_range_compare.Text>,
   InGroup<TautologicalConstantCompare>;
+def warn_integer_constants_in_conditional_always_true : Warning<
+  "converting the result of '?:' with integer constants to a boolean always "
+  "evaluates to 'true'">,
+  InGroup<TautologicalConstantCompare>;
 def warn_comparison_of_mixed_enum_types : Warning<
   "comparison of two values with different enumeration types"
   "%diff{ ($ and $)|}0,1">,
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index ddf58aba1e..7aeace5c2a 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -11256,6 +11256,44 @@ static bool isSameWidthConstantConversion(Sema &S, Expr *E, QualType T,
   return true;
 }
 
+static void DiagnoseIntInBoolContext(Sema &S, const Expr *E) {
+  E = E->IgnoreParenImpCasts();
+  SourceLocation ExprLoc = E->getExprLoc();
+
+  if (const auto *CO = dyn_cast<ConditionalOperator>(E)) {
+    const auto *LHS = dyn_cast<IntegerLiteral>(CO->getTrueExpr());
+    if (!LHS) {
+      if (auto *UO = dyn_cast<UnaryOperator>(CO->getTrueExpr())) {
+        if (UO->getOpcode() == UO_Minus)
+          LHS = dyn_cast<IntegerLiteral>(UO->getSubExpr());
+        if (!LHS)
+          return;
+      } else {
+        return;
+      }
+    }
+
+    const auto *RHS = dyn_cast<IntegerLiteral>(CO->getFalseExpr());
+    if (!RHS) {
+      if (auto *UO = dyn_cast<UnaryOperator>(CO->getFalseExpr())) {
+        if (UO->getOpcode() == UO_Minus)
+          RHS = dyn_cast<IntegerLiteral>(UO->getSubExpr());
+        if (!RHS)
+          return;
+      } else {
+        return;
+      }
+    }
+
+    if ((LHS->getValue() == 0 || LHS->getValue() == 1) &&
+        (RHS->getValue() == 0 || RHS->getValue() == 1))
+      // Do not diagnose common idioms
+      return;
+    if (LHS->getValue() != 0 && LHS->getValue() != 0)
+      S.Diag(ExprLoc, diag::warn_integer_constants_in_conditional_always_true);
+  }
+}
+
 static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
                                     SourceLocation CC,
                                     bool *ICContext = nullptr,
@@ -11708,6 +11746,9 @@ static void CheckConditionalOperator(Sema &S, ConditionalOperator *E,
   CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious);
   CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);
 
+  if (T->isBooleanType())
+    DiagnoseIntInBoolContext(S, E);
+
   // If -Wconversion would have warned about either of the candidates
   // for a signedness conversion to the context type...
   if (!Suspicious) return;
diff --git a/test/Sema/warn-integer-constants-in-ternary.c b/test/Sema/warn-integer-constants-in-ternary.c
new file mode 100644
index 0000000000..287b91ecdb
--- /dev/null
+++ b/test/Sema/warn-integer-constants-in-ternary.c
@@ -0,0 +1,32 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wtautological-constant-compare %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wtautological-constant-compare %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s
+
+#define ONE 1
+#define TWO 2
+
+#define TERN(c, l, r) c ? l : r
+
+#ifdef __cplusplus
+typedef bool boolean;
+#else
+typedef _Bool boolean;
+#endif
+
+void test(boolean a) {
+  boolean r;
+  r = a ? (1) : TWO;
+  r = a ? 3 : TWO; // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}}
+  r = a ? -2 : 0;  // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}}
+  r = a ? 3 : -2;  // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}}
+  r = a ? 0 : TWO;
+  r = a ? 3 : ONE; // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}}
+  r = a ? ONE : 0;
+  r = a ? 0 : -0;
+  r = a ? 1 : 0;
+  r = a ? ONE : 0;
+  r = a ? ONE : ONE;
+  r = TERN(a, 4, 8);   // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}}
+  r = TERN(a, -1, -8); // expected-warning {{converting the result of '?:' with integer constants to a boolean always evaluates to 'true'}}
+}
-- 
2.40.0