Handle super.x++ and similar
#1738
Conversation
andreabergia
force-pushed
the
super-inc-dec
branch
from
37c3998 to
9b6bb0b
Compare
andreabergia
force-pushed
the
super-inc-dec
branch
from
9b6bb0b to
0b147b9
Compare
Co-authored-by: Satish Srinivasan <[email protected]>
Co-authored-by: Satish Srinivasan <[email protected]>
andreabergia
force-pushed
the
super-inc-dec
branch
from
0b147b9 to
2a6683f
Compare
There was a problem hiding this comment.
This is fine but I'm not sure that you need to unwrap and re-wrap that value, rather than using the methods we already have that can operate directly.
FWIW, I have considered adding "MATH:INCREMENT" and "MATH:DECREMENT" INDY instructions, and getting rid of all the "elemIncrDecr" and "propIncrDecr" stuff, which I think is overcomplicated and doesn't help performance. If I do that it would clean this up too.
| } | ||
|
|
||
| // Increment or decrement the value | ||
| addObjectToDouble(); // Unbox |
There was a problem hiding this comment.
I'm not sure that this is more efficient than leaving the object as it was, and adding a "MATH:ADD" instruction, or a call to ScriptRuntime.subtract, instead.
There was a problem hiding this comment.
We've copy&pasted the code from the pre-existing visitIncDec. If we want to change the approach, it would be better to do it in both places (so, IMHO in a different PR)
There was a problem hiding this comment.
I did some experimenting with increment stuff last week and it's both not simple to completely change this stuff, and not faster to do it the way that I suggested. So I think that the way you have it here is fine. Thanks!
See spec 13.5.1.2 at https://tc39.es/ecma262/#sec-delete-operator-runtime-semantics-evaluation Co-authored-by: Satish Srinivasan <[email protected]>
Co-authored-by: Andrea Bergia <[email protected]>
Co-authored-by: Satish Srinivasan <[email protected]>
This reverts commit b845d14. Co-authored-by: Satish Srinivasan <[email protected]>
Co-authored-by: Satish Srinivasan <[email protected]>
andreabergia
force-pushed
the
super-inc-dec
branch
from
2a6683f to
9011ce0
Compare
|
Merged onto #1735 |
This PR, co-authored with @0xe, adds support for
super.x++and similar. It is stacked on top of #1735.Rhino has special opcodes (and similar in compiled classes) for
a.b++, probably for historical reasons. Given the number of bytecodes already added by #1735, we have decided to implement this one differently - we just replacesuper.x++withsuper.x = super.x + 1in the bytecode.