The original code is broken. Instead of making it worse (uglier, hackier,
more
brittle), let's fix it properly. Here's an example demonstrating failure:
var a = new Uint8Array(4);
Object.defineProperty(a, "length", {get: function() { return "blah"; }});
function getlength(x) { return x.length; }
print(getlength(a)); // correct
print(getlength(a)); // correct
print(getlength(a)); // wrong
%OptimizeFunctionOnNextCall(getlength);
print(getlength(a)); // wrong
I think what we need to do is the following:
- check if the name is one of the expected names
- perform a lookup of the corresponding property on the given map (using
LookupIterator)
- check if the holder found by the LookupIterator is identical to the
original
prototype
That should fix the existing implementation for all corner cases while
adding
support for arbitrary-length prototype chains and hiding low-level
implementation details as far as possible.
https://codereview.chromium.org/1313493005/diff/20001/src/accessors.cc
File src/accessors.cc (right):
https://codereview.chromium.org/1313493005/diff/20001/src/accessors.cc#newcode105
src/accessors.cc:105: if (Name::Equals(name,
isolate->factory()->length_string())) {
Let's keep using the existing CheckForName helper:
if (!CheckForName(...) && !CheckForName(...) && !...) { return false; }
https://codereview.chromium.org/1313493005/diff/20001/src/accessors.cc#newcode119
src/accessors.cc:119: // %TypedArray%.prototype is non-configurable, and
so are the following
outdated comment (there are no "following named properties" any more);
also it correctly describes an incorrect implementation
https://codereview.chromium.org/1313493005/diff/20001/src/accessors.cc#newcode133
src/accessors.cc:133: // constructor
nit: trailing full stop
https://codereview.chromium.org/1313493005/diff/20001/src/accessors.cc#newcode136
src/accessors.cc:136: // If the middle-prototype does not override the
fast property -
s/ -/, then/
s/it's/its/
s/offset/offset./
https://codereview.chromium.org/1313493005/diff/20001/src/accessors.cc#newcode139
src/accessors.cc:139:
JSReceiver::GetOwnPropertyAttributes(Handle<JSObject>(js_proto),
This should use JSReceiver::HasOwnProperty(), but see overall comment
https://codereview.chromium.org/1313493005/
--
--
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.
