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).
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.
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.
