Hmmmm... I'm really confused about this change. I only looked at the diffs
to
the test cases so far, but many of them don't seem to match what I read in
the
(new) spec.
I also don't understand why we need to handle the symbol wrapper object
different than before, and different from other wrappers. (If the spec
really
makes that necessary then I think the spec should be changed.)
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js
File test/mjsunit/harmony/symbols.js (right):
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode75
test/mjsunit/harmony/symbols.js:75: assertSame(Function.prototype,
Symbol.prototype)
That seems wrong. Symbol.prototype is an ordinary object.
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode78
test/mjsunit/harmony/symbols.js:78: assertSame(symbolPrototype,
Object(Symbol()).__proto__)
Is this different from Symbol.prototype? Why?
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode79
test/mjsunit/harmony/symbols.js:79: assertSame(Symbol.prototype,
Symbol.__proto__)
Shouldn't Symbol.__proto__ be Function.prototype?
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode81
test/mjsunit/harmony/symbols.js:81: assertSame(Function.prototype,
(Symbol()).__proto__)
And Symbol().__proto__ should be Symbol.prototype
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode82
test/mjsunit/harmony/symbols.js:82: assertTrue(typeof
((Symbol()).prototype) === "undefined")
Not sure why this test is necessary.
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode91
test/mjsunit/harmony/symbols.js:91: assertFalse(Object ===
Symbol.prototype.constructor)
This should be true now.
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode93
test/mjsunit/harmony/symbols.js:93: assertSame(Function,
Symbol.prototype.constructor)
This should be Object, not Function
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode94
test/mjsunit/harmony/symbols.js:94: assertSame(Function,
Symbol().__proto__.constructor)
Dito
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode95
test/mjsunit/harmony/symbols.js:95: assertSame(Function,
Object(Symbol()).valueOf().__proto__.constructor)
Dito here. And why the redundant valueOf()?
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode96
test/mjsunit/harmony/symbols.js:96: assertSame(Function,
(Symbol).__proto__.constructor)
This is an unrelated test, since it tests the constructor of the
constructor.
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode97
test/mjsunit/harmony/symbols.js:97: assertSame(Function,
(Symbol()).__proto__.constructor)
This is repeating line 94
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode98
test/mjsunit/harmony/symbols.js:98: assertSame(Symbol,
(Object(Symbol())).__proto__.constructor)
Nit: redundant parens (here and above)
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode101
test/mjsunit/harmony/symbols.js:101: assertSame(Function,
symbols[i].__proto__.constructor)
Should also be Symbol
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode109
test/mjsunit/harmony/symbols.js:109: var value =
Object(symbols[i]).valueOf()
Why the redundant Object()?
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode119
test/mjsunit/harmony/symbols.js:119: assertDoesNotThrow(function () {
stringified = String(Object(symbols[i])); })
Nit: line length (just remove the space after 'function')
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode125
test/mjsunit/harmony/symbols.js:125: assertDoesNotThrow(function() {
Object(symbols[i]).toString() })
Why not check the expected result?
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode166
test/mjsunit/harmony/symbols.js:166: assertTrue(symbols[i] ===
Object(symbols[i]).valueOf())
The original test was to ensure that symbols are distinct from their
wrappers,
so this should be
assertFalse(symbols[i] === Object(symbols[i]))
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode167
test/mjsunit/harmony/symbols.js:167:
assertTrue(Object(symbols[i]).valueOf() === symbols[i])
Likewise
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode170
test/mjsunit/harmony/symbols.js:170: assertEquals(Object(symbols[i]),
Object(symbols[i]))
This is surprising. Why are the two objects equal?
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode175
test/mjsunit/harmony/symbols.js:175: assertFalse(sym1 ==
Object(symbols[i]))
That seems to contradict line 170 -- I'm really confused now.
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode205
test/mjsunit/harmony/symbols.js:205: assertEquals(symbols[i],
Object(symbols[i]).valueOf())
Why is Object() needed?
https://codereview.chromium.org/118553003/diff/1/test/mjsunit/harmony/symbols.js#newcode220
test/mjsunit/harmony/symbols.js:220: assertEquals(symbols[i],
Object(symbols[i]).valueOf())
Dito
https://codereview.chromium.org/118553003/
--
--
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/groups/opt_out.
