http://codereview.chromium.org/1374005/diff/6001/7003
File src/conversions.cc (right):
http://codereview.chromium.org/1374005/diff/6001/7003#newcode277
src/conversions.cc:277: static bool inline isDigit(int x, int radix) {
On 2010/03/29 11:12:23, Florian Loitsch wrote:
I don't think it's necessary to mark a function as 'inline'. The
compiler is
usually better at these things than we are.
ymmv
Done.
http://codereview.chromium.org/1374005/diff/6001/7003#newcode285
src/conversions.cc:285: template <int radix_log_2, class Iterator, class
EndMark>
On 2010/03/29 11:12:23, Florian Loitsch wrote:
Add comment that current must not be end.
Done.
http://codereview.chromium.org/1374005/diff/6001/7003#newcode318
src/conversions.cc:318: number = (number << radix_log_2) | digit;
On 2010/03/29 11:12:23, Florian Loitsch wrote:
number = number * radix + digit.
The compiler will be smart enough to change that into a shifts and
pluses.
Done.
http://codereview.chromium.org/1374005/diff/6001/7003#newcode335
src/conversions.cc:335: if (dropped_bits >= middle_value) {
On 2010/03/29 11:12:23, Florian Loitsch wrote:
Since half-way cases do not always round down (see my next comment) I
suggest
splitting into three cases:
if (dropped_bits < middle_value) {
// round down
} else if (dropped_bits > middle_value) {
// round up
} else { // dropped_bits == middle_value
// wait for next digits
}
Done.
http://codereview.chromium.org/1374005/diff/6001/7003#newcode338
src/conversions.cc:338: // if the tail consists of zeros.
On 2010/03/29 11:12:23, Florian Loitsch wrote:
I don't think this is the right approach. Generally we prefer rounding
towards
even. That is, if the last non-dropped digit is even, round down,
otherwise
round up.
Otherwise we obtain different numbers when reading a number as decimal
or as
hexadecimal.
OK, I changed code to round half-way numbers (with insignificant bits
'10...0b') to even.
http://codereview.chromium.org/1374005/diff/6001/7003#newcode339
src/conversions.cc:339: do {
On 2010/03/29 11:12:23, Florian Loitsch wrote:
Since we need to run through the remaining digits anyways, it would
probably
make sense to determine the final rounding direction in the next loop.
The 'if'-case would be smaller, and I don't think it would add too
much in the
next loop.
I moved the loop up and added a variable 'zero_tail' to achieve the same
effect.
http://codereview.chromium.org/1374005/diff/6001/7003#newcode352
src/conversions.cc:352: } else { // driopped_bit > middle_value
On 2010/03/29 11:12:23, Florian Loitsch wrote:
dropped
This line has gone.
http://codereview.chromium.org/1374005/diff/6001/7003#newcode397
src/conversions.cc:397: printf("Exponent: %d (%f))\n", exponent,
pow(2.0, exponent));
On 2010/03/29 11:12:23, Florian Loitsch wrote:
remove printf.
Done.
http://codereview.chromium.org/1374005/diff/6001/7002
File test/mjsunit/str-to-num.js (right):
http://codereview.chromium.org/1374005/diff/6001/7002#newcode1
test/mjsunit/str-to-num.js:1:
On 2010/03/29 11:20:57, Florian Loitsch wrote:
No need for new line.
Done.
http://codereview.chromium.org/1374005
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
To unsubscribe from this group, send email to v8-dev+unsubscribegooglegroups.com or reply
to this email with the words "REMOVE ME" as the subject.
