<div dir="ltr">Shouldn't the new case be in -Wc++11-narrowing instead? This is warning in similar places where that warning used to warn.<div><br></div><div>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?</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 29, 2016 at 6:51 PM, Richard Trieu 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">Author: rtrieu<br>
Date: Fri Jan 29 17:51:16 2016<br>
New Revision: 259271<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=259271&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=259271&view=rev</a><br>
Log:<br>
Improve -Wconstant-conversion<br>
<br>
Switch the evaluation from isIntegerConstantExpr to EvaluateAsInt.<br>
EvaluateAsInt will evaluate more types of expressions than<br>
isIntegerConstantExpr.<br>
<br>
Move one case from -Wsign-conversion to -Wconstant-conversion. The case is:<br>
1) Source and target types are signed<br>
2) Source type is wider than the target type<br>
3) The source constant value is positive<br>
4) The conversion will store the value as negative in the target.<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaChecking.cpp<br>
cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp<br>
cfe/trunk/test/Sema/constant-conversion.c<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=259271&r1=259270&r2=259271&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=259271&r1=259270&r2=259271&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Fri Jan 29 17:51:16 2016<br>
@@ -7578,7 +7578,7 @@ void CheckImplicitConversion(Sema &S, Ex<br>
// If the source is a constant, use a default-on diagnostic.<br>
// TODO: this should happen for bitfield stores, too.<br>
llvm::APSInt Value(32);<br>
- if (E->isIntegerConstantExpr(Value, S.Context)) {<br>
+ if (E->EvaluateAsInt(Value, S.Context, Expr::SE_AllowSideEffects)) {<br>
if (S.SourceMgr.isInSystemMacro(CC))<br>
return;<br>
<br>
@@ -7603,6 +7603,42 @@ void CheckImplicitConversion(Sema &S, Ex<br>
return DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_integer_precision);<br>
}<br>
<br>
+ if (TargetRange.Width == SourceRange.Width && !TargetRange.NonNegative &&<br>
+ SourceRange.NonNegative && Source->isSignedIntegerType()) {<br>
+ // Warn when doing a signed to signed conversion, warn if the positive<br>
+ // source value is exactly the width of the target type, which will<br>
+ // cause a negative value to be stored.<br>
+<br>
+ llvm::APSInt Value;<br>
+ if (E->EvaluateAsInt(Value, S.Context, Expr::SE_AllowSideEffects)) {<br>
+ if (!S.SourceMgr.isInSystemMacro(CC)) {<br>
+<br>
+ IntegerLiteral *IntLit =<br>
+ dyn_cast<IntegerLiteral>(E->IgnoreParenImpCasts());<br>
+<br>
+ // If initializing from a constant, and the constant starts with '0',<br>
+ // then it is a binary, octal, or hexadecimal. Allow these constants<br>
+ // to fill all the bits, even if there is a sign change.<br>
+ if (!IntLit ||<br>
+ *(S.getSourceManager().getCharacterData(IntLit->getLocStart())) !=<br>
+ '0') {<br>
+<br>
+ std::string PrettySourceValue = Value.toString(10);<br>
+ std::string PrettyTargetValue =<br>
+ PrettyPrintInRange(Value, TargetRange);<br>
+<br>
+ S.DiagRuntimeBehavior(<br>
+ E->getExprLoc(), E,<br>
+ S.PDiag(diag::warn_impcast_integer_precision_constant)<br>
+ << PrettySourceValue << PrettyTargetValue << E->getType() << T<br>
+ << E->getSourceRange() << clang::SourceRange(CC));<br>
+ return;<br>
+ }<br>
+ }<br>
+ }<br>
+ // Fall through for non-constants to give a sign conversion warning.<br>
+ }<br>
+<br>
if ((TargetRange.NonNegative && !SourceRange.NonNegative) ||<br>
(!TargetRange.NonNegative && SourceRange.NonNegative &&<br>
SourceRange.Width == TargetRange.Width)) {<br>
<br>
Modified: cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp?rev=259271&r1=259270&r2=259271&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp?rev=259271&r1=259270&r2=259271&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp (original)<br>
+++ cfe/trunk/test/CXX/expr/expr.const/p2-0x.cpp Fri Jan 29 17:51:16 2016<br>
@@ -242,8 +242,8 @@ namespace UndefinedBehavior {<br>
constexpr int n13 = n5 + n5; // expected-error {{constant expression}} expected-note {{value -4294967296 is outside the range of }}<br>
constexpr int n14 = n3 - n5; // expected-error {{constant expression}} expected-note {{value 4294967295 is outside the range of }}<br>
constexpr int n15 = n5 * n5; // expected-error {{constant expression}} expected-note {{value 4611686018427387904 is outside the range of }}<br>
- constexpr signed char c1 = 100 * 2; // ok<br>
- constexpr signed char c2 = '\x64' * '\2'; // also ok<br>
+ constexpr signed char c1 = 100 * 2; // ok expected-warning{{changes value}}<br>
+ constexpr signed char c2 = '\x64' * '\2'; // also ok expected-warning{{changes value}}<br>
constexpr long long ll1 = 0x7fffffffffffffff; // ok<br>
constexpr long long ll2 = ll1 + 1; // expected-error {{constant}} expected-note {{ 9223372036854775808 }}<br>
constexpr long long ll3 = -ll1 - 1; // ok<br>
<br>
Modified: cfe/trunk/test/Sema/constant-conversion.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/constant-conversion.c?rev=259271&r1=259270&r2=259271&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/constant-conversion.c?rev=259271&r1=259270&r2=259271&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/constant-conversion.c (original)<br>
+++ cfe/trunk/test/Sema/constant-conversion.c Fri Jan 29 17:51:16 2016<br>
@@ -80,3 +80,34 @@ void test8() {<br>
struct { enum E x : 1; } f;<br>
f.x = C; // expected-warning {{implicit truncation from 'int' to bitfield changes value from 2 to 0}}<br>
}<br>
+<br>
+void test9() {<br>
+ const char max_char = 0x7F;<br>
+ const short max_short = 0x7FFF;<br>
+ const int max_int = 0x7FFFFFFF;<br>
+<br>
+ const short max_char_plus_one = (short)max_char + 1;<br>
+ const int max_short_plus_one = (int)max_short + 1;<br>
+ const long max_int_plus_one = (long)max_int + 1;<br>
+<br>
+ char new_char = max_char_plus_one; // expected-warning {{implicit conversion from 'const short' to 'char' changes value from 128 to -128}}<br>
+ short new_short = max_short_plus_one; // expected-warning {{implicit conversion from 'const int' to 'short' changes value from 32768 to -32768}}<br>
+ int new_int = max_int_plus_one; // expected-warning {{implicit conversion from 'const long' to 'int' changes value from 2147483648 to -2147483648}}<br>
+<br>
+ char hex_char = 0x80;<br>
+ short hex_short = 0x8000;<br>
+ int hex_int = 0x80000000;<br>
+<br>
+ char oct_char = 0200;<br>
+ short oct_short = 0100000;<br>
+ int oct_int = 020000000000;<br>
+<br>
+ char bin_char = 0b10000000;<br>
+ short bin_short = 0b1000000000000000;<br>
+ int bin_int = 0b10000000000000000000000000000000;<br>
+<br>
+#define CHAR_MACRO_HEX 0xff<br>
+ char macro_char_hex = CHAR_MACRO_HEX;<br>
+#define CHAR_MACRO_DEC 255<br>
+ char macro_char_dec = CHAR_MACRO_DEC; // expected-warning {{implicit conversion from 'int' to 'char' changes value from 255 to -1}}<br>
+}<br>
<br>
<br>
_______________________________________________<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/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>