r320124 - Fold together the in-range and out-of-range portions of -Wtautological-compare.
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Thu Dec 7 21:20:31 PST 2017
I've reverted in r320133 since it caused new warnings in Chromium that
I'm not sure were intentional. See comment on the revert.
On Thu, Dec 7, 2017 at 5:00 PM, Richard Smith via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> 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 at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list