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 Hexidecomal -> Hexadecimal digit -> digits I can't understand the sentence that starts "Mean of 2" http://codereview.chromium.org/1096002/diff/3003/39003#newcode281 src/conversions.cc:281: // hexidecimal may have n + 1. hexidecimal -> hexadecimal http://codereview.chromium.org/1096002/diff/3003/39003#newcode282 src/conversions.cc:282: const int max_significant_digits = (52) / 4 + 2; I don't see the point in putting 52 in brackets here. max_significant_digits should be named as a constant ie kMaxSignificantDigits http://codereview.chromium.org/1096002/diff/3003/39003#newcode328 src/conversions.cc:328: // Multiplying by power of 2 doesn't cause a loss of precision. power -> a power http://codereview.chromium.org/1096002/diff/3003/39003#newcode345 src/conversions.cc:345: // To make sure that iterator unreferencing is valid the following unreferencing -> dereferencing http://codereview.chromium.org/1096002/diff/3003/39003#newcode351 src/conversions.cc:351: // 4. 'current' is not unreferenced after the 'parsing_done' label. and here http://codereview.chromium.org/1096002/diff/3003/39003#newcode358 src/conversions.cc:358: const int max_significant_digits = 772; Name as a constant. Is it possible to provide a one or two line explanation for why this is enough? http://codereview.chromium.org/1096002/diff/3003/39003#newcode360 src/conversions.cc:360: const int buffer_size = max_significant_digits + 10; Name as a constant. http://codereview.chromium.org/1096002/diff/3003/39003#newcode374 src/conversions.cc:374: // Ignore leading sign; Skip following spaces. Skip -> skip http://codereview.chromium.org/1096002/diff/3003/39003#newcode384 src/conversions.cc:384: static const char infinity_symbol[] = "Infinity"; Name as a constant. http://codereview.chromium.org/1096002/diff/3003/39003#newcode409 src/conversions.cc:409: double result = InternalHexidecimalStringToDouble(current, Hexi -> Hexa http://codereview.chromium.org/1096002/diff/3003/39003#newcode413 src/conversions.cc:413: return (buffer_pos > 0 && buffer[0] == '-') ? -result : result; Should this be buffer[buffer_pos - 1]? 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 '8' and '9' -> '8' or '9' 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.
