LGTM.  Some further changes to comments suggested.

http://codereview.chromium.org/3468003/diff/61001/src/bignum-dtoa.cc
File src/bignum-dtoa.cc (right):

http://codereview.chromium.org/3468003/diff/61001/src/bignum-dtoa.cc#newcode243
src/bignum-dtoa.cc:243: // The initial start values consist of:
How about:
Computes v / 10^estimated_power exactly, as a ratio of two bignums,
numerator and denominator. The functions GenerateShortestDigits and
GenerateCountedDigits will then convert this ratio to its decimal
representation, d, with the required accuracy, and then d *
10^estimated_power is the representation of v.

http://codereview.chromium.org/3468003/diff/61001/src/bignum-dtoa.cc#newcode246
src/bignum-dtoa.cc:246: //  optionally (depending on the flag
need_boundary_deltas):
Might as well say here: used by GenerateShortestDigits to decide if it
has the shortest decimal converting back to v.

http://codereview.chromium.org/3468003/diff/61001/src/bignum-dtoa.cc#newcode249
src/bignum-dtoa.cc:249: // The scaling consist of multiplying the
numerator by 10^estimated_power, or
Not needed.  This will be obvious from the code, or commented there.

http://codereview.chromium.org/3468003/diff/61001/src/bignum-dtoa.cc#newcode252
src/bignum-dtoa.cc:252: // Note that the boundary-deltas are scaled too.
If the common denominator has
Not needed, except to say that v, m+, m-, and therefore v - m- and m+ -
v all share the same denominator.

http://codereview.chromium.org/3468003/diff/61001/src/bignum-dtoa.cc#newcode338
src/bignum-dtoa.cc:338: // when the generated digits yield a number that
is close enough. The number
yield the shortest decimal representation of v.  A decimal
representation of v is a number lying closer to v than to any other
double, so it converts to v when read.

This is true if d, the decimal representation, is between m- and m+, the
upper and lower boundaries.  d must be strictly between them if
!is_even.
m+ = ...
m- = ...

http://codereview.chromium.org/3468003/diff/61001/src/bignum-dtoa.cc#newcode455
src/bignum-dtoa.cc:455: // Let v = numerator / denominator.
// Let v = numerator / denominator < 10.
// Then we generate 'count' digits of d = x.xxxxxxx... from left to
right. Once 'count' digits have
been produced we decide whether to round up or down. Remainders of
exactly .5 round upwards.  Numbers such 9.99999 propagate a carry all
the way, and change
the exponent, when rounding upwards.

http://codereview.chromium.org/3468003/diff/61001/src/bignum-dtoa.cc#newcode571
src/bignum-dtoa.cc:571: Bignum numerator;
As long as there is a header file, could the order of the functions be
reversed, putting the major functions first, and the helpers they call
later?  It would be easier to read.

Or even use forward declarations for the static functions, with a short
description of their function?

http://codereview.chromium.org/3468003/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to