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
Fri Dec 8 08:55:19 PST 2017
Sorry, it seems it was r320124 that caused this; I shouldn't be
sheriffing after-hours.
I've reverted that one in r320162.
On Thu, Dec 7, 2017 at 9:20 PM, Hans Wennborg <hans at chromium.org> wrote:
> 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