gh-97786: Fix compiler warnings in pytime.c #101826
Conversation
|
If you want to schedule another build, you need to add the |
| @@ -910,7 +933,7 @@ py_get_system_clock(_PyTime_t *tp, _Py_clock_info_t *info, int raise_exc) | |||
| info->monotonic = 0; | |||
| info->adjustable = 1; | |||
| if (clock_getres(CLOCK_REALTIME, &res) == 0) { | |||
| info->resolution = res.tv_sec + res.tv_nsec * 1e-9; | |||
| info->resolution = (double)res.tv_sec + (double)res.tv_nsec * 1e-9; | |||
There was a problem hiding this comment.
Clang's -Wimplicit-int-float-conversion was warning here; the casts silence those warnings.
|
I've removed the The only possible issue with a non-integer
So the failure mode is benign. But in any case, |
|
I think this is ready to merge. @gpshead Do you have bandwidth for a quick re-review? |
|
Thanks @mdickinson for the PR, and @gpshead for merging it |
|
Sorry, @mdickinson and @gpshead, I could not cleanly backport this to |
Fixes compiler warnings in pytime.c. (cherry picked from commit b1b375e2670a58fc37cb4c2629ed73b045159918) Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
|
GH-102062 is a backport of this pull request to the 3.11 branch. |
This PR fixes some compiler warnings in
pytime.c, and at the same time fixes our out-of-range double-to-integer checks to properly avoid undefined behaviour.It's hard to write strictly portable code for this kind of thing, but the new check should work under a set of (very) mild assumptions, that are highly unlikely to be violated on any actual platform that we care about:
time_tis a two's complement signed integer type with no trap representation. (The weakest part of this is the assumption thattime_tis a signed integer type at all, but we're already making that assumption.)_PyTime_tis also a two's complement signed integer type with no trap representation. (This is already true with the currenttypedef int64_t _PyTime_t;declaration.)PY_TIME_T_MINand_PyTime_MINare within the range of a C double. These days we're assuming IEEE 754 binary64 doubles, so we're safe so long as the integer types_PyTime_tandtime_tdon't have a width of more than 1024.Here's the underlying logic for the changes, for the record:
xis within the range of a (potentially unknown) signed integer typeIwith min and max valuesIMINandIMAX. More specifically, we want to be sure that a conversion fromxto typeIwill not invoke undefined behaviour; this means that the integer part ofx(discarding the fractional part, if any) must be in[IMIN, IMAX].Iuses two's complement with no trap representation,IMINmust be the negation of a power of two, andIMAX == -1 - IMIN.IMINis exactly representable as adouble(assuming that it's not larger than2**1023), and(double)IMINdoes not change the value.(double)IMIN <= x, does exactly what we want it to.x <= IMAXis problematic since the implicit conversion ofIMAXto typedoublemay change the value. But assuming thatxis an integer (which it is for all cases in this PR),x <= IMAXis mathematically equivalent tox < (IMAX + 1), which with our two's complement assumption is equivalent tox < -IMIN, and we can express this asx < -(double)IMIN._Py_InIntegralTypeRange#97786