On Mon, May 20, 2013 at 9:33 AM, Hans Wennborg <[email protected]> wrote:
> On Mon, May 20, 2013 at 5:14 PM, Jordan Rose <[email protected]> > wrote: > > > > On May 20, 2013, at 6:59 , Hans Wennborg <[email protected]> wrote: > > > >> Hi Richard, > >> > >> On Fri, Apr 26, 2013 at 3:36 PM, Richard Smith > >> <[email protected]> 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. Yes, this was intentional. When checking for overflow, we were trying to visit all subexpressions by hitting the full-expression with the constant expression evaluator in its 'keep going after a failure' mode, but the code which handled assignment operators didn't correctly deal with this case.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
