All reviewed except fixed-dtoa.cc. Still working on that one.
http://codereview.chromium.org/1865001/diff/1/5002 File src/dtoa.cc (right): http://codereview.chromium.org/1865001/diff/1/5002#newcode68 src/dtoa.cc:68: if (FastDtoa(v, buffer, length, point)) return true; What does FastDtoa return if not "true"? If you don't use this value, why not return true, for parallelism with FastFixedDtoa? http://codereview.chromium.org/1865001/diff/1/13001 File src/dtoa.h (right): http://codereview.chromium.org/1865001/diff/1/13001#newcode37 src/dtoa.h:37: // Fixed number of digits after the comma. after the radix point (or decimal point). To me, commas are what go between every three digits. http://codereview.chromium.org/1865001/diff/1/13001#newcode62 src/dtoa.h:62: // 'requested_digits' digits after the comma. The produced digits might be too decimal point http://codereview.chromium.org/1865001/diff/1/13001#newcode69 src/dtoa.h:69: // any other digit would not satisfy the internal identity requirement. I would say "The returned buffer may contain digits that would be truncated from the shortest representation of the input." http://codereview.chromium.org/1865001/diff/1/13001#newcode72 src/dtoa.h:72: // 'requested_digits', the function is allowed to return less digits, in which fewer digits http://codereview.chromium.org/1865001/diff/1/19001 File src/fast-dtoa.h (right): http://codereview.chromium.org/1865001/diff/1/19001#newcode39 src/fast-dtoa.h:39: // v must be strictly greater than 0 and it must not be Infinity or NaN. A strictly positive finite double? http://codereview.chromium.org/1865001/diff/1/21001 File src/fixed-dtoa.h (right): http://codereview.chromium.org/1865001/diff/1/21001#newcode35 src/fixed-dtoa.h:35: // 'fractional_count' digits after the comma. radix point, decimal point, radix marker? You are not even typing a comma in your examples - you are typing a period ("0.001"). http://codereview.chromium.org/1865001/diff/1/21001#newcode43 src/fixed-dtoa.h:43: // Note: the length of the returned buffer has no meaning wrt the significance Same change to wording as suggested earlier. http://codereview.chromium.org/1865001/diff/1/3001 File test/cctest/gay-fixed.h (right): http://codereview.chromium.org/1865001/diff/1/3001#newcode35 test/cctest/gay-fixed.h:35: double v; Are these just precomputed values? What do they have to do with the Gay algorithm, except that it was used to compute them? Why not "PrecomputedFixed"? http://codereview.chromium.org/1865001/diff/1/5001 File test/cctest/test-fixed-dtoa.cc (right): http://codereview.chromium.org/1865001/diff/1/5001#newcode77 test/cctest/test-fixed-dtoa.cc:77: CHECK(FastFixedDtoa(6.9999999999999989514240000e+21, 5, buffer, &length, &point)); Line too long? http://codereview.chromium.org/1865001/show -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
