Array.prototype.push should always perform [[Set]] in strict mode
Created attachment 407877 [details] Patch
(In reply to Alexey Shvayka from comment #1) > Created attachment 407877 [details] > Patch Warmed-up runs, --outer 50: r266507 patch new-array-push 6.5605+-0.4641 6.2634+-0.4522 might be 1.0474x faster array-push-0 350.3329+-2.1429 348.3786+-1.2194 array-push-1 733.3222+-13.1726 ? 739.4494+-13.0772 ? might be 1.0084x slower array-push-2 822.0986+-4.4731 ? 822.2864+-3.1685 ? array-push-3 836.1861+-24.9409 832.5322+-23.3867 might be 1.0044x faster --- I've vetted all puts via PutPropertySlot in JSC: other usages set strict mode correctly.
Do you understand why the JSC tests are failing on EWS?
(In reply to Darin Adler from comment #3) > Do you understand why the JSC tests are failing on EWS? Igalia's servers seem to be down. I don't think it is related.
EWS is all-green now. Thank you for review!
Committed r266581: <https://trac.webkit.org/changeset/266581> All reviewed patches have been landed. Closing bug and clearing flags on attachment 407877 [details].
<rdar://problem/68330312>
Comment on attachment 407877 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407877&action=review > Source/JavaScriptCore/runtime/ArrayPrototype.cpp:919 > + thisObj->putByIndexInline(globalObject, static_cast<uint64_t>(length + n), callFrame->uncheckedArgument(n), true); Sorry I didn’t notice this when reviewing. Can we remove this unnecessary static_cast? It caused me concern just now: I was worried that the addition was incorrectly performed in 32 bits with possible overflow before promoting the result to 64 bits. But it’s not, because length is already uint64_t, and for the same reason the static_cast is not needed or helpful.
Committed r266641: <https://trac.webkit.org/changeset/266641>
(In reply to Darin Adler from comment #8) > Can we remove this unnecessary static_cast? It caused me concern just now: I > was worried that the addition was incorrectly performed in 32 bits with > possible overflow before promoting the result to 64 bits. But it’s not, > because length is already uint64_t, and for the same reason the static_cast > is not needed or helpful. Thank you for the follow-up! I've also checked another casts in ArrayPrototype.cpp, all seem necessary.