r180603 - C++1y: support simple variable assignments in constexpr functions.

[Hans Wennborg]

On Mon, May 20, 2013 at 5:14 PM, Jordan Rose <jordan_rose at apple.com> wrote:
>
> On May 20, 2013, at 6:59 , Hans Wennborg <hans at chromium.org> wrote:
>
>> Hi Richard,
>>
>> On Fri, Apr 26, 2013 at 3:36 PM, Richard Smith
>> <richard-llvm at metafoo.co.uk> wrote:
>>> Author: rsmith
>>> Date: Fri Apr 26 09:36:30 2013
>>> New Revision: 180603
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=180603&view=rev
>>> Log:
>>> C++1y: support simple variable assignments in constexpr functions.
>>
>> This seems to have caused -Winteger-overflow to become much more aggressive.
>>
>> I don't know what in your code causes this, but I bisected it down to
>> this revision.
>>
>> The following code didn't use to warn, but now does (only in C++11 mode though):
>>
>> #include <time.h>
>> #include <limits>
>>
>> void f() {
>>  long long x;
>>  x = std::numeric_limits<time_t>::min() * 1000;
>> }
>>
>> /tmp/a.cc:6:42: warning: overflow in expression; result is 0 with type
>> 'long' [-Winteger-overflow]
>>  x = std::numeric_limits<time_t>::min() * 1000;
>>                                         ^
>>
>> This is causing problems in Chromium where we have such code, but we
>> know we won't hit it on platforms where time_t is 64-bit.
>>
>> Was the warning behaviour change intentional?
>
> That looks like a real warning to me:
> - 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
> - if time_t is signed 64-bit, the min value will be -2^63, and multiplying by 1000 will result in negative overflow
>
> 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).

Sorry, my example was a simplified version of the original Chromium
code. We do handle the 32-bit case, 1000 is really a 64-bit constant
variable (static const long long kMillisecondsPerSecond = 1000).

> The 64-bit case will never be right unless your long long is 128 bits.

Right, but we never hit that code in the 64-bit case. The problem is
that Clang doesn't know that. And it doesn't help if I add very
explicit guards either, I can do this:

void f() {
  long long x;
  if (sizeof(time_t) == 4)
    x = std::numeric_limits<time_t>::min() * 1000LL;
  else
    x = 5;
}

and it will still warn.

> 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.

Yes, I'm just wondering if this effect was intended here before I
start jumping through hoops to avoid it.

Thanks,
Hans