Mostly nits.
http://codereview.chromium.org/19537/diff/1/8 File src/factory.cc (right): http://codereview.chromium.org/19537/diff/1/8#newcode163 Line 163: script->set_line_ends(*(Factory::NewFixedArray(0))); Use Heap::undefined_value() to mark not initialized. http://codereview.chromium.org/19537/diff/1/7 File src/log.cc (right): http://codereview.chromium.org/19537/diff/1/7#newcode589 Line 589: Please use two empty lines between methods. http://codereview.chromium.org/19537/diff/1/7#newcode604 Line 604: Please use two empty lines between methods. http://codereview.chromium.org/19537/diff/1/7#newcode616 Line 616: Please use two empty lines between methods. http://codereview.chromium.org/19537/diff/1/6 File src/objects-inl.h (right): http://codereview.chromium.org/19537/diff/1/6#newcode1992 Line 1992: ACCESSORS(Script, line_ends, FixedArray, kLineEndsOffset) You should use Object here to allow it to be initialized to undefined. http://codereview.chromium.org/19537/diff/1/3 File src/objects.cc (right): http://codereview.chromium.org/19537/diff/1/3#newcode6780 Line 6780: if (line_ends()->length() > 0) return; Check for undefined using IsUndefined(). Add an ASSERT IsFixedArray (or IsJSArray - see below) before returning. http://codereview.chromium.org/19537/diff/1/3#newcode6783 Line 6783: SmartPointer<uc16> data = src->ToWideCString(); ToWideCString is only for debugging and logging purposes. Try to look at how indexOf is implemented in Runtime_StringIndexOf. Perhaps using Runtime::StringMatch with a one character pattern might work. Runtime::StringMatch is used for simple string search in the RegExp implementation. http://codereview.chromium.org/19537/diff/1/3#newcode6800 Line 6800: set_line_ends(FixedArray::cast(line_ends()->AddKeysFromJSArray(*array))); When exposing this to JavaScript you will probably end up storing the JSArray instead of the FixedArray, but you can wait with that change until the next changelist. http://codereview.chromium.org/19537/diff/1/3#newcode6813 Line 6813: for (int i = 1; i < line_ends()->length(); ++i) { Issue 213 (http://code.google.com/p/v8/issues/detail?id=213) suggests we change this to binary search. You can look at that in a separate changelist. http://codereview.chromium.org/19537/diff/1/5 File src/objects.h (right): http://codereview.chromium.org/19537/diff/1/5#newcode2541 Line 2541: DECL_ACCESSORS(line_ends, FixedArray) FixedArray -> Object http://codereview.chromium.org/19537 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
