r259271 - Improve -Wconstant-conversion
Richard Trieu via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 5 15:07:22 PST 2016
r259947 will stop this warning on char arrays.
On Mon, Feb 1, 2016 at 1:40 PM, Richard Trieu <rtrieu at google.com> wrote:
> C++11 narrowing only happens during certain conversions while this warning
> checks all conversions.
>
> We might be able to avoid char array initialization, but it would be a
> little tricky. These warning usually have little information about where
> the conversion is coming from, so distinguishing array initialization
> versus variable initialization could be hard.
>
> On Sat, Jan 30, 2016 at 6:41 AM, Nico Weber <thakis at chromium.org> wrote:
>
>> Shouldn't the new case be in -Wc++11-narrowing instead? This is warning
>> in similar places where that warning used to warn.
>>
>> In practice, char arrays seem to be often used for storing binary blobs
>> in header files, and those are usually initialized with numbers 0...255
>> instead of -128...127 (why not make the array uint8_t then? Because maybe
>> the function you want to pass the data from wants a char* array, and having
>> to reinterpret_cast from uint8_t to char is annoying). Maybe this shouldn't
>> fire for char arrays?
>>
>> On Fri, Jan 29, 2016 at 6:51 PM, Richard Trieu via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: rtrieu
>>> Date: Fri Jan 29 17:51:16 2016
>>> New Revision: 259271
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=259271&view=rev
>>> Log:
>>> Improve -Wconstant-conversion
>>>
>>> Switch the evaluation from isIntegerConstantExpr to EvaluateAsInt.
>>> EvaluateAsInt will evaluate more types of expressions than
>>> isIntegerConstantExpr.
>>>
>>> Move one case from -Wsign-conversion to -Wconstant-conversion. The case
>>> is:
>>> 1) Source and target types are signed
>>> 2) Source type is wider than the target type
>>> 3) The source constant value is positive
>>> 4) The conversion will store the value as negative in the target.
>>>
>>> Modified:
>>> cfe/trunk/lib/Sema/SemaChecking.cpp
>>> cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
>>> cfe/trunk/test/Sema/constant-conversion.c
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=259271&r1=259270&r2=259271&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Jan 29 17:51:16 2016
>>> @@ -7578,7 +7578,7 @@ void CheckImplicitConversion(Sema &S, Ex
>>> // If the source is a constant, use a default-on diagnostic.
>>> // TODO: this should happen for bitfield stores, too.
>>> llvm::APSInt Value(32);
>>> - if (E->isIntegerConstantExpr(Value, S.Context)) {
>>> + if (E->EvaluateAsInt(Value, S.Context, Expr::SE_AllowSideEffects)) {
>>> if (S.SourceMgr.isInSystemMacro(CC))
>>> return;
>>>
>>> @@ -7603,6 +7603,42 @@ void CheckImplicitConversion(Sema &S, Ex
>>> return DiagnoseImpCast(S, E, T, CC,
>>> diag::warn_impcast_integer_precision);
>>> }
>>>
>>> + if (TargetRange.Width == SourceRange.Width &&
>>> !TargetRange.NonNegative &&
>>> + SourceRange.NonNegative && Source->isSignedIntegerType()) {
>>> + // Warn when doing a signed to signed conversion, warn if the
>>> positive
>>> + // source value is exactly the width of the target type, which will
>>> + // cause a negative value to be stored.
>>> +
>>> + llvm::APSInt Value;
>>> + if (E->EvaluateAsInt(Value, S.Context, Expr::SE_AllowSideEffects)) {
>>> + if (!S.SourceMgr.isInSystemMacro(CC)) {
>>> +
>>> + IntegerLiteral *IntLit =
>>> + dyn_cast<IntegerLiteral>(E->IgnoreParenImpCasts());
>>> +
>>> + // If initializing from a constant, and the constant starts
>>> with '0',
>>> + // then it is a binary, octal, or hexadecimal. Allow these
>>> constants
>>> + // to fill all the bits, even if there is a sign change.
>>> + if (!IntLit ||
>>> +
>>> *(S.getSourceManager().getCharacterData(IntLit->getLocStart())) !=
>>> + '0') {
>>> +
>>> + std::string PrettySourceValue = Value.toString(10);
>>> + std::string PrettyTargetValue =
>>> + PrettyPrintInRange(Value, TargetRange);
>>> +
>>> + S.DiagRuntimeBehavior(
>>> + E->getExprLoc(), E,
>>> + S.PDiag(diag::warn_impcast_integer_precision_constant)
>>> + << PrettySourceValue << PrettyTargetValue <<
>>> E->getType() << T
>>> + << E->getSourceRange() << clang::SourceRange(CC));
>>> + return;
>>> + }
>>> + }
>>> + }
>>> + // Fall through for non-constants to give a sign conversion warning.
>>> + }
>>> +
>>> if ((TargetRange.NonNegative && !SourceRange.NonNegative) ||
>>> (!TargetRange.NonNegative && SourceRange.NonNegative &&
>>> SourceRange.Width == TargetRange.Width)) {
>>>
>>> Modified: cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp?rev=259271&r1=259270&r2=259271&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp (original)
>>> +++ cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp Fri Jan 29 17:51:16 2016
>>> @@ -242,8 +242,8 @@ namespace UndefinedBehavior {
>>> constexpr int n13 = n5 + n5; // expected-error {{constant
>>> expression}} expected-note {{value -4294967296 is outside the range of }}
>>> constexpr int n14 = n3 - n5; // expected-error {{constant
>>> expression}} expected-note {{value 4294967295 is outside the range of }}
>>> constexpr int n15 = n5 * n5; // expected-error {{constant
>>> expression}} expected-note {{value 4611686018427387904 is outside the range
>>> of }}
>>> - constexpr signed char c1 = 100 * 2; // ok
>>> - constexpr signed char c2 = '\x64' * '\2'; // also ok
>>> + constexpr signed char c1 = 100 * 2; // ok expected-warning{{changes
>>> value}}
>>> + constexpr signed char c2 = '\x64' * '\2'; // also ok
>>> expected-warning{{changes value}}
>>> constexpr long long ll1 = 0x7fffffffffffffff; // ok
>>> constexpr long long ll2 = ll1 + 1; // expected-error {{constant}}
>>> expected-note {{ 9223372036854775808 }}
>>> constexpr long long ll3 = -ll1 - 1; // ok
>>>
>>> Modified: cfe/trunk/test/Sema/constant-conversion.c
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/constant-conversion.c?rev=259271&r1=259270&r2=259271&view=diff
>>>
>>> ==============================================================================
>>> --- cfe/trunk/test/Sema/constant-conversion.c (original)
>>> +++ cfe/trunk/test/Sema/constant-conversion.c Fri Jan 29 17:51:16 2016
>>> @@ -80,3 +80,34 @@ void test8() {
>>> struct { enum E x : 1; } f;
>>> f.x = C; // expected-warning {{implicit truncation from 'int' to
>>> bitfield changes value from 2 to 0}}
>>> }
>>> +
>>> +void test9() {
>>> + const char max_char = 0x7F;
>>> + const short max_short = 0x7FFF;
>>> + const int max_int = 0x7FFFFFFF;
>>> +
>>> + const short max_char_plus_one = (short)max_char + 1;
>>> + const int max_short_plus_one = (int)max_short + 1;
>>> + const long max_int_plus_one = (long)max_int + 1;
>>> +
>>> + char new_char = max_char_plus_one; // expected-warning {{implicit
>>> conversion from 'const short' to 'char' changes value from 128 to -128}}
>>> + short new_short = max_short_plus_one; // expected-warning {{implicit
>>> conversion from 'const int' to 'short' changes value from 32768 to -32768}}
>>> + int new_int = max_int_plus_one; // expected-warning {{implicit
>>> conversion from 'const long' to 'int' changes value from 2147483648 to -
>>> 2147483648}}
>>> +
>>> + char hex_char = 0x80;
>>> + short hex_short = 0x8000;
>>> + int hex_int = 0x80000000;
>>> +
>>> + char oct_char = 0200;
>>> + short oct_short = 0100000;
>>> + int oct_int = 020000000000;
>>> +
>>> + char bin_char = 0b10000000;
>>> + short bin_short = 0b1000000000000000;
>>> + int bin_int = 0b10000000000000000000000000000000;
>>> +
>>> +#define CHAR_MACRO_HEX 0xff
>>> + char macro_char_hex = CHAR_MACRO_HEX;
>>> +#define CHAR_MACRO_DEC 255
>>> + char macro_char_dec = CHAR_MACRO_DEC; // expected-warning {{implicit
>>> conversion from 'int' to 'char' changes value from 255 to -1}}
>>> +}
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160205/281d348c/attachment.html>
More information about the cfe-commits
mailing list