Lgtm, with more comments.
http://codereview.chromium.org/463040/diff/5/1006 File src/messages.js (right): http://codereview.chromium.org/463040/diff/5/1006#newcode162 src/messages.js:162: value_and_getter_setter: "Invalid property. 'value' present on property with getter or setter", I am very strongly against giving lower quality error messages to match JSC. It's up to you to decide what to do but you should at least consider giving a better message and rebaselining the test. http://codereview.chromium.org/463040/diff/5/1005 File src/v8natives.js (right): http://codereview.chromium.org/463040/diff/5/1005#newcode312 src/v8natives.js:312: throw MakeTypeError(property_desc_object, [obj]); *Ahem* http://codereview.chromium.org/463040/diff/5/1005#newcode316 src/v8natives.js:316: var enumerable = obj.enumerable; How is this not premature optimization? http://codereview.chromium.org/463040/diff/5/1005#newcode360 src/v8natives.js:360: this.value_ = void 0; > These property descriptors are never exposed to the user. The spec does not > give any details on how they should be implemented. The FromPropertyDescriptor > method in 8.10.4 makes a new object with a well-defined structure which is > passed to the user. Okay, ignore my comment then. http://codereview.chromium.org/463040/diff/9/11#newcode327 src/v8natives.js:327: if (value !== void 0 || value in obj) desc.set_value(value); IS_UNDEFINED http://codereview.chromium.org/463040/diff/9/11#newcode344 src/v8natives.js:344: if (set !== void 0 && typeof(set) !== 'function') { IS_UNDEFINED http://codereview.chromium.org/463040/diff/9/11#newcode367 src/v8natives.js:367: this.get_set_ = false; Calling a boolean xxxSet is not a particularly obvious choice and in this case, where xxx is 'get' and 'set', it is really confusing. Consider hasGetter, hasSetter instead. http://codereview.chromium.org/463040/diff/9/10 File test/mjsunit/object-create.js (right): http://codereview.chromium.org/463040/diff/9/10#newcode43 test/mjsunit/object-create.js:43: var foo_proto = { foo: function() { ctr++; }}; lowerCamelCase. http://codereview.chromium.org/463040 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
