[Issue 7632] Incorrect _Py_dg_strtod result for long strings

[eric2, 2010-01-15 22:41:28]

I have a few minor comments, but given the constraints of working with the
original code, it looks good.

Eric.

http://codereview.appspot.com/186168/diff/1003/1004
File Python/dtoa.c (right):

http://codereview.appspot.com/186168/diff/1003/1004#newcode1479
Python/dtoa.c:1479: * s0, nd0, nd, e, y and z such that:
Are these the only local variables that carry forward to the rest of this
routine? I know you probably don't want to completely restructure this, but it
would be much easier to analyze this if the code before this point was in a
separate function that communicated with this code only through these
parameters. As it is, it's not at all easy to see which of the many local
variables communicate information throughout this routine.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1493
Python/dtoa.c:1493: *    currently assumes that it's nonzero; this is a bug)
Where is the "some code" that makes the assumption? In this file, or elsewhere?
Should we fix that code, or at least file a bug?

http://codereview.appspot.com/186168/diff/1003/1004#newcode1627
Python/dtoa.c:1627: if ((i < nd0 ? s0[i] : s0[i+1]) != '0') {
It doesn't really make much difference, but I would write this as:
if (s0[i < nd0 ? i : i+1] != '0') {
I think it makes it more clear that s0 is always the thing being indexed. Of
course then I'd have to worry about whether or not I need parens around i+1.

[dickinsm, 2010-01-16 08:34:31]

http://codereview.appspot.com/186168/diff/1003/1004
File Python/dtoa.c (right):

http://codereview.appspot.com/186168/diff/1003/1004#newcode1309
Python/dtoa.c:1309: if (i < nd - 1)
The next line assumes that at least one of the digits beyond this point is
nonzero, which needn't be true.  If all digits are zero, dd should be 0 rather
than 1.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1325
Python/dtoa.c:1325: if (i < nd - 1)
Ditto for the next line.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1479
Python/dtoa.c:1479: * s0, nd0, nd, e, y and z such that:
On 2010/01/15 22:41:28, eric2 wrote:
> it
> would be much easier to analyze this if the code before this point was in a
> separate function that communicated with this code only through these
> parameters.

Agreed.  I am interested in doing some more serious refactoring, but I'll try to
fix the known bugs non-invasively first.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1493
Python/dtoa.c:1493: *    currently assumes that it's nonzero; this is a bug)
In this file.  I've flagged those places with comments.

The bugs are already filed: they're bug 5 and bug 7 in the tracker.  The fix I'm
working on involves rewriting the parser so that nd never takes trailing zeros
into account.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1627
Python/dtoa.c:1627: if ((i < nd0 ? s0[i] : s0[i+1]) != '0') {
Will do.  Thanks.

http://codereview.appspot.com/186168/diff/1003/1004#newcode1793
Python/dtoa.c:1793: i = -1; /* Discarded digits make delta smaller. */
This code assumes that at least one of the discarded digits is nonzero, which
needn't be true.  If all discarded digits are zero and i == 0, then i should be
left unchanged here.