<div dir="auto"><div><div class="gmail_extra"><div class="gmail_quote">On 8 Dec 2017 08:54, "Hans Wennborg via cfe-commits" <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br type="attribution"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: hans<br>
Date: Fri Dec 8 08:54:08 2017<br>
New Revision: 320162<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=320162&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=320162&view=rev</a><br>
Log:<br>
Revert "Unify implementation of our two different flavours of -Wtautological-compare."<br>
<br>
> Unify implementation of our two different flavours of -Wtautological-compare.<br>
><br>
> In so doing, fix a handful of remaining bugs where we would report false<br>
> positives or false negatives if we promote a signed value to an unsigned type<br>
> for the comparison.<br>
<br>
This caused a new warning in Chromium:<br>
<br>
../../base/trace_event/trace_<wbr>log.cc:1545:29: error: comparison of constant 64<br>
with expression of type 'unsigned int' is always true<br>
[-Werror,-Wtautological-<wbr>constant-out-of-range-compare]<br>
DCHECK(handle.event_index < TraceBufferChunk::<wbr>kTraceBufferChunkSize);<br>
~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~<wbr>~~~~~~~~~<br>
<br>
The 'unsigned int' is really a 6-bit bitfield, which is why it's always<br>
less than 64.<br>
<br>
I thought we didn't use to warn (with out-of-range-compare) when comparing<br>
against the boundaries of a type?<br></blockquote></div></div></div><div dir="auto"><br></div><div dir="auto">This isn't a "boundary of the type" case, though -- 64 is out of range for a 6 bit bitfield. Your CHECK does nothing. Do you think that it's not reasonable to warn on this bug, or that it's not a bug?</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_extra"><div class="gmail_quote"><blockquote class="quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Modified:<br>
cfe/trunk/lib/Sema/<wbr>SemaChecking.cpp<br>
cfe/trunk/test/Sema/<wbr>tautological-constant-compare.<wbr>c<br>
cfe/trunk/test/Sema/<wbr>tautological-constant-enum-<wbr>compare.c<br>
cfe/trunk/test/SemaCXX/<wbr>compare.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/<wbr>SemaChecking.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=320162&r1=320161&r2=320162&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Sema/<wbr>SemaChecking.cpp?rev=320162&<wbr>r1=320161&r2=320162&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/<wbr>SemaChecking.cpp (original)<br>
+++ cfe/trunk/lib/Sema/<wbr>SemaChecking.cpp Fri Dec 8 08:54:08 2017<br>
@@ -8662,113 +8662,54 @@ static bool isKnownToHaveUnsignedValue(E<br>
}<br>
<br>
namespace {<br>
-/// The promoted range of values of a type. In general this has the<br>
-/// following structure:<br>
-///<br>
-/// |-----------| . . . |-----------|<br>
-/// ^ ^ ^ ^<br>
-/// Min HoleMin HoleMax Max<br>
-///<br>
-/// ... where there is only a hole if a signed type is promoted to unsigned<br>
-/// (in which case Min and Max are the smallest and largest representable<br>
-/// values).<br>
-struct PromotedRange {<br>
- // Min, or HoleMax if there is a hole.<br>
- llvm::APSInt PromotedMin;<br>
- // Max, or HoleMin if there is a hole.<br>
- llvm::APSInt PromotedMax;<br>
-<br>
- PromotedRange(IntRange R, unsigned BitWidth, bool Unsigned) {<br>
- if (R.Width == 0)<br>
- PromotedMin = PromotedMax = llvm::APSInt(BitWidth, Unsigned);<br>
- else {<br>
- PromotedMin = llvm::APSInt::getMinValue(R.<wbr>Width, R.NonNegative)<br>
- .extOrTrunc(BitWidth);<br>
- PromotedMin.setIsUnsigned(<wbr>Unsigned);<br>
-<br>
- PromotedMax = llvm::APSInt::getMaxValue(R.<wbr>Width, R.NonNegative)<br>
- .extOrTrunc(BitWidth);<br>
- PromotedMax.setIsUnsigned(<wbr>Unsigned);<br>
- }<br>
- }<br>
<br>
- // Determine whether this range is contiguous (has no hole).<br>
- bool isContiguous() const { return PromotedMin <= PromotedMax; }<br>
+enum class LimitType {<br>
+ Max = 1U << 0U, // e.g. 32767 for short<br>
+ Min = 1U << 1U, // e.g. -32768 for short<br>
+ Both = Max | Min // When the value is both the Min and the Max limit at the<br>
+ // same time; e.g. in C++, A::a in enum A { a = 0 };<br>
+};<br>
<br>
- // Where a constant value is within the range.<br>
- enum ComparisonResult {<br>
- LT = 0x1,<br>
- LE = 0x2,<br>
- GT = 0x4,<br>
- GE = 0x8,<br>
- EQ = 0x10,<br>
- NE = 0x20,<br>
- InRangeFlag = 0x40,<br>
-<br>
- Less = LE | LT | NE,<br>
- Min = LE | InRangeFlag,<br>
- InRange = InRangeFlag,<br>
- Max = GE | InRangeFlag,<br>
- Greater = GE | GT | NE,<br>
+} // namespace<br>
<br>
- OnlyValue = LE | GE | EQ | InRangeFlag,<br>
- InHole = NE<br>
- };<br>
+/// Checks whether Expr 'Constant' may be the<br>
+/// std::numeric_limits<>::max() or std::numeric_limits<>::min()<br>
+/// of the Expr 'Other'. If true, then returns the limit type (min or max).<br>
+/// The Value is the evaluation of Constant<br>
+static llvm::Optional<LimitType> IsTypeLimit(Sema &S, Expr *Constant,<br>
+ Expr *Other,<br>
+ const llvm::APSInt &Value) {<br>
+ if (IsEnumConstOrFromMacro(S, Constant))<br>
+ return llvm::Optional<LimitType>();<br>
<br>
- ComparisonResult compare(const llvm::APSInt &Value) const {<br>
- assert(Value.getBitWidth() == PromotedMin.getBitWidth() &&<br>
- Value.isUnsigned() == PromotedMin.isUnsigned());<br>
- if (!isContiguous()) {<br>
- assert(Value.isUnsigned() && "discontiguous range for signed compare");<br>
- if (Value.isMinValue()) return Min;<br>
- if (Value.isMaxValue()) return Max;<br>
- if (Value >= PromotedMin) return InRange;<br>
- if (Value <= PromotedMax) return InRange;<br>
- return InHole;<br>
- }<br>
+ if (isKnownToHaveUnsignedValue(<wbr>Other) && Value == 0)<br>
+ return LimitType::Min;<br>
<br>
- switch (llvm::APSInt::compareValues(<wbr>Value, PromotedMin)) {<br>
- case -1: return Less;<br>
- case 0: return PromotedMin == PromotedMax ? OnlyValue : Min;<br>
- case 1:<br>
- switch (llvm::APSInt::compareValues(<wbr>Value, PromotedMax)) {<br>
- case -1: return InRange;<br>
- case 0: return Max;<br>
- case 1: return Greater;<br>
- }<br>
- }<br>
+ // TODO: Investigate using GetExprRange() to get tighter bounds<br>
+ // on the bit ranges.<br>
+ QualType OtherT = Other->IgnoreParenImpCasts()-><wbr>getType();<br>
+ if (const auto *AT = OtherT->getAs<AtomicType>())<br>
+ OtherT = AT->getValueType();<br>
<br>
- llvm_unreachable("impossible compare result");<br>
- }<br>
+ IntRange OtherRange = IntRange::forValueOfType(S.<wbr>Context, OtherT);<br>
+ if (Other-><wbr>isKnownToHaveBooleanValue())<br>
+ OtherRange = IntRange::forBoolType();<br>
<br>
- static llvm::Optional<bool> constantValue(<wbr>BinaryOperatorKind Op,<br>
- ComparisonResult R,<br>
- bool ConstantOnRHS) {<br>
- ComparisonResult TrueFlag, FalseFlag;<br>
- if (Op == BO_EQ) {<br>
- TrueFlag = EQ;<br>
- FalseFlag = NE;<br>
- } else if (Op == BO_NE) {<br>
- TrueFlag = NE;<br>
- FalseFlag = EQ;<br>
- } else {<br>
- if ((Op == BO_LT || Op == BO_GE) ^ ConstantOnRHS) {<br>
- TrueFlag = LT;<br>
- FalseFlag = GE;<br>
- } else {<br>
- TrueFlag = GT;<br>
- FalseFlag = LE;<br>
- }<br>
- if (Op == BO_GE || Op == BO_LE)<br>
- std::swap(TrueFlag, FalseFlag);<br>
- }<br>
- if (R & TrueFlag)<br>
- return true;<br>
- if (R & FalseFlag)<br>
- return false;<br>
- return llvm::None;<br>
- }<br>
-};<br>
+ // Special-case for C++ for enum with one enumerator with value of 0.<br>
+ if (OtherRange.Width == 0)<br>
+ return Value == 0 ? LimitType::Both : llvm::Optional<LimitType>();<br>
+<br>
+ if (llvm::APSInt::isSameValue(<br>
+ llvm::APSInt::getMaxValue(<wbr>OtherRange.Width, OtherRange.NonNegative),<br>
+ Value))<br>
+ return LimitType::Max;<br>
+<br>
+ if (llvm::APSInt::isSameValue(<br>
+ llvm::APSInt::getMinValue(<wbr>OtherRange.Width, OtherRange.NonNegative),<br>
+ Value))<br>
+ return LimitType::Min;<br>
+<br>
+ return llvm::None;<br>
}<br>
<br>
static bool HasEnumType(Expr *E) {<br>
@@ -8806,47 +8747,29 @@ static bool CheckTautologicalComparison(<br>
if (S.inTemplateInstantiation() || !E->isRelationalOp())<br>
return false;<br>
<br>
- if (IsEnumConstOrFromMacro(S, Constant))<br>
- return false;<br>
-<br>
- Expr *OriginalOther = Other;<br>
+ BinaryOperatorKind Op = E->getOpcode();<br>
<br>
- Constant = Constant->IgnoreParenImpCasts(<wbr>);<br>
- Other = Other->IgnoreParenImpCasts();<br>
-<br>
- // TODO: Investigate using GetExprRange() to get tighter bounds<br>
- // on the bit ranges.<br>
- QualType OtherT = Other->getType();<br>
- if (const auto *AT = OtherT->getAs<AtomicType>())<br>
- OtherT = AT->getValueType();<br>
- IntRange OtherRange = IntRange::forValueOfType(S.<wbr>Context, OtherT);<br>
-<br>
- // Whether we're treating Other as being a bool because of the form of<br>
- // expression despite it having another type (typically 'int' in C).<br>
- bool OtherIsBooleanDespiteType =<br>
- !OtherT->isBooleanType() && Other-><wbr>isKnownToHaveBooleanValue();<br>
- if (OtherIsBooleanDespiteType)<br>
- OtherRange = IntRange::forBoolType();<br>
-<br>
- if (FieldDecl *Bitfield = Other->getSourceBitField())<br>
- if (!Bitfield->getBitWidth()-><wbr>isValueDependent())<br>
- OtherRange.Width =<br>
- std::min(Bitfield-><wbr>getBitWidthValue(S.Context), OtherRange.Width);<br>
-<br>
- // Check whether the constant value can be represented in OtherRange. Bail<br>
- // out if so; this isn't an out-of-range comparison.<br>
- PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(),<br>
- Value.isUnsigned());<br>
-<br>
- auto Cmp = OtherPromotedRange.compare(<wbr>Value);<br>
- if (Cmp != PromotedRange::Min && Cmp != PromotedRange::Max &&<br>
- Cmp != PromotedRange::OnlyValue)<br>
+ QualType OType = Other->IgnoreParenImpCasts()-><wbr>getType();<br>
+ if (!OType->isIntegerType())<br>
return false;<br>
<br>
- auto Result = PromotedRange::constantValue(<wbr>E->getOpcode(), Cmp, RhsConstant);<br>
- if (!Result)<br>
+ // Determine which limit (min/max) the constant is, if either.<br>
+ llvm::Optional<LimitType> ValueType = IsTypeLimit(S, Constant, Other, Value);<br>
+ if (!ValueType)<br>
return false;<br>
<br>
+ bool ConstIsLowerBound = (Op == BO_LT || Op == BO_LE) ^ RhsConstant;<br>
+ bool ResultWhenConstEqualsOther = (Op == BO_LE || Op == BO_GE);<br>
+ if (ValueType != LimitType::Both) {<br>
+ bool ResultWhenConstNeOther =<br>
+ ConstIsLowerBound ^ (ValueType == LimitType::Max);<br>
+ if (ResultWhenConstEqualsOther != ResultWhenConstNeOther)<br>
+ return false; // The comparison is not tautological.<br>
+ } else if (ResultWhenConstEqualsOther == ConstIsLowerBound)<br>
+ return false; // The comparison is not tautological.<br>
+<br>
+ const bool Result = ResultWhenConstEqualsOther;<br>
+<br>
// Should be enough for uint128 (39 decimal digits)<br>
SmallString<64> PrettySourceValue;<br>
llvm::raw_svector_ostream OS(PrettySourceValue);<br>
@@ -8859,20 +8782,20 @@ static bool CheckTautologicalComparison(<br>
S.DiagRuntimeBehavior(<br>
E->getOperatorLoc(), E,<br>
S.PDiag(diag::warn_<wbr>tautological_bool_compare)<br>
- << OS.str() << classifyConstantValue(<wbr>Constant)<br>
- << OtherT << !OtherT->isBooleanType() << *Result<br>
+ << OS.str() << classifyConstantValue(<wbr>Constant->IgnoreParenImpCasts(<wbr>))<br>
+ << OType << !OType->isBooleanType() << Result<br>
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange())<wbr>;<br>
return true;<br>
}<br>
<br>
- unsigned Diag = (isKnownToHaveUnsignedValue(<wbr>OriginalOther) && Value == 0)<br>
- ? (HasEnumType(OriginalOther)<br>
+ unsigned Diag = (isKnownToHaveUnsignedValue(<wbr>Other) && Value == 0)<br>
+ ? (HasEnumType(Other)<br>
? diag::warn_unsigned_enum_<wbr>always_true_comparison<br>
: diag::warn_unsigned_always_<wbr>true_comparison)<br>
: diag::warn_tautological_<wbr>constant_compare;<br>
<br>
S.Diag(E->getOperatorLoc(), Diag)<br>
- << RhsConstant << OtherT << E->getOpcodeStr() << OS.str() << *Result<br>
+ << RhsConstant << OType << E->getOpcodeStr() << OS.str() << Result<br>
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();<br>
<br>
return true;<br>
@@ -8894,6 +8817,7 @@ static bool DiagnoseOutOfRangeComparison<br>
QualType OtherT = Other->getType();<br>
if (const auto *AT = OtherT->getAs<AtomicType>())<br>
OtherT = AT->getValueType();<br>
+<br>
IntRange OtherRange = IntRange::forValueOfType(S.<wbr>Context, OtherT);<br>
<br>
// Whether we're treating Other as being a bool because of the form of<br>
@@ -8903,25 +8827,91 @@ static bool DiagnoseOutOfRangeComparison<br>
if (OtherIsBooleanDespiteType)<br>
OtherRange = IntRange::forBoolType();<br>
<br>
- if (FieldDecl *Bitfield = Other->getSourceBitField())<br>
- if (!Bitfield->getBitWidth()-><wbr>isValueDependent())<br>
- OtherRange.Width =<br>
- std::min(Bitfield-><wbr>getBitWidthValue(S.Context), OtherRange.Width);<br>
+ unsigned OtherWidth = OtherRange.Width;<br>
+<br>
+ BinaryOperatorKind op = E->getOpcode();<br>
+ bool IsTrue = true;<br>
<br>
// Check whether the constant value can be represented in OtherRange. Bail<br>
// out if so; this isn't an out-of-range comparison.<br>
- PromotedRange OtherPromotedRange(OtherRange, Value.getBitWidth(),<br>
- Value.isUnsigned());<br>
- auto Cmp = OtherPromotedRange.compare(<wbr>Value);<br>
-<br>
- // If Value is in the range of possible Other values, this comparison is not<br>
- // tautological.<br>
- if (Cmp & PromotedRange::InRangeFlag)<br>
- return false;<br>
+ {<br>
+ QualType ConstantT = Constant->getType();<br>
+ QualType CommonT = E->getLHS()->getType();<br>
<br>
- auto IsTrue = PromotedRange::constantValue(<wbr>E->getOpcode(), Cmp, RhsConstant);<br>
- if (!IsTrue)<br>
- return false;<br>
+ if (S.Context.<wbr>hasSameUnqualifiedType(OtherT, ConstantT) &&<br>
+ !OtherIsBooleanDespiteType)<br>
+ return false;<br>
+ assert((OtherT->isIntegerType(<wbr>) && ConstantT->isIntegerType()) &&<br>
+ "comparison with non-integer type");<br>
+<br>
+ bool ConstantSigned = ConstantT-><wbr>isSignedIntegerType();<br>
+ bool CommonSigned = CommonT->isSignedIntegerType()<wbr>;<br>
+<br>
+ bool EqualityOnly = false;<br>
+<br>
+ if (CommonSigned) {<br>
+ // The common type is signed, therefore no signed to unsigned conversion.<br>
+ if (!OtherRange.NonNegative) {<br>
+ // Check that the constant is representable in type OtherT.<br>
+ if (ConstantSigned) {<br>
+ if (OtherWidth >= Value.getMinSignedBits())<br>
+ return false;<br>
+ } else { // !ConstantSigned<br>
+ if (OtherWidth >= Value.getActiveBits() + 1)<br>
+ return false;<br>
+ }<br>
+ } else { // !OtherSigned<br>
+ // Check that the constant is representable in type OtherT.<br>
+ // Negative values are out of range.<br>
+ if (ConstantSigned) {<br>
+ if (Value.isNonNegative() && OtherWidth >= Value.getActiveBits())<br>
+ return false;<br>
+ } else { // !ConstantSigned<br>
+ if (OtherWidth >= Value.getActiveBits())<br>
+ return false;<br>
+ }<br>
+ }<br>
+ } else { // !CommonSigned<br>
+ if (OtherRange.NonNegative) {<br>
+ if (OtherWidth >= Value.getActiveBits())<br>
+ return false;<br>
+ } else { // OtherSigned<br>
+ assert(!ConstantSigned &&<br>
+ "Two signed types converted to unsigned types.");<br>
+ // Check to see if the constant is representable in OtherT.<br>
+ if (OtherWidth > Value.getActiveBits())<br>
+ return false;<br>
+ // Check to see if the constant is equivalent to a negative value<br>
+ // cast to CommonT.<br>
+ if (S.Context.getIntWidth(<wbr>ConstantT) ==<br>
+ S.Context.getIntWidth(CommonT) &&<br>
+ Value.isNegative() && Value.getMinSignedBits() <= OtherWidth)<br>
+ return false;<br>
+ // The constant value rests between values that OtherT can represent<br>
+ // after conversion. Relational comparison still works, but equality<br>
+ // comparisons will be tautological.<br>
+ EqualityOnly = true;<br>
+ }<br>
+ }<br>
+<br>
+ bool PositiveConstant = !ConstantSigned || Value.isNonNegative();<br>
+<br>
+ if (op == BO_EQ || op == BO_NE) {<br>
+ IsTrue = op == BO_NE;<br>
+ } else if (EqualityOnly) {<br>
+ return false;<br>
+ } else if (RhsConstant) {<br>
+ if (op == BO_GT || op == BO_GE)<br>
+ IsTrue = !PositiveConstant;<br>
+ else // op == BO_LT || op == BO_LE<br>
+ IsTrue = PositiveConstant;<br>
+ } else {<br>
+ if (op == BO_LT || op == BO_LE)<br>
+ IsTrue = !PositiveConstant;<br>
+ else // op == BO_GT || op == BO_GE<br>
+ IsTrue = PositiveConstant;<br>
+ }<br>
+ }<br>
<br>
// If this is a comparison to an enum constant, include that<br>
// constant in the diagnostic.<br>
@@ -8940,7 +8930,7 @@ static bool DiagnoseOutOfRangeComparison<br>
E->getOperatorLoc(), E,<br>
S.PDiag(diag::warn_out_of_<wbr>range_compare)<br>
<< OS.str() << classifyConstantValue(<wbr>Constant)<br>
- << OtherT << OtherIsBooleanDespiteType << *IsTrue<br>
+ << OtherT << OtherIsBooleanDespiteType << IsTrue<br>
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange())<wbr>;<br>
<br>
return true;<br>
<br>
Modified: cfe/trunk/test/Sema/<wbr>tautological-constant-compare.<wbr>c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/tautological-constant-compare.c?rev=320162&r1=320161&r2=320162&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/Sema/<wbr>tautological-constant-compare.<wbr>c?rev=320162&r1=320161&r2=<wbr>320162&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Sema/<wbr>tautological-constant-compare.<wbr>c (original)<br>
+++ cfe/trunk/test/Sema/<wbr>tautological-constant-compare.<wbr>c Fri Dec 8 08:54:08 2017<br>
@@ -94,17 +94,15 @@ int main()<br>
if (-32768 >= s)<br>
return 0;<br>
<br>
- // Note: both sides are promoted to unsigned long prior to the comparison, so<br>
- // it is perfectly possible for a short to compare greater than 32767UL.<br>
if (s == 32767UL)<br>
return 0;<br>
if (s != 32767UL)<br>
return 0;<br>
if (s < 32767UL)<br>
return 0;<br>
- if (s <= 32767UL)<br>
+ if (s <= 32767UL) // expected-warning {{comparison 'short' <= 32767 is always true}}<br>
return 0;<br>
- if (s > 32767UL)<br>
+ if (s > 32767UL) // expected-warning {{comparison 'short' > 32767 is always false}}<br>
return 0;<br>
if (s >= 32767UL)<br>
return 0;<br>
@@ -113,66 +111,13 @@ int main()<br>
return 0;<br>
if (32767UL != s)<br>
return 0;<br>
- if (32767UL < s)<br>
+ if (32767UL < s) // expected-warning {{comparison 32767 < 'short' is always false}}<br>
return 0;<br>
if (32767UL <= s)<br>
return 0;<br>
if (32767UL > s)<br>
return 0;<br>
- if (32767UL >= s)<br>
- return 0;<br>
-<br>
- if (s == 0UL)<br>
- return 0;<br>
- if (s != 0UL)<br>
- return 0;<br>
- if (s < 0UL) // expected-warning {{comparison of unsigned expression < 0 is always false}}<br>
- return 0;<br>
- if (s <= 0UL)<br>
- return 0;<br>
- if (s > 0UL)<br>
- return 0;<br>
- if (s >= 0UL) // expected-warning {{comparison of unsigned expression >= 0 is always true}}<br>
- return 0;<br>
-<br>
- if (0UL == s)<br>
- return 0;<br>
- if (0UL != s)<br>
- return 0;<br>
- if (0UL < s)<br>
- return 0;<br>
- if (0UL <= s) // expected-warning {{comparison of 0 <= unsigned expression is always true}}<br>
- return 0;<br>
- if (0UL > s) // expected-warning {{comparison of 0 > unsigned expression is always false}}<br>
- return 0;<br>
- if (0UL >= s)<br>
- return 0;<br>
-<br>
- enum { ULONG_MAX = (2UL * (unsigned long)__LONG_MAX__ + 1UL) };<br>
- if (s == 2UL * (unsigned long)__LONG_MAX__ + 1UL)<br>
- return 0;<br>
- if (s != 2UL * (unsigned long)__LONG_MAX__ + 1UL)<br>
- return 0;<br>
- if (s < 2UL * (unsigned long)__LONG_MAX__ + 1UL)<br>
- return 0;<br>
- if (s <= 2UL * (unsigned long)__LONG_MAX__ + 1UL) // expected-warning-re {{comparison 'short' <= {{.*}} is always true}}<br>
- return 0;<br>
- if (s > 2UL * (unsigned long)__LONG_MAX__ + 1UL) // expected-warning-re {{comparison 'short' > {{.*}} is always false}}<br>
- return 0;<br>
- if (s >= 2UL * (unsigned long)__LONG_MAX__ + 1UL)<br>
- return 0;<br>
-<br>
- if (2UL * (unsigned long)__LONG_MAX__ + 1UL == s)<br>
- return 0;<br>
- if (2UL * (unsigned long)__LONG_MAX__ + 1UL != s)<br>
- return 0;<br>
- if (2UL * (unsigned long)__LONG_MAX__ + 1UL < s) // expected-warning-re {{comparison {{.*}} < 'short' is always false}}<br>
- return 0;<br>
- if (2UL * (unsigned long)__LONG_MAX__ + 1UL <= s)<br>
- return 0;<br>
- if (2UL * (unsigned long)__LONG_MAX__ + 1UL > s)<br>
- return 0;<br>
- if (2UL * (unsigned long)__LONG_MAX__ + 1UL >= s) // expected-warning-re {{comparison {{.*}} >= 'short' is always true}}<br>
+ if (32767UL >= s) // expected-warning {{comparison 32767 >= 'short' is always true}}<br>
return 0;<br>
<br>
// FIXME: assumes two's complement<br>
@@ -336,6 +281,8 @@ int main()<br>
if (0 >= s)<br>
return 0;<br>
<br>
+ // However the comparison with 0U would warn<br>
+<br>
unsigned short us = value();<br>
<br>
#ifdef TEST<br>
<br>
Modified: cfe/trunk/test/Sema/<wbr>tautological-constant-enum-<wbr>compare.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/tautological-constant-enum-compare.c?rev=320162&r1=320161&r2=320162&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/Sema/<wbr>tautological-constant-enum-<wbr>compare.c?rev=320162&r1=<wbr>320161&r2=320162&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Sema/<wbr>tautological-constant-enum-<wbr>compare.c (original)<br>
+++ cfe/trunk/test/Sema/<wbr>tautological-constant-enum-<wbr>compare.c Fri Dec 8 08:54:08 2017<br>
@@ -9,96 +9,78 @@ int main() {<br>
<br>
#ifdef SILENCE<br>
// expected-no-diagnostics<br>
-#else<br>
- // If we promote to unsigned, it doesn't matter whether the enum's underlying<br>
- // type was signed.<br>
- if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}<br>
- return 0;<br>
- if (0U >= a)<br>
- return 0;<br>
- if (a > 0U)<br>
- return 0;<br>
- if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}<br>
- return 0;<br>
- if (a <= 0U)<br>
- return 0;<br>
- if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}<br>
- return 0;<br>
- if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}<br>
- return 0;<br>
- if (0U < a)<br>
- return 0;<br>
+#endif<br>
<br>
- if (a < 4294967295U)<br>
+#ifdef UNSIGNED<br>
+#ifndef SILENCE<br>
+ if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}<br>
return 0;<br>
- if (4294967295U >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}<br>
+ if (0 >= a)<br>
return 0;<br>
- if (a > 4294967295U) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}<br>
+ if (a > 0)<br>
return 0;<br>
- if (4294967295U <= a)<br>
+ if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}<br>
return 0;<br>
- if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}<br>
+ if (a <= 0)<br>
return 0;<br>
- if (4294967295U > a)<br>
+ if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}<br>
return 0;<br>
- if (a >= 4294967295U)<br>
+ if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}<br>
return 0;<br>
- if (4294967295U < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}<br>
+ if (0 < a)<br>
return 0;<br>
<br>
- if (a < 2147483647U)<br>
+ if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}<br>
return 0;<br>
- if (2147483647U >= a)<br>
+ if (0U >= a)<br>
return 0;<br>
- if (a > 2147483647U)<br>
+ if (a > 0U)<br>
return 0;<br>
- if (2147483647U <= a)<br>
+ if (0U <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}<br>
return 0;<br>
- if (a <= 2147483647U)<br>
+ if (a <= 0U)<br>
return 0;<br>
- if (2147483647U > a)<br>
+ if (0U > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}<br>
return 0;<br>
- if (a >= 2147483647U)<br>
+ if (a >= 0U) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}<br>
return 0;<br>
- if (2147483647U < a)<br>
+ if (0U < a)<br>
return 0;<br>
-#endif<br>
<br>
-#if defined(UNSIGNED) && !defined(SILENCE)<br>
- if (a < 0) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}<br>
+ if (a < 4294967295)<br>
return 0;<br>
- if (0 >= a)<br>
+ if (4294967295 >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}<br>
return 0;<br>
- if (a > 0)<br>
+ if (a > 4294967295) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}<br>
return 0;<br>
- if (0 <= a) // expected-warning {{comparison of 0 <= unsigned enum expression is always true}}<br>
+ if (4294967295 <= a)<br>
return 0;<br>
- if (a <= 0)<br>
+ if (a <= 4294967295) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}<br>
return 0;<br>
- if (0 > a) // expected-warning {{comparison of 0 > unsigned enum expression is always false}}<br>
+ if (4294967295 > a)<br>
return 0;<br>
- if (a >= 0) // expected-warning {{comparison of unsigned enum expression >= 0 is always true}}<br>
+ if (a >= 4294967295)<br>
return 0;<br>
- if (0 < a)<br>
+ if (4294967295 < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}<br>
return 0;<br>
<br>
- if (a < 4294967295)<br>
+ if (a < 4294967295U)<br>
return 0;<br>
- if (4294967295 >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}<br>
+ if (4294967295U >= a) // expected-warning {{comparison 4294967295 >= 'enum A' is always true}}<br>
return 0;<br>
- if (a > 4294967295) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}<br>
+ if (a > 4294967295U) // expected-warning {{comparison 'enum A' > 4294967295 is always false}}<br>
return 0;<br>
- if (4294967295 <= a)<br>
+ if (4294967295U <= a)<br>
return 0;<br>
- if (a <= 4294967295) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}<br>
+ if (a <= 4294967295U) // expected-warning {{comparison 'enum A' <= 4294967295 is always true}}<br>
return 0;<br>
- if (4294967295 > a)<br>
+ if (4294967295U > a)<br>
return 0;<br>
- if (a >= 4294967295)<br>
+ if (a >= 4294967295U)<br>
return 0;<br>
- if (4294967295 < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}<br>
+ if (4294967295U < a) // expected-warning {{comparison 4294967295 < 'enum A' is always false}}<br>
return 0;<br>
-#else // SIGNED || SILENCE<br>
+#else // SILENCE<br>
if (a < 0)<br>
return 0;<br>
if (0 >= a)<br>
@@ -116,24 +98,23 @@ int main() {<br>
if (0 < a)<br>
return 0;<br>
<br>
-#ifndef SILENCE<br>
- if (a < 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}<br>
+ if (a < 0U)<br>
return 0;<br>
- if (4294967295 >= a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}<br>
+ if (0U >= a)<br>
return 0;<br>
- if (a > 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}<br>
+ if (a > 0U)<br>
return 0;<br>
- if (4294967295 <= a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}<br>
+ if (0U <= a)<br>
return 0;<br>
- if (a <= 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}<br>
+ if (a <= 0U)<br>
return 0;<br>
- if (4294967295 > a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always true}}<br>
+ if (0U > a)<br>
return 0;<br>
- if (a >= 4294967295) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}<br>
+ if (a >= 0U)<br>
return 0;<br>
- if (4294967295 < a) // expected-warning {{comparison of constant 4294967295 with expression of type 'enum A' is always false}}<br>
+ if (0U < a)<br>
return 0;<br>
-#else<br>
+<br>
if (a < 4294967295)<br>
return 0;<br>
if (4294967295 >= a)<br>
@@ -150,10 +131,26 @@ int main() {<br>
return 0;<br>
if (4294967295 < a)<br>
return 0;<br>
-#endif<br>
-#endif<br>
<br>
-#if defined(SIGNED) && !defined(SILENCE)<br>
+ if (a < 4294967295U)<br>
+ return 0;<br>
+ if (4294967295U >= a)<br>
+ return 0;<br>
+ if (a > 4294967295U)<br>
+ return 0;<br>
+ if (4294967295U <= a)<br>
+ return 0;<br>
+ if (a <= 4294967295U)<br>
+ return 0;<br>
+ if (4294967295U > a)<br>
+ return 0;<br>
+ if (a >= 4294967295U)<br>
+ return 0;<br>
+ if (4294967295U < a)<br>
+ return 0;<br>
+#endif<br>
+#elif defined(SIGNED)<br>
+#ifndef SILENCE<br>
if (a < -2147483648) // expected-warning {{comparison 'enum A' < -2147483648 is always false}}<br>
return 0;<br>
if (-2147483648 >= a)<br>
@@ -187,25 +184,24 @@ int main() {<br>
return 0;<br>
if (2147483647 < a) // expected-warning {{comparison 2147483647 < 'enum A' is always false}}<br>
return 0;<br>
-#elif defined(UNSIGNED) && !defined(SILENCE)<br>
-#ifndef SILENCE<br>
- if (a < -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}<br>
+<br>
+ if (a < 2147483647U)<br>
return 0;<br>
- if (-2147483648 >= a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}<br>
+ if (2147483647U >= a) // expected-warning {{comparison 2147483647 >= 'enum A' is always true}}<br>
return 0;<br>
- if (a > -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}<br>
+ if (a > 2147483647U) // expected-warning {{comparison 'enum A' > 2147483647 is always false}}<br>
return 0;<br>
- if (-2147483648 <= a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}<br>
+ if (2147483647U <= a)<br>
return 0;<br>
- if (a <= -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}<br>
+ if (a <= 2147483647U) // expected-warning {{comparison 'enum A' <= 2147483647 is always true}}<br>
return 0;<br>
- if (-2147483648 > a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always false}}<br>
+ if (2147483647U > a)<br>
return 0;<br>
- if (a >= -2147483648) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}<br>
+ if (a >= 2147483647U)<br>
return 0;<br>
- if (-2147483648 < a) // expected-warning {{comparison of constant -2147483648 with expression of type 'enum A' is always true}}<br>
+ if (2147483647U < a) // expected-warning {{comparison 2147483647 < 'enum A' is always false}}<br>
return 0;<br>
-#else<br>
+#else // SILENCE<br>
if (a < -2147483648)<br>
return 0;<br>
if (-2147483648 >= a)<br>
@@ -222,7 +218,6 @@ int main() {<br>
return 0;<br>
if (-2147483648 < a)<br>
return 0;<br>
-#endif<br>
<br>
if (a < 2147483647)<br>
return 0;<br>
@@ -240,6 +235,24 @@ int main() {<br>
return 0;<br>
if (2147483647 < a)<br>
return 0;<br>
+<br>
+ if (a < 2147483647U)<br>
+ return 0;<br>
+ if (2147483647U >= a)<br>
+ return 0;<br>
+ if (a > 2147483647U)<br>
+ return 0;<br>
+ if (2147483647U <= a)<br>
+ return 0;<br>
+ if (a <= 2147483647U)<br>
+ return 0;<br>
+ if (2147483647U > a)<br>
+ return 0;<br>
+ if (a >= 2147483647U)<br>
+ return 0;<br>
+ if (2147483647U < a)<br>
+ return 0;<br>
+#endif<br>
#endif<br>
<br>
return 1;<br>
<br>
Modified: cfe/trunk/test/SemaCXX/<wbr>compare.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/compare.cpp?rev=320162&r1=320161&r2=320162&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>SemaCXX/compare.cpp?rev=<wbr>320162&r1=320161&r2=320162&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/SemaCXX/<wbr>compare.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/<wbr>compare.cpp Fri Dec 8 08:54:08 2017<br>
@@ -245,8 +245,8 @@ void test4(short s) {<br>
// unsigned.<br>
const unsigned B = -1;<br>
void (s < B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}<br>
- void (s > B); // expected-warning{{comparison 'short' > 4294967295 is always false}}<br>
- void (s <= B); // expected-warning{{comparison 'short' <= 4294967295 is always true}}<br>
+ void (s > B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}<br>
+ void (s <= B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}<br>
void (s >= B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}<br>
void (s == B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}<br>
void (s != B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div></div>