<div dir="ltr">I'm going to reland without the changes to bit-field handling. If we want those changes (which I think we do, based on the bugs it found in selfhost), that'll need to be done more carefully, and I don't want to get the refactoring and bugfixes here tangled up with that.</div><div class="gmail_extra"><br><div class="gmail_quote">On 8 December 2017 at 13:27, Bill Seurer via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">It also caused all the sanitizer builds to fail. For example:<br>
<br>
<a href="http://lab.llvm.org:8011/builders/sanitizer-ppc64le-linux/builds/3654" rel="noreferrer" target="_blank">http://lab.llvm.org:8011/build<wbr>ers/sanitizer-ppc64le-linux/<wbr>builds/3654</a><br>
<br>
/home/buildbots/ppc64le-saniti<wbr>zer/sanitizer-ppc64le/build/<wbr>llvm/utils/TableGen/X86Recogni<wbr>zableInstr.cpp:789:21: error: comparison of constant -1 with expression of type 'llvm::X86Disassembler::Opcode<wbr>Type' is always true [-Werror,-Wtautological-consta<wbr>nt-out-of-range-compare]<br>
assert(opcodeType != (OpcodeType)-1 &&<br>
~~~~~~~~~~ ^ ~~~~~~~~~~~~~~<br>
/usr/include/assert.h:86:5: note: expanded from macro 'assert'<br>
((expr) \<br>
^~~~<br>
1 error generated.<br>
utils/TableGen/CMakeFiles/obj.<wbr>llvm-tblgen.dir/build.make:102<wbr>2: recipe for target 'utils/TableGen/CMakeFiles/obj<wbr>.llvm-tblgen.dir/X86Recognizab<wbr>leInstr.cpp.o' failed<br>
make[3]: *** [utils/TableGen/CMakeFiles/obj<wbr>.llvm-tblgen.dir/X86Recognizab<wbr>leInstr.cpp.o] Error 1<br>
<br>
<br>
<br>
And there are lots more warnings (which are flagged as errors for the sanitizer builds) when I run a build by hand. For example:<br>
<br>
[4001/4008] Building CXX object tools/clang/tools/libclang/CMa<wbr>keFiles/libclang.dir/CIndex.<wbr>cpp.o<br>
In file included from /home/seurer/llvm/llvm-test2/t<wbr>ools/clang/tools/libclang/CInd<wbr>ex.cpp:23:<br>
In file included from /home/seurer/llvm/llvm-test2/t<wbr>ools/clang/tools/libclang/Curs<wbr>orVisitor.h:16:<br>
In file included from /home/seurer/llvm/llvm-test2/t<wbr>ools/clang/include/clang/AST/D<wbr>eclVisitor.h:19:<br>
In file included from /home/seurer/llvm/llvm-test2/t<wbr>ools/clang/include/clang/AST/D<wbr>eclCXX.h:21:<br>
In file included from /home/seurer/llvm/llvm-test2/t<wbr>ools/clang/include/clang/AST/A<wbr>ttr.h:19:<br>
/home/seurer/llvm/llvm-test2/t<wbr>ools/clang/include/clang/AST/E<wbr>xpr.h:2745:17: warning: comparison of constant -1 with expression of type 'const clang::CastKind' is always true [-Wtautological-constant-out-o<wbr>f-range-compare]<br>
assert(kind != CK_Invalid && "creating cast with invalid cast kind");<br>
~~~~ ^ ~~~~~~~~~~<br>
/usr/include/assert.h:89:5: note: expanded from macro 'assert'<br>
((expr) \<br>
^~~~<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
On 12/08/2017 10:54 AM, Hans Wennborg via cfe-commits wrote:<br>
<blockquote class="gmail_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-pr<wbr>oject?rev=320162&view=rev</a><br>
Log:<br>
Revert "Unify implementation of our two different flavours of -Wtautological-compare."<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
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>
</blockquote>
<br>
This caused a new warning in Chromium:<br>
<br>
../../base/trace_event/trace_l<wbr>og.cc:1545:29: error: comparison of constant 64<br>
with expression of type 'unsigned int' is always true<br>
[-Werror,-Wtautological-consta<wbr>nt-out-of-range-compare]<br>
DCHECK(handle.event_index < TraceBufferChunk::kTraceBuffer<wbr>ChunkSize);<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>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaChecki<wbr>ng.cpp<br>
cfe/trunk/test/Sema/tautologi<wbr>cal-constant-compare.c<br>
cfe/trunk/test/Sema/tautologi<wbr>cal-constant-enum-compare.c<br>
cfe/trunk/test/SemaCXX/compar<wbr>e.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaCheckin<wbr>g.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-pr<wbr>oject/cfe/trunk/lib/Sema/SemaC<wbr>hecking.cpp?rev=320162&r1=<wbr>320161&r2=320162&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Sema/SemaCheckin<wbr>g.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaCheckin<wbr>g.cpp Fri Dec 8 08:54:08 2017<br>
@@ -8662,113 +8662,54 @@ static bool isKnownToHaveUnsignedValue(E<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.Wi<wbr>dth, R.NonNegative)<br>
- .extOrTrunc(BitWidth);<br>
- PromotedMin.setIsUnsigned(Unsi<wbr>gned);<br>
-<br>
- PromotedMax = llvm::APSInt::getMaxValue(R.Wi<wbr>dth, R.NonNegative)<br>
- .extOrTrunc(BitWidth);<br>
- PromotedMax.setIsUnsigned(Unsi<wbr>gned);<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>
- // 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>
- 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>
- 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(Ot<wbr>her) && Value == 0)<br>
+ return LimitType::Min;<br>
- switch (llvm::APSInt::compareValues(V<wbr>alue, PromotedMin)) {<br>
- case -1: return Less;<br>
- case 0: return PromotedMin == PromotedMax ? OnlyValue : Min;<br>
- case 1:<br>
- switch (llvm::APSInt::compareValues(V<wbr>alue, 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>
- llvm_unreachable("impossible compare result");<br>
- }<br>
+ IntRange OtherRange = IntRange::forValueOfType(S.Con<wbr>text, OtherT);<br>
+ if (Other->isKnownToHaveBooleanVa<wbr>lue())<br>
+ OtherRange = IntRange::forBoolType();<br>
- static llvm::Optional<bool> constantValue(BinaryOperatorKi<wbr>nd 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(Othe<wbr>rRange.Width, OtherRange.NonNegative),<br>
+ Value))<br>
+ return LimitType::Max;<br>
+<br>
+ if (llvm::APSInt::isSameValue(<br>
+ llvm::APSInt::getMinValue(Othe<wbr>rRange.Width, OtherRange.NonNegative),<br>
+ Value))<br>
+ return LimitType::Min;<br>
+<br>
+ return llvm::None;<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>
- if (IsEnumConstOrFromMacro(S, Constant))<br>
- return false;<br>
-<br>
- Expr *OriginalOther = Other;<br>
+ BinaryOperatorKind Op = E->getOpcode();<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.Con<wbr>text, 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->isKnownToHaveBooleanVal<wbr>ue();<br>
- if (OtherIsBooleanDespiteType)<br>
- OtherRange = IntRange::forBoolType();<br>
-<br>
- if (FieldDecl *Bitfield = Other->getSourceBitField())<br>
- if (!Bitfield->getBitWidth()->isV<wbr>alueDependent())<br>
- OtherRange.Width =<br>
- std::min(Bitfield->getBitWidth<wbr>Value(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(Val<wbr>ue);<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>
- auto Result = PromotedRange::constantValue(E<wbr>->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>
+ 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_tautologica<wbr>l_bool_compare)<br>
- << OS.str() << classifyConstantValue(Constant<wbr>)<br>
- << OtherT << !OtherT->isBooleanType() << *Result<br>
+ << OS.str() << classifyConstantValue(Constant<wbr>->IgnoreParenImpCasts())<br>
+ << OType << !OType->isBooleanType() << Result<br>
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange())<wbr>;<br>
return true;<br>
}<br>
- unsigned Diag = (isKnownToHaveUnsignedValue(Or<wbr>iginalOther) && Value == 0)<br>
- ? (HasEnumType(OriginalOther)<br>
+ unsigned Diag = (isKnownToHaveUnsignedValue(Ot<wbr>her) && Value == 0)<br>
+ ? (HasEnumType(Other)<br>
? diag::warn_unsigned_enum_alway<wbr>s_true_comparison<br>
: diag::warn_unsigned_always_tru<wbr>e_comparison)<br>
: diag::warn_tautological_consta<wbr>nt_compare;<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>
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.Con<wbr>text, OtherT);<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>
- if (FieldDecl *Bitfield = Other->getSourceBitField())<br>
- if (!Bitfield->getBitWidth()->isV<wbr>alueDependent())<br>
- OtherRange.Width =<br>
- std::min(Bitfield->getBitWidth<wbr>Value(S.Context), OtherRange.Width);<br>
+ unsigned OtherWidth = OtherRange.Width;<br>
+<br>
+ BinaryOperatorKind op = E->getOpcode();<br>
+ bool IsTrue = true;<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(Val<wbr>ue);<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>
- auto IsTrue = PromotedRange::constantValue(E<wbr>->getOpcode(), Cmp, RhsConstant);<br>
- if (!IsTrue)<br>
- return false;<br>
+ if (S.Context.hasSameUnqualifiedT<wbr>ype(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->isSignedIntegerType<wbr>();<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(Constan<wbr>tT) ==<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>
// 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_rang<wbr>e_compare)<br>
<< OS.str() << classifyConstantValue(Constant<wbr>)<br>
- << OtherT << OtherIsBooleanDespiteType << *IsTrue<br>
+ << OtherT << OtherIsBooleanDespiteType << IsTrue<br>
<< E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange())<wbr>;<br>
return true;<br>
<br>
Modified: cfe/trunk/test/Sema/tautologic<wbr>al-constant-compare.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-pr<wbr>oject/cfe/trunk/test/Sema/taut<wbr>ological-constant-compare.c?<wbr>rev=320162&r1=320161&r2=320162<wbr>&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Sema/tautologic<wbr>al-constant-compare.c (original)<br>
+++ cfe/trunk/test/Sema/tautologic<wbr>al-constant-compare.c Fri Dec 8 08:54:08 2017<br>
@@ -94,17 +94,15 @@ int main()<br>
if (-32768 >= s)<br>
return 0;<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>
// FIXME: assumes two's complement<br>
@@ -336,6 +281,8 @@ int main()<br>
if (0 >= s)<br>
return 0;<br>
+ // However the comparison with 0U would warn<br>
+<br>
unsigned short us = value();<br>
#ifdef TEST<br>
<br>
Modified: cfe/trunk/test/Sema/tautologic<wbr>al-constant-enum-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-pr<wbr>oject/cfe/trunk/test/Sema/taut<wbr>ological-constant-enum-compare<wbr>.c?rev=320162&r1=320161&r2=<wbr>320162&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Sema/tautologic<wbr>al-constant-enum-compare.c (original)<br>
+++ cfe/trunk/test/Sema/tautologic<wbr>al-constant-enum-compare.c Fri Dec 8 08:54:08 2017<br>
@@ -9,96 +9,78 @@ int main() {<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>
- 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>
- 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>
-#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>
- 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>
-#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>
-#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 < -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a>) // expected-warning {{comparison 'enum A' < -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a> is always false}}<br>
return 0;<br>
if <a href="tel:%28-2147483648" value="+12147483648" target="_blank">(-2147483648</a> >= a)<br>
@@ -187,25 +184,24 @@ int main() {<br>
return 0;<br>
if <a href="tel:%282147483647" value="+12147483647" target="_blank">(2147483647</a> < a) // expected-warning {{comparison <a href="tel:2147483647" value="+12147483647" target="_blank">2147483647</a> < 'enum A' is always false}}<br>
return 0;<br>
-#elif defined(UNSIGNED) && !defined(SILENCE)<br>
-#ifndef SILENCE<br>
- if (a < -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a>) // expected-warning {{comparison of constant -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a> with expression of type 'enum A' is always false}}<br>
+<br>
+ if (a < 2147483647U)<br>
return 0;<br>
- if <a href="tel:%28-2147483648" value="+12147483648" target="_blank">(-2147483648</a> >= a) // expected-warning {{comparison of constant -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a> with expression of type 'enum A' is always false}}<br>
+ if (2147483647U >= a) // expected-warning {{comparison <a href="tel:2147483647" value="+12147483647" target="_blank">2147483647</a> >= 'enum A' is always true}}<br>
return 0;<br>
- if (a > -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a>) // expected-warning {{comparison of constant -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a> with expression of type 'enum A' is always true}}<br>
+ if (a > 2147483647U) // expected-warning {{comparison 'enum A' > <a href="tel:2147483647" value="+12147483647" target="_blank">2147483647</a> is always false}}<br>
return 0;<br>
- if <a href="tel:%28-2147483648" value="+12147483648" target="_blank">(-2147483648</a> <= a) // expected-warning {{comparison of constant -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a> with expression of type 'enum A' is always true}}<br>
+ if (2147483647U <= a)<br>
return 0;<br>
- if (a <= -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a>) // expected-warning {{comparison of constant -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a> with expression of type 'enum A' is always false}}<br>
+ if (a <= 2147483647U) // expected-warning {{comparison 'enum A' <= <a href="tel:2147483647" value="+12147483647" target="_blank">2147483647</a> is always true}}<br>
return 0;<br>
- if <a href="tel:%28-2147483648" value="+12147483648" target="_blank">(-2147483648</a> > a) // expected-warning {{comparison of constant -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a> with expression of type 'enum A' is always false}}<br>
+ if (2147483647U > a)<br>
return 0;<br>
- if (a >= -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a>) // expected-warning {{comparison of constant -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a> with expression of type 'enum A' is always true}}<br>
+ if (a >= 2147483647U)<br>
return 0;<br>
- if <a href="tel:%28-2147483648" value="+12147483648" target="_blank">(-2147483648</a> < a) // expected-warning {{comparison of constant -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a> with expression of type 'enum A' is always true}}<br>
+ if (2147483647U < a) // expected-warning {{comparison <a href="tel:2147483647" value="+12147483647" target="_blank">2147483647</a> < 'enum A' is always false}}<br>
return 0;<br>
-#else<br>
+#else // SILENCE<br>
if (a < -<a href="tel:2147483648" value="+12147483648" target="_blank">2147483648</a>)<br>
return 0;<br>
if <a href="tel:%28-2147483648" value="+12147483648" target="_blank">(-2147483648</a> >= a)<br>
@@ -222,7 +218,6 @@ int main() {<br>
return 0;<br>
if <a href="tel:%28-2147483648" value="+12147483648" target="_blank">(-2147483648</a> < a)<br>
return 0;<br>
-#endif<br>
if (a < <a href="tel:2147483647" value="+12147483647" target="_blank">2147483647</a>)<br>
return 0;<br>
@@ -240,6 +235,24 @@ int main() {<br>
return 0;<br>
if <a href="tel:%282147483647" value="+12147483647" target="_blank">(2147483647</a> < 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>
return 1;<br>
<br>
Modified: cfe/trunk/test/SemaCXX/compare<wbr>.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-pr<wbr>oject/cfe/trunk/test/SemaCXX/<wbr>compare.cpp?rev=320162&r1=<wbr>320161&r2=320162&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/SemaCXX/compare<wbr>.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/compare<wbr>.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" target="_blank">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>
<br>
</blockquote>
<br>
<br></div></div><span class="HOEnZb"><font color="#888888">
-- <br>
<br>
-Bill Seurer</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">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>
</div></div></blockquote></div><br></div>