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 <h...@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@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
