-
-
Notifications
You must be signed in to change notification settings - Fork 31.6k
bpo-40780: Fix failure of _Py_dg_dtoa to remove trailing zeros #20435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
bpo-40780: Fix failure of _Py_dg_dtoa to remove trailing zeros #20435
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tracking this down, @mdickinson. It's hard to believe that after all these years a bug like this would still exist.
This change looks good to me.
Is it worth adding this to the comment at line 30? Or is this too trivial to matter?
Also, if this really is a bug in Gay's code, should we report it upstream? Or has too much water passed under the bridge for it to matter any more?
Yes, I'll do that. Reporting upstream seems like the right thing to do - I'll do that, too. |
I changed the backport labels: as discussed on the issue, let's not backport to 3.7 or 3.8. I also added a comment about the change to the top of the dtoa.c file, and reworked the logic so that it's clearer that the zero-stripping is safe. Note that it's fine to return an empty string, and that's mostly what dtoa.c already does when the formatted output will be zero. (The exception is on an exact zero, where the string |
The upstream code has diverged, and this case is already fixed upstream, using essentially the same code that I ended up using here (but in a different place). So I won't report upstream. I'm looking at http://www.netlib.org/fp/dtoa.c directly; I can't find any sort of changelog or other information about what changed and when, unfortunately. There may be other upstream fixes that we should be looking at applying, but I don't see a way to find out what those fixes are. The relevant code in the current snapshot of dtoa.c is at line 6163, and looks like this:
|
Thank you for fixing this, @mdickinson. This experience has reminded me why I love open-source software development. ❤️ |
Hi @ericvsmith. I made a few changes; would you have a moment to take a second look? Sorry for the delay here. I got bogged down trying to understand more of the way that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Thanks @mdickinson for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
GH-20514 is a backport of this pull request to the 3.9 branch. |
) (GH-20514) * Fix failure of _Py_dg_dtoa to remove trailing zeros * Add regression test and news entry * Add explanation about why it's safe to strip trailing zeros * Make code safer, clean up comments, add change note at top of file * Nitpick: avoid implicit int-to-float conversion in tests (cherry picked from commit 895c9c1) Co-authored-by: Mark Dickinson <mdickinson@enthought.com>
* 'master' of github.com:python/cpython: (497 commits) bpo-40061: Fix a possible refleak in _asynciomodule.c (pythonGH-19748) bpo-40798: Generate a different message for already removed elements (pythonGH-20483) closes bpo-29017: Update the bindings for Qt information with PySide2 (pythonGH-20149) bpo-39885: Make IDLE context menu cut and copy work again (pythonGH-18951) bpo-29882: Add an efficient popcount method for integers (python#771) Further de-linting of zoneinfo module (python#20499) bpo-40780: Fix failure of _Py_dg_dtoa to remove trailing zeros (pythonGH-20435) Indicate that abs() method accept argument that implement __abs__(), just like call() method in the docs (pythonGH-20509) bpo-39040: Fix parsing of email mime headers with whitespace between encoded-words. (pythongh-17620) bpo-40784: Fix sqlite3 deterministic test (pythonGH-20448) bpo-30064: Properly skip unstable loop.sock_connect() racing test (pythonGH-20494) Note the output ordering of combinatoric functions (pythonGH-19732) bpo-40474: Updated coverage.yml to better report coverage stats (python#19851) bpo-40806: Clarify that itertools.product immediately consumes its inpt (pythonGH-20492) bpo-1294959: Try to clarify the meaning of platlibdir (pythonGH-20332) bpo-37878: PyThreadState_DeleteCurrent() was not removed (pythonGH-20489) bpo-40777: Initialize PyDateTime_IsoCalendarDateType.tp_base at run-time (pythonGH-20493) bpo-40755: Add missing multiset operations to Counter() (pythonGH-20339) bpo-25920: Remove socket.getaddrinfo() lock on macOS (pythonGH-20177) bpo-40275: Fix test.support.threading_helper (pythonGH-20488) ...
The
dtoa.c
code underlying string formatting wasn't stripping trailing zeros in some cases where it should have been. This PR fixes that.Making this a draft PR for now; I still need to add regression tests.EDIT: tests addedhttps://bugs.python.org/issue40780