LGTM.
http://codereview.chromium.org/1096002/diff/3003/39003 File src/conversions.cc (right): http://codereview.chromium.org/1096002/diff/3003/39003#newcode280 src/conversions.cc:280: // Hexidecomal may have (52) / 4 + 1 significant digit. Mean of 2 On 2010/03/25 12:26:16, Erik Corry wrote:
Hexidecomal -> Hexadecimal digit -> digits I can't understand the sentence that starts "Mean of 2"
Maybe the following explanation is easier to understand: A double has a 53bit significand (once the hidden bit has been added). Halfway cases thus have at most 54bits. Therefore 54/4 + 1 digits are sufficient to represent halfway cases. By adding another digit we can keep track of dropped digits. This explanation is still not perfect, but probably better than the current comment. http://codereview.chromium.org/1096002/diff/3003/39003#newcode413 src/conversions.cc:413: return (buffer_pos > 0 && buffer[0] == '-') ? -result : result; On 2010/03/25 12:26:16, Erik Corry wrote:
Should this be buffer[buffer_pos - 1]?
lgtm. http://codereview.chromium.org/1096002/diff/3003/39003#newcode546 src/conversions.cc:546: // ALLOW_OCTALS has set and there is no '8' and '9' in insignificant ALLOW_OCTALS is set ... http://codereview.chromium.org/1096002/diff/3003/39001 File test/cctest/test-conversions.cc (right): http://codereview.chromium.org/1096002/diff/3003/39001#newcode154 test/cctest/test-conversions.cc:154: int n = snprintf(buffer, sizeof(buffer), "%.1000Le", (a + b) / 2); It is (imho) better not to rely on snprintf. Precomputing the string and pasting it here will use up more than 10 lines, but at least there would be no dependency on sprintf. Also it is not certain that sprintf behaves correctly on all platforms (for instance on embedded devices). http://codereview.chromium.org/1096002 -- 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.
