Bug 107503 – Fix DateMath.cpp to compile with -Wshorten-64-to-32

• Status: NEW
• Component: Web Template Framework

[David Kilzer (:ddkilzer)]

<http://webkit.org/b/000000>

Reviewed by NOBODY (OOPS!).

Source/JavaScriptCore:

* runtime/JSDateMath.cpp:
(JSC::parseDateFromNullTerminatedCharacters): Remove unneeded
static_cast<int>().

Source/WTF:

Fixes the following build errors with -Wshorten-64-to-32:

DateMath.cpp:742:47: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
if (month == 2 && day > 28 && !isLeapYear(year))
~~~~~~~~~~ ^~~~
DateMath.cpp:757:48: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
double dateSeconds = ymdhmsToSeconds(year, month, day, hours, minutes, seconds) - timeZoneSeconds;
~~~~~~~~~~~~~~~       ^~~~~
DateMath.cpp:757:55: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
double dateSeconds = ymdhmsToSeconds(year, month, day, hours, minutes, seconds) - timeZoneSeconds;
~~~~~~~~~~~~~~~              ^~~
DateMath.cpp:757:60: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
double dateSeconds = ymdhmsToSeconds(year, month, day, hours, minutes, seconds) - timeZoneSeconds;
~~~~~~~~~~~~~~~                   ^~~~~
DateMath.cpp:757:67: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
double dateSeconds = ymdhmsToSeconds(year, month, day, hours, minutes, seconds) - timeZoneSeconds;
~~~~~~~~~~~~~~~                          ^~~~~~~
DateMath.cpp:996:59: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
offset = ((o / 100) * 60 + (o % 100)) * sgn;
~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
DateMath.cpp:998:37: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
offset = o * 60 * sgn;
~ ~~~~~~~^~~~~
DateMath.cpp:1005:40: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
offset = (o * 60 + o2) * sgn;
~ ~~~~~~~~~~~~~~^~~~~
DateMath.cpp:1041:40: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
return ymdhmsToSeconds(year, month + 1, day, hour, minute, second) * msPerSecond;
~~~~~~~~~~~~~~~       ~~~~~~^~~
DateMath.cpp:1041:45: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
return ymdhmsToSeconds(year, month + 1, day, hour, minute, second) * msPerSecond;
~~~~~~~~~~~~~~~                  ^~~
DateMath.cpp:1041:50: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
return ymdhmsToSeconds(year, month + 1, day, hour, minute, second) * msPerSecond;
~~~~~~~~~~~~~~~                       ^~~~
DateMath.cpp:1041:56: error: implicit conversion loses integer precision: 'long' to 'int' [-Werror,-Wshorten-64-to-32]
return ymdhmsToSeconds(year, month + 1, day, hour, minute, second) * msPerSecond;
~~~~~~~~~~~~~~~                             ^~~~~~
12 errors generated.

* wtf/DateMath.cpp:
(WTF::ymdhmsToSeconds): Change year argument from long to int.
Change mon, day, hour, minute arguments from int to long.
(WTF::parseInt): Add.  Identical to parseLong but bounds checks
for type int.
(WTF::parseLong): Switch to std::numeric_limits<long> instead of
macros.
(WTF::parseES5DatePortion): Change year argument from long to
int.
(WTF::parseES5DateFromNullTerminatedCharacters): Change year
local variable from long to int.
(WTF::parseDateFromNullTerminatedCharacters): Change year and
offset local variables from long to int.  Switch from using
parseLong() to parseInt() as needed.  Ditto for labs() to abs().
Add overflow check when switching from "MM/DD/YYYY" to
"YYYY/MM/DD" parsing.
---
 4 files changed, 104 insertions(+), 17 deletions(-)

[David Kilzer (:ddkilzer)]

Created attachment 183893 [details]
Patch

[David Kilzer (:ddkilzer)]

Created attachment 183996 [details]
Patch 2 of 2

[David Kilzer (:ddkilzer)]

(In reply to comment #1)
> Created an attachment (id=183893) [details]
> Patch

The general idea here is to switch 'offset' and 'year' to be type 'int', and continue using 'long' for other time values.

Patch 2 of 2 is cleanup to merge parseInt() and parseLong() into a single template function.

[Darin Adler]

Comment on attachment 183996 [details]
Patch 2 of 2

Your changes are OK.

There’s a lot that is wrong about this parse function for WebKit. It’s basically a cover for the strtol library function:

- The workaround for Windows CE behavior is done here, but in WebKit such workarounds are supposed to go into <wtf/MathExtras.h>
- The function can’t parse the minimum or maximum values of a given type, an unconventional limitation that I assume is not a problem in the date parsing code.
- The function uses the strtol library function. But we have good numeric parsing functions elsewhere in WTF, which probably didn’t exist when this was first written. The space skipping behavior of strtol is based on isspace and thus is affected by the POSIX locale set in the standard library, which is why we don’t use these kinds of library functions elsewhere in WebKit, and also why our ASCIICType.h header exists.
- The function retains the strtol “base” argument, even though callers always pass 10.
- The function uses pointers for its out arguments. While some of the Chromium project contributors have argued for this, it is not the usual WebKit coding style. We use references for such things.

Because of that, if I was fixing this, I’d probably also rewrite this to not use strtol.

[Darin Adler]

Comment on attachment 183893 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=183893&action=review

Seems OK. Things are still a bit of a mess. If we had more time to clean up further, I think we could stop using long here. There’s no reason we need to use strtol when we have perfectly good numeric conversion functions of our own.

> Source/WTF/wtf/DateMath.cpp:837
> +        if (day <= std::numeric_limits<int>::min() || day >= std::numeric_limits<int>::max())

These should be < and > rather than <= or >=. Unless there is some reason the limit values are a problem. Except that probably will cause warnings on platforms where long and int are the same size. Ugh.

[WebKit Review Bot]

Comment on attachment 183893 [details]
Patch

Clearing flags on attachment: 183893

Committed r140437: <http://trac.webkit.org/changeset/140437>

[David Kilzer (:ddkilzer)]

Comment on attachment 183996 [details]
Patch 2 of 2

I'm going to file some follow-up bugs based on Darin's comments rather than landing this patch.