lebedev.ri updated this revision to Diff 115877.
lebedev.ri added a comment.
1. Correctly added `TautologicalUnsignedEnumZeroCompare` to
`TautologicalCompare` group (sigh)
2. Hopefully fixed spelling of the comments and added a bit better comments
3. Fixed variable names to comply with the coding style.
Repository:
rL LLVM
https://reviews.llvm.org/D37629
Files:
include/clang/Basic/DiagnosticGroups.td
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaChecking.cpp
test/Sema/compare.c
test/Sema/tautological-unsigned-enum-zero-compare.c
Index: test/Sema/tautological-unsigned-enum-zero-compare.c
===================================================================
--- /dev/null
+++ test/Sema/tautological-unsigned-enum-zero-compare.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -fsyntax-only -DTEST -verify %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-tautological-unsigned-enum-zero-compare -verify %s
+
+int main() {
+ enum A { A_foo, A_bar };
+ enum A a;
+
+#ifdef TEST
+ if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+ return 0;
+ if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+ return 0;
+ if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+ return 0;
+ if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+ return 0;
+ if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+ return 0;
+ if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+ return 0;
+ if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+ return 0;
+ if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+ return 0;
+#else
+ // expected-no-diagnostics
+ if (a < 0)
+ return 0;
+ if (a >= 0)
+ return 0;
+ if (0 <= a)
+ return 0;
+ if (0 > a)
+ return 0;
+ if (a < 0U)
+ return 0;
+ if (a >= 0U)
+ return 0;
+ if (0U <= a)
+ return 0;
+ if (0U > a)
+ return 0;
+#endif
+
+ return 1;
+}
Index: test/Sema/compare.c
===================================================================
--- test/Sema/compare.c
+++ test/Sema/compare.c
@@ -308,8 +308,59 @@
int rdar8511238() {
enum A { A_foo, A_bar };
enum A a;
+
+ if (a == 0)
+ return 0;
+ if (a != 0)
+ return 0;
if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
- return 0;
+ return 0;
+ if (a <= 0)
+ return 0;
+ if (a > 0)
+ return 0;
+ if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+ return 0;
+
+ if (0 == a)
+ return 0;
+ if (0 != a)
+ return 0;
+ if (0 < a)
+ return 0;
+ if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+ return 0;
+ if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+ return 0;
+ if (0 >= a)
+ return 0;
+
+ if (a == 0U)
+ return 0;
+ if (a != 0U)
+ return 0;
+ if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
+ return 0;
+ if (a <= 0U)
+ return 0;
+ if (a > 0U)
+ return 0;
+ if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}
+ return 0;
+
+ if (0U == a)
+ return 0;
+ if (0U != a)
+ return 0;
+ if (0U < a)
+ return 0;
+ if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}
+ return 0;
+ if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}
+ return 0;
+ if (0U >= a)
+ return 0;
+
return 20;
}
Index: lib/Sema/SemaChecking.cpp
===================================================================
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -8583,35 +8583,89 @@
// bool values are handled by DiagnoseOutOfRangeComparison().
- BinaryOperatorKind op = E->getOpcode();
+ BinaryOperatorKind Op = E->getOpcode();
if (E->isValueDependent())
return false;
Expr *LHS = E->getLHS();
Expr *RHS = E->getRHS();
- bool Match = true;
+ // Result is used for two purposes. If the Result is empty, then we decided
+ // that given comparison is non-tautological, and no warning should emitted.
+ // Alternatively, if the Result is not empty, then it contains the result of
+ // the comparison. Since the comparison is tautological, it always evaluates
+ // to the same result - be it either true or false.
+ llvm::Optional<bool> Result;
- if (op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
- S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison)
- << "< 0" << "false" << HasEnumType(LHS)
- << LHS->getSourceRange() << RHS->getSourceRange();
- } else if (op == BO_GE && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
- S.Diag(E->getOperatorLoc(), diag::warn_lunsigned_always_true_comparison)
- << ">= 0" << "true" << HasEnumType(LHS)
- << LHS->getSourceRange() << RHS->getSourceRange();
- } else if (op == BO_GT && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
- S.Diag(E->getOperatorLoc(), diag::warn_runsigned_always_true_comparison)
- << "0 >" << "false" << HasEnumType(RHS)
- << LHS->getSourceRange() << RHS->getSourceRange();
- } else if (op == BO_LE && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
- S.Diag(E->getOperatorLoc(), diag::warn_runsigned_always_true_comparison)
- << "0 <=" << "true" << HasEnumType(RHS)
- << LHS->getSourceRange() << RHS->getSourceRange();
- } else
- Match = false;
+ // The comparison has two sides - left and right.
+ // This function is called when some side is a constant and other is a
+ // variable. In Value we store the side which contains the variable.
+ Expr *Value;
- return Match;
+ const char *Cmp; // Simplified, pretty-printed comparison string
+ // Similar to E->getOpcodeStr(), but with extra 0 on either LHS or RHS
+
+ if (Op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
+ Result = false;
+ Value = LHS;
+ Cmp = "< 0";
+ } else if (Op == BO_GE && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) {
+ Result = true;
+ Value = LHS;
+ Cmp = ">= 0";
+ } else if (Op == BO_GT && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
+ Result = false;
+ Value = RHS;
+ Cmp = "0 >";
+ } else if (Op == BO_LE && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) {
+ Result = true;
+ Value = RHS;
+ Cmp = "0 <=";
+ }
+
+ if (!Result)
+ return false;
+
+ const bool ValueHasEnumType = HasEnumType(Value);
+ enum IsEnum { False, True, SizeOfIsEnum };
+ static const struct Diagnostics {
+ unsigned BO_LT_OP[SizeOfIsEnum];
+ unsigned BO_GE_OP[SizeOfIsEnum];
+ unsigned BO_GT_OP[SizeOfIsEnum];
+ unsigned BO_LE_OP[SizeOfIsEnum];
+ } DiagnosticsTable = {
+ {diag::warn_lunsigned_always_true_comparison,
+ diag::warn_lunsigned_enum_always_true_comparison},
+ {diag::warn_lunsigned_always_true_comparison,
+ diag::warn_lunsigned_enum_always_true_comparison},
+ {diag::warn_runsigned_always_true_comparison,
+ diag::warn_runsigned_enum_always_true_comparison},
+ {diag::warn_runsigned_always_true_comparison,
+ diag::warn_runsigned_enum_always_true_comparison},
+ };
+
+ unsigned DiagID;
+ switch (Op) {
+ case BO_LT:
+ DiagID = DiagnosticsTable.BO_LT_OP[ValueHasEnumType];
+ break;
+ case BO_GE:
+ DiagID = DiagnosticsTable.BO_GE_OP[ValueHasEnumType];
+ break;
+ case BO_GT:
+ DiagID = DiagnosticsTable.BO_GT_OP[ValueHasEnumType];
+ break;
+ case BO_LE:
+ DiagID = DiagnosticsTable.BO_LE_OP[ValueHasEnumType];
+ break;
+ default:
+ llvm_unreachable("can't get here");
+ }
+
+ S.Diag(E->getOperatorLoc(), DiagID)
+ << Cmp << *Result << LHS->getSourceRange() << RHS->getSourceRange();
+
+ return true;
}
void DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E, Expr *Constant,
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -5900,19 +5900,26 @@
"member function %q1 is declared const here|"
"%select{|nested }1data member %2 declared const here}0">;
+def warn_lunsigned_always_true_comparison : Warning<
+ "comparison of unsigned expression %0 is always %select{false|true}1">,
+ InGroup<TautologicalUnsignedZeroCompare>;
+def warn_runsigned_always_true_comparison : Warning<
+ "comparison of %0 unsigned expression is always %select{false|true}1">,
+ InGroup<TautologicalUnsignedZeroCompare>;
+def warn_lunsigned_enum_always_true_comparison : Warning<
+ "comparison of unsigned enum expression %0 is always %select{false|true}1">,
+ InGroup<TautologicalUnsignedEnumZeroCompare>;
+def warn_runsigned_enum_always_true_comparison : Warning<
+ "comparison of %0 unsigned enum expression is always %select{false|true}1">,
+ InGroup<TautologicalUnsignedEnumZeroCompare>;
+
def warn_mixed_sign_comparison : Warning<
"comparison of integers of different signs: %0 and %1">,
InGroup<SignCompare>, DefaultIgnore;
-def warn_lunsigned_always_true_comparison : Warning<
- "comparison of unsigned%select{| enum}2 expression %0 is always %1">,
- InGroup<TautologicalUnsignedZeroCompare>;
def warn_out_of_range_compare : Warning<
"comparison of %select{constant %0|true|false}1 with "
"%select{expression of type %2|boolean expression}3 is always "
"%select{false|true}4">, InGroup<TautologicalOutOfRangeCompare>;
-def warn_runsigned_always_true_comparison : Warning<
- "comparison of %0 unsigned%select{| enum}2 expression is always %1">,
- InGroup<TautologicalUnsignedZeroCompare>;
def warn_comparison_of_mixed_enum_types : Warning<
"comparison of two values with different enumeration types"
"%diff{ ($ and $)|}0,1">,
Index: include/clang/Basic/DiagnosticGroups.td
===================================================================
--- include/clang/Basic/DiagnosticGroups.td
+++ include/clang/Basic/DiagnosticGroups.td
@@ -428,12 +428,14 @@
def StringPlusChar : DiagGroup<"string-plus-char">;
def StrncatSize : DiagGroup<"strncat-size">;
def TautologicalUnsignedZeroCompare : DiagGroup<"tautological-unsigned-zero-compare">;
+def TautologicalUnsignedEnumZeroCompare : DiagGroup<"tautological-unsigned-enum-zero-compare">;
def TautologicalOutOfRangeCompare : DiagGroup<"tautological-constant-out-of-range-compare">;
def TautologicalPointerCompare : DiagGroup<"tautological-pointer-compare">;
def TautologicalOverlapCompare : DiagGroup<"tautological-overlap-compare">;
def TautologicalUndefinedCompare : DiagGroup<"tautological-undefined-compare">;
def TautologicalCompare : DiagGroup<"tautological-compare",
[TautologicalUnsignedZeroCompare,
+ TautologicalUnsignedEnumZeroCompare,
TautologicalOutOfRangeCompare,
TautologicalPointerCompare,
TautologicalOverlapCompare,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
