Reviewed up to, but not including, DigitGenCounted in fast-dtoa.cc.


http://codereview.chromium.org/2000004/diff/19001/14002
File src/fast-dtoa.cc (right):

http://codereview.chromium.org/2000004/diff/19001/14002#newcode78
src/fast-dtoa.cc:78: // ]w_low; w_low[ (often written as "(w_low;
w_low)")
(w_low, w_high)

http://codereview.chromium.org/2000004/diff/19001/14002#newcode126
src/fast-dtoa.cc:126: // the buffer. If we have two potential
representations we need to make sure
when finding the closest representation of 'w'.  If we have two
potential representations, and one is closer to both w_low and w_high,
then we know it is closer to the actual value v.

http://codereview.chromium.org/2000004/diff/19001/14002#newcode146
src/fast-dtoa.cc:146: buffer[length - 1]--;
Comment that all these tests are necessary in this order to detect
overflow and underflow of uint64 values.  ASSERT(rest <=
unsafe_interval).

http://codereview.chromium.org/2000004/diff/19001/14002#newcode169
src/fast-dtoa.cc:169: // Rounds the buffer according to rest for the
counted digit generation method.
Rounds the buffer upwards, if the result is closer to v, by possibly
adding 1 to the buffer.  If the precision of the calculation is not
sufficient to round correctly, return false.

http://codereview.chromium.org/2000004/diff/19001/14002#newcode192
src/fast-dtoa.cc:192: // over/underflow.)
Comment that these tests are designed to avoid overflow, and will work
correctly for any uint64 values of rest < ten_kappa and unit.

http://codereview.chromium.org/2000004/diff/19001/14002#newcode210
src/fast-dtoa.cc:210: // the power (the kappa) is increased.
This is slightly weird because the calling function will need to update
ten_kappa, but also needs to update rest with the old value of
ten_kappa.  Does this function have the right interface, or could it be
simplified by doing more or less?

http://codereview.chromium.org/2000004/diff/19001/14002#newcode232
src/fast-dtoa.cc:232: // Precondition: (1 << number_bits) <= number < (1
<< (number_bits + 1)).
Because we always test, in the switch statement, the first inequality of
the precondition is not actually necessary.
Should you avoid the testing in the switch statement (for example, if
number_bits = 31, you know number > kTen9)?

http://codereview.chromium.org/2000004/diff/19001/25001
File src/fast-dtoa.h (right):

http://codereview.chromium.org/2000004/diff/19001/25001#newcode35
src/fast-dtoa.h:35: // 0.9999999999999999 becomes 0.1
I don't understand this comment.

http://codereview.chromium.org/2000004/diff/19001/25001#newcode59
src/fast-dtoa.h:59: //     0.099999999999 instead of 0.1.
If 0.099999999999 and 0.1 represent the same double value, "1" is
returned with point = 0.

http://codereview.chromium.org/2000004/diff/19001/25001#newcode65
src/fast-dtoa.h:65: //     the difference v - (buffer *
10^(point-length)) is the smallest for all
is the closest to zero
has the smallest absolute value

if two values are equally close, we round to an even last digit.

http://codereview.chromium.org/2000004/diff/19001/25001#newcode66
src/fast-dtoa.h:66: //     possible decimal numbers of requested_digits
digits.
for all decimal integers "buffer" with requested_digits digits.

I don't like quotes though.

http://codereview.chromium.org/2000004/diff/19001/25001#newcode67
src/fast-dtoa.h:67: // For both modes the buffer must be big enough to
hold the result.
must be large enough

http://codereview.chromium.org/2000004/show

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

Reply via email to