Thanks, Mads. Made the changes. martin
http://codereview.chromium.org/6694044/diff/1023/src/bootstrapper.cc File src/bootstrapper.cc (right): http://codereview.chromium.org/6694044/diff/1023/src/bootstrapper.cc#newcode510 src/bootstrapper.cc:510: Handle<DescriptorArray> Genesis::ComputeStrictFunctionDescriptor( On 2011/03/16 09:29:13, Mads Ager wrote:
Keep the 'Instance' in the name?
ComputeStrictFunctionInstanceDescriptor?
Can the structure of this be closer to
ComputeFunctionInstanceDescriptor using
CopyAddDescriptor? (Or the other way around, use this structure for
the other
one.) Would be nice to use the same structure for doing the same
thing. Done. Used what I think is cleaner code which directly sets the descriptors rather than iteratively append them, copying the descriptor array each time. http://codereview.chromium.org/6694044/diff/1023/src/bootstrapper.cc#newcode560 src/bootstrapper.cc:560: Handle<JSFunction> pill = Factory::NewFunctionWithoutPrototypeStrict(name); On 2011/03/16 09:29:13, Mads Ager wrote:
This means that you will get two ThrowTypeError functions doesn't it?
I think
that the spec says that there should be only one?
If that is the case, rename this to CreateThrowTypeErrorFunction and
have it
return the pill. Then set it as the get and set for both of the
callback arrays? Yes. The reason I am doing it this way is to provide better error message. Experiments with jsc showed that it creates new instance of poison pill per strict mode function so I felt it is better choice to give better error message at the expense of having 4 instances of poison pills in the whole system (rather than unbounded number which jsc appears to allocate). Also, renamed to CreateThrowTypeErrorFunction. Thanks for the suggestion! http://codereview.chromium.org/6694044/diff/1023/src/bootstrapper.cc#newcode576 src/bootstrapper.cc:576: void Genesis::CreateThrowTypeError(Handle<JSFunction> empty) { On 2011/03/16 09:29:13, Mads Ager wrote:
This method seems to have a strange name. It is really a mix of
allocating
strict mode function maps and setting up two accessors. Rename to CreateStrictFunctionsMaps or something along those lines?
Done. http://codereview.chromium.org/6694044/diff/1023/src/bootstrapper.cc#newcode1943 src/bootstrapper.cc:1943: // Extract arguments and caller from the original strict function map. On 2011/03/16 09:29:13, Mads Ager wrote:
It seems a bit complicated to have to extract the arguments and caller
arrays
here, but I don't really have a good suggestion.
On a second look ... there is an option which I find cleanest so far. The Genesis object can maintain some state. empty_function_ (unused) was an example. The writable prototype maps are created alongside the rest of the maps, stored away in the Genesis object state and here simply installed, no computation necessary. I am also deleting the unused empty_function_. http://codereview.chromium.org/6694044/diff/1023/src/bootstrapper.cc#newcode1956 src/bootstrapper.cc:1956: Handle<Map> map = Factory::NewMap(JS_FUNCTION_TYPE, JSFunction::kSize); On 2011/03/16 09:29:13, Mads Ager wrote:
Is there a reason why you do not use CopyMapDropDescriptors here as we
do for
the normal function map and avoid the set_prototype below?
:) I confess it is perhaps reflection of my continued learning of the codebase (and general fear of functions that appear to 'do a whole lot'). I was trying to use simpler building blocks which I understood well at the time. http://codereview.chromium.org/6694044/diff/1023/test/mjsunit/strict-mode.js File test/mjsunit/strict-mode.js (right): http://codereview.chromium.org/6694044/diff/1023/test/mjsunit/strict-mode.js#newcode1025 test/mjsunit/strict-mode.js:1025: (function KrakenTest() { On 2011/03/16 09:29:13, Mads Ager wrote:
Kraken -> WritablePrototype?
Done. (TestStrictFunctionWritablePrototype) http://codereview.chromium.org/6694044/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
