◐ Shell
reader mode source ↗
Skip to content

bpo-39277, pytime: Fix overflow check on double to int cast#17933

Closed
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:pytime_round
Closed

bpo-39277, pytime: Fix overflow check on double to int cast#17933
vstinner wants to merge 1 commit into
python:mainfrom
vstinner:pytime_round

Conversation

@vstinner

@vstinner vstinner commented Jan 10, 2020

Copy link
Copy Markdown
Member

Fix time.sleep() to properly detect integer overflow when converting
a floating-point number of seconds to an integer.

https://bugs.python.org/issue39277

Fix time.sleep() to properly detect integer overflow when converting
a floating-point number of seconds to an integer.
@vstinner

Copy link
Copy Markdown
Member Author

@csabella csabella requested a review from mdickinson May 28, 2020 23:04
@mdickinson

mdickinson commented May 29, 2020

Copy link
Copy Markdown
Member

A safe way to do what you want is:

(1) Take the integral part of the double v that you want to check, for example using modf, still represented as a double. (Note that the integral part of an IEEE 754 floating-point number is always exactly representable in the same format.)
(2) Check that (double)TYPE_MIN <= int_part_of_v < (double)(TYPE_MAX + 1) (using a mixture of C casts and Python chained comparisons here, but you know what I mean)

Computing TYPE_MAX + 1 while avoiding overflow may take some type-specific effort.

The reason that step (2) is safe is that, making the fairly safe assumption of 2s complement (no trap representation, etc., etc.) for signed integers, TYPE_MIN and TYPE_MAX + 1 will always be exactly representable as a double (well, assuming that we're not dealing with 1024-bit integers, that is). But TYPE_MAX can't be assumed to be exactly representable as a double - whether it is or not will depend on the width of the type being tested.

@mdickinson mdickinson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hide comment

This isn't the right way to check for convertibility; see the other comments I made on the PR.

@bedevere-bot

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

And if you don't make the requested changes, you will be put in the comfy chair!

@graingert

Copy link
Copy Markdown
Contributor

@vstinner any update on this?

@vstinner

Copy link
Copy Markdown
Member Author

I failed finding time to finish the PR. I prefer to abandon it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants