Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support ES2019 Array.prototype.flatMap #1372

Merged
merged 4 commits into from
Aug 18, 2023
Merged

Conversation

midgleyc
Copy link
Contributor

Closes #949.

I followed what I did in #1313 but for flatMap this time.

@rbri
Copy link
Collaborator

rbri commented Aug 14, 2023

please run spotlessApply to have the code in the correct layout

@rbri
Copy link
Collaborator

rbri commented Aug 14, 2023

Thanks for the PR; do you see any chance to get array-like-objects.js also passing - i think i have seen some code already dealing with array-like objects....

@midgleyc
Copy link
Contributor Author

midgleyc commented Aug 14, 2023

Array-like objects already work to some degree. There are four total test files:

array-like-objects.js
array-like-objects-nested.js
array-like-objects-typedarrays.js
array-like-objects-poisoned-length.js

Of those, the base and poisoned length fail, and the others pass. I'll see if I can spot why.

Poisoned length fails due to not supporting [Symbol.toPrimitive]. The base fails due to not supporting a property named [10000]. Looks like the [X] syntax in general. Probably #913.

@rbri
Copy link
Collaborator

rbri commented Aug 14, 2023

@midgleyc build failed

org.mozilla.javascript.tests.MozillaSuiteTest > runMozillaTest[6550, js=testsrc/tests/js1_5/Regress/regress-89443.js, opt=9] STANDARD_ERROR
java.lang.StackOverflowError
at org.mozilla.javascript.optimizer.Optimizer.rewriteForNumberVariables(Optimizer.java)
at org.mozilla.javascript.optimizer.Optimizer.rewriteAsObjectChildren(Optimizer.java:409)
at org.mozilla.javascript.optimizer.Optimizer.rewriteForNumberVariables(Optimizer.java:393)
at org.mozilla.javascript.optimizer.Optimizer.rewriteAsObjectChildren(Optimizer.java:409)
at org.mozilla.javascript.optimizer.Optimizer.rewriteForNumberVariables(Optimizer.java:399)
at org.mozilla.javascript.optimizer.Optimizer.rewriteAsObjectChildren(Optimizer.java:409)
at org.mozilla.javascript.optimizer.Optimizer.rewriteForNumberVariables(Optimizer.java:399)
at org.mozilla.javascript.optimizer.Optimizer.rewriteAsObjectChildren(Optimizer.java:409)
at org.mozilla.javascript.optimizer.Optimizer.rewriteForNumberVariables(Optimizer.java:399)
at org.mozilla.javascript.optimizer.Optimizer.rewriteAsObjectChildren(Optimizer.java:409)
at org.mozilla.javascript.optimizer.Optimizer.rewriteForNumberVariables(Optimizer.java:399)
at org.mozilla.javascript.optimizer.Optimizer.rewriteAsObjectChildren(Optimizer.java:409)
at org.mozilla.javascript.optimizer.Optimizer.rewriteForNumberVariables(Optimizer.java:399)
at org.mozilla.javascript.optimizer.Optimizer.rewriteAsObjectChildren(Optimizer.java:409)
at org.mozilla.javascript.optimizer.Optimizer.rewriteForNumberVariables(Optimizer.java:399)
at org.mozilla.javascript.optimizer.Optimizer.rewriteAsObjectChildren(Optimizer.java:409)
at org.mozilla.javascript.optimizer.Optimizer.rewriteForNumberVariables(Optimizer.java:399)
at org.mozilla.javascript.optimizer.Optimizer.rewriteAsObjectChildren(Optimizer.java:409)

@midgleyc
Copy link
Contributor Author

Two tests fail: regress-89443 and regress-98901, both with Stack Overflows. Unfortunately, when I run them alone (in Java 17) they both pass. Any idea why they might be failing on GitHub?

@midgleyc
Copy link
Contributor Author

I rebased and the issue went away.

Copy link
Contributor

@tuchida tuchida left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Others look good. Thanks!

Comment on lines 448 to 449
case Id_flat_map:
return js_flat_map(cx, scope, thisObj, args);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case Id_flat_map:
return js_flat_map(cx, scope, thisObj, args);
case Id_flatMap:
return js_flatMap(cx, scope, thisObj, args);

I want the case to be the same as other method names.

case Id_lastIndexOf:
return js_lastIndexOf(cx, scope, thisObj, args);

@gbrail
Copy link
Collaborator

gbrail commented Aug 18, 2023

Looks good. Thanks for working on this!

@gbrail gbrail merged commit 97c51f5 into mozilla:master Aug 18, 2023
@midgleyc midgleyc deleted the array-flat-map branch August 19, 2023 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support ES2019 Array.prototype.flatMap
4 participants