On 2014/02/02 20:08:59, arv wrote:
On 2014/02/02 19:59:56, sof wrote:
>
https://codereview.chromium.org/118553003/diff/170001/test/mjsunit/harmony/symbols.js
> File test/mjsunit/harmony/symbols.js (right):
>
>
https://codereview.chromium.org/118553003/diff/170001/test/mjsunit/harmony/symbols.js#newcode172
> test/mjsunit/harmony/symbols.js:172: assertEquals(Object(symbols[i]),
> Object(symbols[i]))
> On 2014/02/02 19:49:35, arv wrote:
> > On 2014/02/01 13:58:10, sof wrote:
> > > On 2014/02/01 01:33:10, arv wrote:
> > > > This looks strange. If symbol[i] is a symbol (not a symbol
wrapper)
then
> > this
> > > > should be false. Are all elements in symbols wrappers? It does not
seem
> like
> > > > that is the case on line 49.
> > >
> > > They're all Symbol values. This is an equals check, not same, so why
should
> > > deepObjectEquals() return false? (Perhaps not a very interesting to
test
to
> > add,
> > > I can remove.)
> >
> > If all elements in symbols are symbol values (not symbol wrapper
objects)
this
> > should be assertNotEquals since a new wrapper is always created when
you
do
> > Object(value).
>
> I know; the object also doesn't have any "keys" in the (Object.keys()
sense),
so
> I don't think it much of a difference how you want to phrase that
equality
check
> for these wrapper objects. Had assertNotEquals existed, that is.
>
> The offer to remove the test still stands; I don't think there is
anything
> fundamentally wrong here in the interpretation of symbol values vs
wrappers.
I see, assertEquals is just broken. For other wrappers it unwraps which is
clearly wrong. It also trats two completely different object as equal.
Yikes.
Can you change the test to?
assertTrue(Object(symbol[i]) !== Object(symbol[i]));
or add assertNotSame and use that?
I added a (!==) variant (but kept the assertEquals() version..will tidy
that up
tomorrow.)
I don't like to leave stuff in a broken state, so I added some autoboxing of
Symbol value receivers to the x64 and ia32 code generators, to have this CL
work
again wrt HEAD. Along with FIXMEs; I hope someone can give me a nudge in a
better direction on how to go about this more properly.
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.
