On 2013/02/26 16:19:30, adamk wrote:
On 2013/02/26 12:01:45, rossberg wrote:
> On 2013/02/25 19:36:03, adamk wrote:
> > https://codereview.chromium.org/12328064/diff/1/src/objects.cc
> > File src/objects.cc (left):
> >
> >
https://codereview.chromium.org/12328064/diff/1/src/objects.cc#oldcode3373
> > src/objects.cc:3373: // Handle [] on String objects.
> > On 2013/02/25 10:46:25, rossberg wrote:
> > > I'm not sure removing this is correct. This function is also called
by
> > > GetPropertyAttributeWithReceiver, and on that path, I don't think
strings
> > would
> > > be handled otherwise.
> >
> > I've added a test case covering this case, and it still passes. The
reason
is
> > that, although I've removed the check here, the very next thing this
code
does
> > is call GetElementAttributeWithoutInterceptor, which itself does the
> > IsStringObjectWithCharacterAt check. Please take a look at the test
case
and
> see
> > if there's other stuff that I need to cover.
>
> Hm, I'm not sure what the simplest JS test is then, but I'm pretty
certain
that
> the following API code will fail after the change:
>
> Handle<String> s = String::New("JS rocks so hard");
> ASSERT(s->Has(1));
I assume you mean StringObject here, not String? Anyway, that works fine
(I
can
upload a test case when I get in; looks like StringObject doesn't have
any API
coverage).
Yes, sorry, I managed to forget that wrapping has to be done manually
outside JS
land :). So more like:
Handle<String> s = String::New("JS rocks so hard");
Handle<StringObject> o = StringObject::New(s);
ASSERT(o->Has(1));
Perhaps there's some confusion here? The only way the code I'm removing
could
have an impact is if, somehow, you managed to have a JSValue representing
a
String that _also_ managed to have elements for the same indices as the
String
(in the same Object, not in a prototype), and those elements had different
attributes. But I don't think this is possible, since it's not possible
to add
elements to a JSValue _before_ creation, and after creation those indices
are
read-only.
I'm not sure I follow. AFAICS, JSReceiver::HasElement calls
JSObject::GetElementAttributeWithReceiver, which is where you removed the
special case for string objects. And the call to Has in the example above
should
take that path, shouldn't it? If not, I'm indeed confused, where on earth
is it
going?
https://codereview.chromium.org/12328064/
--
--
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.
