From b0f0a1ea03b55df2c0fcd7e7d56006b7a849124b Mon Sep 17 00:00:00 2001 From: Roman Lebedev Date: Wed, 20 Sep 2017 09:54:47 +0000 Subject: [Sema] Move some stuff into -Wtautological-unsigned-enum-zero-compare Recommit. Original commit was reverted because buildbots broke. The error was only reproducible in the build with assertions. The problem was that the diagnostic expected true/false as bool, while it was provided as string "true"/"false". Summary: As requested by Sam McCall: > Enums (not new I guess). Typical case: if (enum < 0 || enum > MAX) > The warning strongly suggests that the enum < 0 check has no effect > (for enums with nonnegative ranges). > Clang doesn't seem to optimize such checks out though, and they seem > likely to catch bugs in some cases. Yes, only if there's UB elsewhere, > but I assume not optimizing out these checks indicates a deliberate > decision to stay somewhat compatible with a technically-incorrect > mental model. > If this is the case, should we move these to a > -Wtautological-compare-enum subcategory? Reviewers: rjmccall, rsmith, aaron.ballman, sammccall, bkramer, djasper Reviewed By: aaron.ballman Subscribers: jroelofs, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D37629 llvm-svn: 313745 --- clang/lib/Sema/SemaChecking.cpp | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) (limited to 'clang/lib/Sema/SemaChecking.cpp') diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 212fe65c76f..87634ad57e5 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -8583,7 +8583,7 @@ bool CheckTautologicalComparisonWithZero(Sema &S, BinaryOperator *E) { // bool values are handled by DiagnoseOutOfRangeComparison(). - BinaryOperatorKind op = E->getOpcode(); + BinaryOperatorKind Op = E->getOpcode(); if (E->isValueDependent()) return false; @@ -8592,22 +8592,26 @@ bool CheckTautologicalComparisonWithZero(Sema &S, BinaryOperator *E) { bool Match = true; - 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(); + if (Op == BO_LT && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) { + S.Diag(E->getOperatorLoc(), + HasEnumType(LHS) ? diag::warn_lunsigned_enum_always_true_comparison + : diag::warn_lunsigned_always_true_comparison) + << "< 0" << false << LHS->getSourceRange() << RHS->getSourceRange(); + } else if (Op == BO_GE && isNonBooleanUnsignedValue(LHS) && IsZero(S, RHS)) { + S.Diag(E->getOperatorLoc(), + HasEnumType(LHS) ? diag::warn_lunsigned_enum_always_true_comparison + : diag::warn_lunsigned_always_true_comparison) + << ">= 0" << true << LHS->getSourceRange() << RHS->getSourceRange(); + } else if (Op == BO_GT && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) { + S.Diag(E->getOperatorLoc(), + HasEnumType(RHS) ? diag::warn_runsigned_enum_always_true_comparison + : diag::warn_runsigned_always_true_comparison) + << "0 >" << false << LHS->getSourceRange() << RHS->getSourceRange(); + } else if (Op == BO_LE && isNonBooleanUnsignedValue(RHS) && IsZero(S, LHS)) { + S.Diag(E->getOperatorLoc(), + HasEnumType(RHS) ? diag::warn_runsigned_enum_always_true_comparison + : diag::warn_runsigned_always_true_comparison) + << "0 <=" << true << LHS->getSourceRange() << RHS->getSourceRange(); } else Match = false; -- cgit v1.2.3