Comments addressed.
https://codereview.chromium.org/622523004/diff/20001/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):
https://codereview.chromium.org/622523004/diff/20001/src/ia32/full-codegen-ia32.cc#newcode2750
src/ia32/full-codegen-ia32.cc:2750: // - this (receiver) <--
LoadFromSuper will pop here and below.
On 2014/10/02 09:32:05, Igor Sheludko wrote:
LoadKeyedFromSuper in the comment
Done in all ports
https://codereview.chromium.org/622523004/diff/20001/src/runtime/runtime-classes.cc
File src/runtime/runtime-classes.cc (right):
https://codereview.chromium.org/622523004/diff/20001/src/runtime/runtime-classes.cc#newcode98
src/runtime/runtime-classes.cc:98: if (!Runtime::ToName(isolate,
key).ToHandle(&name)) {
On 2014/10/02 09:32:05, Igor Sheludko wrote:
I think this should look like:
ASSIGN_RETURN_ON_EXCEPTION(isolate, name, Runtime::ToName(isolate,
key),
Object);
to correctly handle propagate potential exception from toString() and
not
replace it with UnsupportedSuper.
Exception is propagated correctly (there is a test for that!), but what
you suggest follows the pattern in other places - done.
https://codereview.chromium.org/622523004/diff/20001/src/runtime/runtime-classes.cc#newcode102
src/runtime/runtime-classes.cc:102: if (name->AsArrayIndex(&index)) {
On 2014/10/02 09:42:01, Igor Sheludko wrote:
On 2014/10/02 09:32:05, Igor Sheludko wrote:
> And this case should be handled before Runtime::ToName(). Check the
other
> ToName() usages.
Indeed, I mixed up ToArrayIndex() and AsArrayIndex(). So, ignore this
comment.
Yup, ToArrayIndex in other places is a fast path that we do not handle
(yet)
https://codereview.chromium.org/622523004/diff/20001/src/runtime/runtime.cc
File src/runtime/runtime.cc (right):
https://codereview.chromium.org/622523004/diff/20001/src/runtime/runtime.cc#newcode466
src/runtime/runtime.cc:466: MUST_USE_RESULT
On 2014/10/02 09:32:06, Igor Sheludko wrote:
Remove MUST_USE_RESULT as it is already in the .h file.
Done.
https://codereview.chromium.org/622523004/diff/40001/test/mjsunit/harmony/super.js
File test/mjsunit/harmony/super.js (right):
https://codereview.chromium.org/622523004/diff/40001/test/mjsunit/harmony/super.js#newcode168
test/mjsunit/harmony/super.js:168: super[1]; // Indexed properties
unsupported yet.
On 2014/10/02 09:32:06, Igor Sheludko wrote:
This case seems to be checked in TestUnsupportedCases().
Will keep it here as well if you don't mind.
https://codereview.chromium.org/622523004/diff/40001/test/mjsunit/harmony/super.js#newcode177
test/mjsunit/harmony/super.js:177: derived.testGetterWithToString();
On 2014/10/02 09:32:06, Igor Sheludko wrote:
And please add a test for the toString() returning "1".
Done.
https://codereview.chromium.org/622523004/diff/40001/test/mjsunit/harmony/super.js#newcode638
test/mjsunit/harmony/super.js:638:
assertThrows(function(){f2.toMethod(o)(15);}, ReferenceError);
On 2014/10/02 09:32:06, Igor Sheludko wrote:
f1?
Done.
https://codereview.chromium.org/622523004/
--
--
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.
