LGTM (if all comments addressed).
https://chromiumcodereview.appspot.com/9616016/diff/4001/src/runtime.cc File src/runtime.cc (left): https://chromiumcodereview.appspot.com/9616016/diff/4001/src/runtime.cc#oldcode4324 src/runtime.cc:4324: Please leave this empty line in (or at least keep layout in sync with Runtime_DefineOrRedefineDataProperty). I cleaned it up a month ago. https://chromiumcodereview.appspot.com/9616016/diff/4001/src/runtime.cc File src/runtime.cc (right): https://chromiumcodereview.appspot.com/9616016/diff/4001/src/runtime.cc#newcode4318 src/runtime.cc:4318: static bool IsValidAccessor(Object* obj) { Since this is only used as part of an assertion, I would prefer to have the condition inline instead of in a function. https://chromiumcodereview.appspot.com/9616016/diff/4001/src/runtime.cc#newcode4334 src/runtime.cc:4334: CONVERT_ARG_HANDLE_CHECKED(String, name, 1); Is there a particular reason you handlified the name (and not the getter and setter)? https://chromiumcodereview.appspot.com/9616016/diff/4001/src/runtime.cc#newcode4349 src/runtime.cc:4349: getter = FixedArray::cast(array->elements())->get(GETTER_INDEX); I assume that this goes away once the above TODO is addressed? Then I am fine with this intermediate step. https://chromiumcodereview.appspot.com/9616016/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
