[cfe-commits] r167992 - in /cfe/trunk: lib/Sema/SemaChecking.cpp test/Analysis/additive-folding.cpp test/SemaCXX/compare.cpp
NAKAMURA Takumi
geek4civic at gmail.com
Wed Nov 14 15:53:44 PST 2012
Richard,
It brought new warnings in selfhosting.
clang/lib/Sema/SemaExpr.cpp:8479:14: error:
comparison of constant -1 with expression of type 'BinOp::Opcode' (aka
'clang::BinaryOperatorKind') is always false
[-Werror,-Wtautological-constant-out-of-range-compare]
if (LHSopc == -1 && RHSopc == -1)
~~~~~~ ^ ~~
clang/lib/Sema/SemaExpr.cpp:8479:30: error:
comparison of constant -1 with expression of type 'BinOp::Opcode' (aka
'clang::BinaryOperatorKind') is always false
[-Werror,-Wtautological-constant-out-of-range-compare]
if (LHSopc == -1 && RHSopc == -1)
~~~~~~ ^ ~~
2 errors generated.
2012/11/15 Richard Trieu <rtrieu at google.com>:
> Author: rtrieu
> Date: Wed Nov 14 16:50:24 2012
> New Revision: 167992
>
> URL: http://llvm.org/viewvc/llvm-project?rev=167992&view=rev
> Log:
> Improve -Wtautological-constant-out-of-range-compare by taking into account
> type conversion between integers. This allows the warning to be more accurate.
>
> Also, turned the warning off in an analyzer test. The relavent test cases
> are covered by the tests in Sema.
>
> Modified:
> cfe/trunk/lib/Sema/SemaChecking.cpp
> cfe/trunk/test/Analysis/additive-folding.cpp
> cfe/trunk/test/SemaCXX/compare.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=167992&r1=167991&r2=167992&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Wed Nov 14 16:50:24 2012
> @@ -4328,38 +4328,96 @@
> Expr *Constant, Expr *Other,
> llvm::APSInt Value,
> bool RhsConstant) {
> + // 0 values are handled later by CheckTrivialUnsignedComparison().
> + if (Value == 0)
> + return;
> +
> BinaryOperatorKind op = E->getOpcode();
> QualType OtherT = Other->getType();
> QualType ConstantT = Constant->getType();
> + QualType CommonT = E->getLHS()->getType();
> if (S.Context.hasSameUnqualifiedType(OtherT, ConstantT))
> return;
> assert((OtherT->isIntegerType() && ConstantT->isIntegerType())
> && "comparison with non-integer type");
> - // FIXME. handle cases for signedness to catch (signed char)N == 200
> +
> + bool ConstantSigned = ConstantT->isSignedIntegerType();
> + bool OtherSigned = OtherT->isSignedIntegerType();
> + bool CommonSigned = CommonT->isSignedIntegerType();
> +
> + bool EqualityOnly = false;
> +
> + // TODO: Investigate using GetExprRange() to get tighter bounds on
> + // on the bit ranges.
> IntRange OtherRange = IntRange::forValueOfType(S.Context, OtherT);
> - IntRange LitRange = GetValueRange(S.Context, Value, Value.getBitWidth());
> - if (OtherRange.Width >= LitRange.Width)
> - return;
> + unsigned OtherWidth = OtherRange.Width;
> +
> + if (CommonSigned) {
> + // The common type is signed, therefore no signed to unsigned conversion.
> + if (OtherSigned) {
> + // Check that the constant is representable in type OtherT.
> + if (ConstantSigned) {
> + if (OtherWidth >= Value.getMinSignedBits())
> + return;
> + } else { // !ConstantSigned
> + if (OtherWidth >= Value.getActiveBits() + 1)
> + return;
> + }
> + } else { // !OtherSigned
> + // Check that the constant is representable in type OtherT.
> + // Negative values are out of range.
> + if (ConstantSigned) {
> + if (Value.isNonNegative() && OtherWidth >= Value.getActiveBits())
> + return;
> + } else { // !ConstantSigned
> + if (OtherWidth >= Value.getActiveBits())
> + return;
> + }
> + }
> + } else { // !CommonSigned
> + if (!OtherSigned) {
> + if (OtherWidth >= Value.getActiveBits())
> + return;
> + } else if (OtherSigned && !ConstantSigned) {
> + // Check to see if the constant is representable in OtherT.
> + if (OtherWidth > Value.getActiveBits())
> + return;
> + // Check to see if the constant is equivalent to a negative value
> + // cast to CommonT.
> + if (S.Context.getIntWidth(ConstantT) == S.Context.getIntWidth(CommonT) &&
> + Value.isNegative() && Value.getMinSignedBits() < OtherWidth)
> + return;
> + // The constant value rests between values that OtherT can represent after
> + // conversion. Relational comparison still works, but equality
> + // comparisons will be tautological.
> + EqualityOnly = true;
> + } else { // OtherSigned && ConstantSigned
> + assert(0 && "Two signed types converted to unsigned types.");
> + }
> + }
> +
> + bool PositiveConstant = !ConstantSigned || Value.isNonNegative();
> +
> bool IsTrue = true;
> - if (op == BO_EQ)
> - IsTrue = false;
> - else if (op == BO_NE)
> - IsTrue = true;
> - else if (RhsConstant) {
> + if (op == BO_EQ || op == BO_NE) {
> + IsTrue = op == BO_NE;
> + } else if (EqualityOnly) {
> + return;
> + } else if (RhsConstant) {
> if (op == BO_GT || op == BO_GE)
> - IsTrue = !LitRange.NonNegative;
> + IsTrue = !PositiveConstant;
> else // op == BO_LT || op == BO_LE
> - IsTrue = LitRange.NonNegative;
> + IsTrue = PositiveConstant;
> } else {
> if (op == BO_LT || op == BO_LE)
> - IsTrue = !LitRange.NonNegative;
> + IsTrue = !PositiveConstant;
> else // op == BO_GT || op == BO_GE
> - IsTrue = LitRange.NonNegative;
> + IsTrue = PositiveConstant;
> }
> SmallString<16> PrettySourceValue(Value.toString(10));
> S.Diag(E->getOperatorLoc(), diag::warn_out_of_range_compare)
> - << PrettySourceValue << OtherT << IsTrue
> - << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
> + << PrettySourceValue << OtherT << IsTrue
> + << E->getLHS()->getSourceRange() << E->getRHS()->getSourceRange();
> }
>
> /// Analyze the operands of the given comparison. Implements the
>
> Modified: cfe/trunk/test/Analysis/additive-folding.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/additive-folding.cpp?rev=167992&r1=167991&r2=167992&view=diff
> ==============================================================================
> --- cfe/trunk/test/Analysis/additive-folding.cpp (original)
> +++ cfe/trunk/test/Analysis/additive-folding.cpp Wed Nov 14 16:50:24 2012
> @@ -1,4 +1,4 @@
> -// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -analyzer-constraints=range -Wno-tautological-compare -Wtautological-constant-out-of-range-compare %s
> +// RUN: %clang_cc1 -analyze -analyzer-checker=core,debug.ExprInspection -verify -analyzer-constraints=range -Wno-tautological-compare %s
>
> void clang_analyzer_eval(bool);
>
> @@ -128,10 +128,10 @@
>
> // Tautologies from outside the range of the symbol
> void tautologiesOutside(unsigned char a) {
> - clang_analyzer_eval(a <= 0x100); // expected-warning{{comparison of constant 256 with expression of type 'unsigned char' is always true}} expected-warning{{TRUE}}
> - clang_analyzer_eval(a < 0x100); // expected-warning{{comparison of constant 256 with expression of type 'unsigned char' is always true}} expected-warning{{TRUE}}
> + clang_analyzer_eval(a <= 0x100); // expected-warning{{TRUE}}
> + clang_analyzer_eval(a < 0x100); // expected-warning{{TRUE}}
>
> - clang_analyzer_eval(a != 0x100); // expected-warning{{comparison of constant 256 with expression of type 'unsigned char' is always true}} expected-warning{{TRUE}}
> + clang_analyzer_eval(a != 0x100); // expected-warning{{TRUE}}
> clang_analyzer_eval(a != -1); // expected-warning{{TRUE}}
>
> clang_analyzer_eval(a > -1); // expected-warning{{TRUE}}
>
> Modified: cfe/trunk/test/SemaCXX/compare.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/compare.cpp?rev=167992&r1=167991&r2=167992&view=diff
> ==============================================================================
> --- cfe/trunk/test/SemaCXX/compare.cpp (original)
> +++ cfe/trunk/test/SemaCXX/compare.cpp Wed Nov 14 16:50:24 2012
> @@ -223,3 +223,117 @@
> (void) (true ? b : a);
> (void) (true ? (unsigned char)b : (signed char)a);
> }
> +
> +// Test comparison of short to unsigned. If tautological compare does not
> +// trigger, then the signed comparision warning will.
> +void test4(short s) {
> + // A is max short plus 1. All zero and positive shorts are smaller than it.
> + // All negative shorts are cast towards the max unsigned range. Relation
> + // comparisons are possible, but equality comparisons are tautological.
> + const unsigned A = 32768;
> + void (s < A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> + void (s > A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> + void (s <= A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> + void (s >= A); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> +
> + void (s == A); // expected-warning{{comparison of constant 32768 with expression of type 'short' is always false}}
> + void (s != A); // expected-warning{{comparison of constant 32768 with expression of type 'short' is always true}}
> +
> + // When negative one is converted to an unsigned value, it becomes the max
> + // unsigned. Likewise, a negative one short can also be converted to max
> + // unsigned.
> + const unsigned B = -1;
> + void (s < B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> + void (s > B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> + void (s <= B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> + void (s >= B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> + void (s == B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> + void (s != B); // expected-warning{{comparison of integers of different signs: 'short' and 'const unsigned int'}}
> +
> +}
> +
> +void test5(bool b) {
> + (void) (b < -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always false}}
> + (void) (b > -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always true}}
> + (void) (b == -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always false}}
> + (void) (b != -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always true}}
> + (void) (b <= -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always false}}
> + (void) (b >= -1); // expected-warning{{comparison of constant -1 with expression of type 'bool' is always true}}
> +
> + (void) (b < -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always false}}
> + (void) (b > -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always true}}
> + (void) (b == -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always false}}
> + (void) (b != -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always true}}
> + (void) (b <= -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always false}}
> + (void) (b >= -10); // expected-warning{{comparison of constant -10 with expression of type 'bool' is always true}}
> +
> + (void) (b < 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always true}}
> + (void) (b > 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always false}}
> + (void) (b == 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always false}}
> + (void) (b != 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always true}}
> + (void) (b <= 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always true}}
> + (void) (b >= 2); // expected-warning{{comparison of constant 2 with expression of type 'bool' is always false}}
> +
> + (void) (b < 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always true}}
> + (void) (b > 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always false}}
> + (void) (b == 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always false}}
> + (void) (b != 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always true}}
> + (void) (b <= 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always true}}
> + (void) (b >= 10); // expected-warning{{comparison of constant 10 with expression of type 'bool' is always false}}
> +}
> +
> +void test6(signed char sc) {
> + (void)(sc < 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
> + (void)(sc > 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
> + (void)(sc <= 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
> + (void)(sc >= 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
> + (void)(sc == 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
> + (void)(sc != 200); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
> +
> + (void)(200 < sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
> + (void)(200 > sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
> + (void)(200 <= sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
> + (void)(200 >= sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
> + (void)(200 == sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always false}}
> + (void)(200 != sc); // expected-warning{{comparison of constant 200 with expression of type 'signed char' is always true}}
> +}
> +
> +// Test many signedness combinations.
> +void test7(unsigned long other) {
> + // Common unsigned, other unsigned, constant unsigned
> + (void)((unsigned)other != (unsigned long)(0x1ffffffff)); // expected-warning{{true}}
> + (void)((unsigned)other != (unsigned long)(0xffffffff));
> + (void)((unsigned long)other != (unsigned)(0x1ffffffff));
> + (void)((unsigned long)other != (unsigned)(0xffffffff));
> +
> + // Common unsigned, other signed, constant unsigned
> + (void)((int)other != (unsigned long)(0xffffffffffffffff)); // expected-warning{{different signs}}
> + (void)((int)other != (unsigned long)(0x00000000ffffffff)); // expected-warning{{true}}
> + (void)((int)other != (unsigned long)(0x000000000fffffff));
> + (void)((int)other < (unsigned long)(0x00000000ffffffff)); // expected-warning{{different signs}}
> +
> + // Common unsigned, other unsigned, constant signed
> + (void)((unsigned long)other != (int)(0xffffffff)); // expected-warning{{different signs}}
> +
> + // Common unsigned, other signed, constant signed
> + // Should not be possible as the common type should also be signed.
> +
> + // Common signed, other signed, constant signed
> + (void)((int)other != (long)(0xffffffff)); // expected-warning{{true}}
> + (void)((int)other != (long)(0xffffffff00000000)); // expected-warning{{true}}
> + (void)((int)other != (long)(0xfffffff));
> + (void)((int)other != (long)(0xfffffffff0000000));
> +
> + // Common signed, other signed, constant unsigned
> + (void)((int)other != (unsigned char)(0xffff));
> + (void)((int)other != (unsigned char)(0xff));
> +
> + // Common signed, other unsigned, constant signed
> + (void)((unsigned char)other != (int)(0xff));
> + (void)((unsigned char)other != (int)(0xffff)); // expected-warning{{true}}
> +
> + // Common signed, other unsigned, constant unsigned
> + (void)((unsigned char)other != (unsigned short)(0xff));
> + (void)((unsigned char)other != (unsigned short)(0x100)); // expected-warning{{true}}
> + (void)((unsigned short)other != (unsigned char)(0xff));
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list