<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, May 20, 2013 at 9:33 AM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Mon, May 20, 2013 at 5:14 PM, Jordan Rose <<a href="mailto:jordan_rose@apple.com">jordan_rose@apple.com</a>> wrote:<br>
<div><div class="h5">><br>
> On May 20, 2013, at 6:59 , Hans Wennborg <<a href="mailto:hans@chromium.org">hans@chromium.org</a>> wrote:<br>
><br>
>> Hi Richard,<br>
>><br>
>> On Fri, Apr 26, 2013 at 3:36 PM, Richard Smith<br>
>> <<a href="mailto:richard-llvm@metafoo.co.uk">richard-llvm@metafoo.co.uk</a>> wrote:<br>
>>> Author: rsmith<br>
>>> Date: Fri Apr 26 09:36:30 2013<br>
>>> New Revision: 180603<br>
>>><br>
>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=180603&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=180603&view=rev</a><br>
>>> Log:<br>
>>> C++1y: support simple variable assignments in constexpr functions.<br>
>><br>
>> This seems to have caused -Winteger-overflow to become much more aggressive.<br>
>><br>
>> I don't know what in your code causes this, but I bisected it down to<br>
>> this revision.<br>
>><br>
>> The following code didn't use to warn, but now does (only in C++11 mode though):<br>
>><br>
>> #include <time.h><br>
>> #include <limits><br>
>><br>
>> void f() {<br>
>> long long x;<br>
>> x = std::numeric_limits<time_t>::min() * 1000;<br>
>> }<br>
>><br>
>> /tmp/a.cc:6:42: warning: overflow in expression; result is 0 with type<br>
>> 'long' [-Winteger-overflow]<br>
>> x = std::numeric_limits<time_t>::min() * 1000;<br>
>> ^<br>
>><br>
>> This is causing problems in Chromium where we have such code, but we<br>
>> know we won't hit it on platforms where time_t is 64-bit.<br>
>><br>
>> Was the warning behaviour change intentional?<br>
><br>
</div></div>> That looks like a real warning to me:<br>
> - if time_t is signed 32-bit and int is 32-bit, the min value will be -2^31, and multiplying by 1000 will result in negative overflow<br>
> - if time_t is signed 64-bit, the min value will be -2^63, and multiplying by 1000 will result in negative overflow<br>
><br>
> The correct way to handle the 32-bit case would be to use "1000LL", so that the calculation is done in long-long space (64 bits) instead of int-space (presumably 32 bits).<br>
<br>
Sorry, my example was a simplified version of the original Chromium<br>
code. We do handle the 32-bit case, 1000 is really a 64-bit constant<br>
variable (static const long long kMillisecondsPerSecond = 1000).<br>
<br>
> The 64-bit case will never be right unless your long long is 128 bits.<br>
<br>
Right, but we never hit that code in the 64-bit case. The problem is<br>
that Clang doesn't know that. And it doesn't help if I add very<br>
explicit guards either, I can do this:<br>
<div class="im"><br>
void f() {<br>
long long x;<br>
</div> if (sizeof(time_t) == 4)<br>
x = std::numeric_limits<time_t>::min() * 1000LL;<br>
else<br>
x = 5;<br>
}<br>
<br>
and it will still warn.<br>
<br>
> In general any improvements to constexpr will up the aggressiveness of warnings based on expression values, since the compiler can calculate more values at compile time. I think this is generally a good thing.<br>
<br>
Yes, I'm just wondering if this effect was intended here before I<br>
start jumping through hoops to avoid it.</blockquote><div><br></div><div style>Chances are this warning just needs to be reclassified/moved to an AnalysisBasedWarning so that it doesn't fire on possibly-unreachable code. Your example above is a fairly classic one of how people might reasonable write this & we use similar solutions for other warnings with such issues.<br>
<br>(alternatively, you can rewrite your checks with preprocessor macros instead - but then you loose the syntax checking on that code when you're in the other build mode, of course, which is unfortunate/undesirable) </div>
</div></div></div>