Skip to content

Commit

Permalink
[Diagnostics] Warn if ?: with integer constants always evaluates to true
Browse files Browse the repository at this point in the history
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.

llvm-svn: 372531
  • Loading branch information
davidbolvansky committed Sep 22, 2019
1 parent 914c4c3 commit fb21817
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 0 deletions.
4 changes: 4 additions & 0 deletions clang/include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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">,
Expand Down
41 changes: 41 additions & 0 deletions clang/lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
32 changes: 32 additions & 0 deletions clang/test/Sema/warn-integer-constants-in-ternary.c
Original file line number Diff line number Diff line change
@@ -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'}}
}

0 comments on commit fb21817

Please sign in to comment.