Author: rsmith Date: Thu Dec 7 17:00:27 2017 New Revision: 320124 URL: http://llvm.org/viewvc/llvm-project?rev=320124&view=rev Log: Fold together the in-range and out-of-range portions of -Wtautological-compare.
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp Modified: cfe/trunk/lib/Sema/SemaChecking.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320124&r1=320123&r2=320124&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Dec 7 17:00:27 2017 @@ -8801,12 +8801,7 @@ static bool CheckTautologicalComparison( Expr *Constant, Expr *Other, const llvm::APSInt &Value, bool RhsConstant) { - // Disable warning in template instantiations - // and only analyze <, >, <= and >= operations. - if (S.inTemplateInstantiation() || !E->isRelationalOp()) - return false; - - if (IsEnumConstOrFromMacro(S, Constant)) + if (S.inTemplateInstantiation()) return false; Expr *OriginalOther = Other; @@ -8833,94 +8828,23 @@ static bool CheckTautologicalComparison( OtherRange.Width = std::min(Bitfield->getBitWidthValue(S.Context), OtherRange.Width); - // Check whether the constant value can be represented in OtherRange. Bail - // out if so; this isn't an out-of-range comparison. + // Determine the promoted range of the other type and see if a comparison of + // the constant against that range is tautological. PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(), Value.isUnsigned()); - auto Cmp = OtherPromotedRange.compare(Value); - if (Cmp != PromotedRange::Min && Cmp != PromotedRange::Max && - Cmp != PromotedRange::OnlyValue) - return false; - auto Result = PromotedRange::constantValue(E->getOpcode(), Cmp, RhsConstant); if (!Result) return false; - // Should be enough for uint128 (39 decimal digits) - SmallString<64> PrettySourceValue; - llvm::raw_svector_ostream OS(PrettySourceValue); - OS << Value; - - // FIXME: We use a somewhat different formatting for the cases involving - // boolean values for historical reasons. We should pick a consistent way - // of presenting these diagnostics. - if (Other->isKnownToHaveBooleanValue()) { - S.DiagRuntimeBehavior( - E->getOperatorLoc(), E, - S.PDiag(diag::warn_tautological_bool_compare) - << OS.str() << classifyConstantValue(Constant) - << OtherT << !OtherT->isBooleanType() << *Result - << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange()); - return true; - } - - unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0) - ? (HasEnumType(OriginalOther) - ? diag::warn_unsigned_enum_always_true_comparison - : diag::warn_unsigned_always_true_comparison) - : diag::warn_tautological_constant_compare; - - S.Diag(E->getOperatorLoc(), Diag) - << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result - << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange(); - - return true; -} - -static bool DiagnoseOutOfRangeComparison(Sema &S, BinaryOperator *E, - Expr *Constant, Expr *Other, - const llvm::APSInt &Value, - bool RhsConstant) { - // Disable warning in template instantiations. - if (S.inTemplateInstantiation()) - return false; - - Constant = Constant->IgnoreParenImpCasts(); - Other = Other->IgnoreParenImpCasts(); - - // TODO: Investigate using GetExprRange() to get tighter bounds - // on the bit ranges. - QualType OtherT = Other->getType(); - if (const auto *AT = OtherT->getAs<AtomicType>()) - OtherT = AT->getValueType(); - IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT); - - // Whether we're treating Other as being a bool because of the form of - // expression despite it having another type (typically 'int' in C). - bool OtherIsBooleanDespiteType = - !OtherT->isBooleanType() && Other->isKnownToHaveBooleanValue(); - if (OtherIsBooleanDespiteType) - OtherRange = IntRange::forBoolType(); - - if (FieldDecl *Bitfield = Other->getSourceBitField()) - if (!Bitfield->getBitWidth()->isValueDependent()) - OtherRange.Width = - std::min(Bitfield->getBitWidthValue(S.Context), OtherRange.Width); - - // Check whether the constant value can be represented in OtherRange. Bail - // out if so; this isn't an out-of-range comparison. - PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(), - Value.isUnsigned()); - auto Cmp = OtherPromotedRange.compare(Value); - - // If Value is in the range of possible Other values, this comparison is not - // tautological. - if (Cmp & PromotedRange::InRangeFlag) - return false; - - auto IsTrue = PromotedRange::constantValue(E->getOpcode(), Cmp, RhsConstant); - if (!IsTrue) + // Suppress the diagnostic for an in-range comparison if the constant comes + // from a macro or enumerator. We don't want to diagnose + // + // some_long_value <= INT_MAX + // + // when sizeof(int) == sizeof(long). + bool InRange = Cmp & PromotedRange::InRangeFlag; + if (InRange && IsEnumConstOrFromMacro(S, Constant)) return false; // If this is a comparison to an enum constant, include that @@ -8929,6 +8853,7 @@ static bool DiagnoseOutOfRangeComparison if (const DeclRefExpr *DR = dyn_cast<DeclRefExpr>(Constant)) ED = dyn_cast<EnumConstantDecl>(DR->getDecl()); + // Should be enough for uint128 (39 decimal digits) SmallString<64> PrettySourceValue; llvm::raw_svector_ostream OS(PrettySourceValue); if (ED) @@ -8936,14 +8861,30 @@ static bool DiagnoseOutOfRangeComparison else OS << Value; - S.DiagRuntimeBehavior( - E->getOperatorLoc(), E, - S.PDiag(diag::warn_out_of_range_compare) - << OS.str() << classifyConstantValue(Constant) - << OtherT << OtherIsBooleanDespiteType << *IsTrue - << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange()); + // FIXME: We use a somewhat different formatting for the in-range cases and + // cases involving boolean values for historical reasons. We should pick a + // consistent way of presenting these diagnostics. + if (!InRange || Other->isKnownToHaveBooleanValue()) { + S.DiagRuntimeBehavior( + E->getOperatorLoc(), E, + S.PDiag(!InRange ? diag::warn_out_of_range_compare + : diag::warn_tautological_bool_compare) + << OS.str() << classifyConstantValue(Constant) + << OtherT << OtherIsBooleanDespiteType << *Result + << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange()); + } else { + unsigned Diag = (isKnownToHaveUnsignedValue(OriginalOther) && Value == 0) + ? (HasEnumType(OriginalOther) + ? diag::warn_unsigned_enum_always_true_comparison + : diag::warn_unsigned_always_true_comparison) + : diag::warn_tautological_constant_compare; + + S.Diag(E->getOperatorLoc(), Diag) + << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result + << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange(); + } - return true; + return true; } /// Analyze the operands of the given comparison. Implements the @@ -8993,12 +8934,8 @@ static void AnalyzeComparison(Sema &S, B // Check whether an integer constant comparison results in a value // of 'true' or 'false'. - if (CheckTautologicalComparison(S, E, Const, Other, Value, RhsConstant)) return AnalyzeImpConvsInComparison(S, E); - - if (DiagnoseOutOfRangeComparison(S, E, Const, Other, Value, RhsConstant)) - return AnalyzeImpConvsInComparison(S, E); } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits