LGTM, are there some chromium guys that needs to change some bindings code?


http://codereview.chromium.org/7324027/diff/6001/src/objects.cc
File src/objects.cc (right):

http://codereview.chromium.org/7324027/diff/6001/src/objects.cc#newcode2439
src/objects.cc:2439: // will silently fail).
I can't see where we actually silently fail in this method, and it is
not immediately clear which of the method calls would do that.

http://codereview.chromium.org/7324027/diff/6001/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/7324027/diff/6001/src/runtime.cc#newcode1966
src/runtime.cc:1966: CONVERT_CHECKED(JSFunction, object, args[0]);
it's a function, maybe call it fun, function or f instead of object

http://codereview.chromium.org/7324027/diff/6001/src/runtime.cc#newcode1973
src/runtime.cc:1973: if (object->HasFastProperties()) {
If it is not performance critical to keep fast properties consider just
converting to dictionary mode instead of the descriptor array juggling

http://codereview.chromium.org/7324027/diff/6001/src/runtime.cc#newcode1976
src/runtime.cc:1976: int index = instance_desc->Search(name);
assert that index != kNotFound

http://codereview.chromium.org/7324027/diff/6001/src/runtime.cc#newcode2004
src/runtime.cc:2004: int entry =
object->property_dictionary()->FindEntry(name);
assert that entry != kNotFound

http://codereview.chromium.org/7324027/diff/6001/src/runtime.cc#newcode2008
src/runtime.cc:2008: details.type(), details.index());
nit, put last argument on seperate line

http://codereview.chromium.org/7324027/diff/6001/src/runtime.h
File src/runtime.h (right):

http://codereview.chromium.org/7324027/diff/6001/src/runtime.h#newcode213
src/runtime.h:213: F(FunctionReadOnlyPrototype, 1, 1) \
maybe rename to FunctionSetReadOnlyPrototype

http://codereview.chromium.org/7324027/diff/6001/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/7324027/diff/6001/test/cctest/test-api.cc#newcode6997
test/cctest/test-api.cc:6997: CHECK_EQ(42,
CompileRun("func2.prototype.x")->Int32Value());
We could also add a check that when we actually write to the prototype
it is not allowed (i.e., to prototype remains the same)

http://codereview.chromium.org/7324027/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to