lgtm
https://codereview.chromium.org/1142393003/diff/20001/src/messages.h
File src/messages.h (right):
https://codereview.chromium.org/1142393003/diff/20001/src/messages.h#newcode231
src/messages.h:231: "Cannot redefine non-configurable property '%' of
strong object % to be " \
Nit: sylise
https://codereview.chromium.org/1142393003/diff/20001/src/v8natives.js
File src/v8natives.js (right):
https://codereview.chromium.org/1142393003/diff/20001/src/v8natives.js#newcode810
src/v8natives.js:810: if (!currentIsWritable) {
Nit: if (!currentIsWritable || IS_STRONG(obj)) here
https://codereview.chromium.org/1142393003/diff/20001/src/v8natives.js#newcode812
src/v8natives.js:812: throw MakeTypeError(kRedefineDisallowed, p);
...and then throw currentIsWritable ? MakeTypeError(...) :
MakeTypeError(...) here
https://codereview.chromium.org/1142393003/diff/20001/src/v8natives.js#newcode817
src/v8natives.js:817: if (IS_STRONG(obj)) {
...and eliminate this duplication
https://codereview.chromium.org/1142393003/diff/20001/test/mjsunit/strong/object-freeze-property.js
File test/mjsunit/strong/object-freeze-property.js (right):
https://codereview.chromium.org/1142393003/diff/20001/test/mjsunit/strong/object-freeze-property.js#newcode21
test/mjsunit/strong/object-freeze-property.js:21: // Strong functions
can't have properties added to them.
Nit: indentation
https://codereview.chromium.org/1142393003/diff/20001/test/mjsunit/strong/object-freeze-property.js#newcode30
test/mjsunit/strong/object-freeze-property.js:30: let nonStrongObjects =
sloppyObjects.concat(strictObjects);
Nit: weakObjects
https://codereview.chromium.org/1142393003/diff/20001/test/mjsunit/strong/object-freeze-property.js#newcode33
test/mjsunit/strong/object-freeze-property.js:33:
Object.defineProperty(o, "foo", { enumerable:true, writable:true });
Nit: why care about enumerable in all these tests?
https://codereview.chromium.org/1142393003/diff/20001/test/mjsunit/strong/object-freeze-property.js#newcode41
test/mjsunit/strong/object-freeze-property.js:41: let defProp =
function(o) {
Nit: function defProp(o) { (same below)
https://codereview.chromium.org/1142393003/diff/20001/test/mjsunit/strong/object-freeze-property.js#newcode51
test/mjsunit/strong/object-freeze-property.js:51: for (let func of
[defProp, defProps, freeze]) {
Nit: you can just use OBject.freeze here directly
https://codereview.chromium.org/1142393003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
