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 generator methods in object literals, e.g. o = {*g() {...}} #1747

Merged

Conversation

andreabergia
Copy link
Contributor

@andreabergia andreabergia commented Dec 4, 2024

This makes quite a few test262 cases pass.

There's a few tests that were passing and apparently are broken now, but that is a false positive. The reason is that, before this PR, Rhino would not support the syntax altogether and thus would complain with a SyntaxError, whereas now we accept the method definition, and also accept some syntax that we shouldn't have inside the method body. For example one of the test cases, method-definition/generator-param-redecl-let.js has this body:

var obj = {
    *foo(a) {
        let a = 3;
    }
};

Previously rhino would have thrown an error because *foo was not supported. After this PR, the method declaration is allowed and the test is passing, but rhino should throw a SyntaxError on let a = 3. However, the same test exists for non-generator methods, named method-definition/name-param-redecl-let.js, which was already failing because of the same underlying reason.
I've checked that this is true for all the test262 cases where there is a regression.

Closes #1413

@rbri
Copy link
Collaborator

rbri commented Dec 4, 2024

😎

@gbrail
Copy link
Collaborator

gbrail commented Dec 6, 2024

This is also promising! I'm in "everyone have meetings this week so we can be on vacation" mode at work so it'll be a few days but I will look at and test all this!

Copy link
Collaborator

@gbrail gbrail left a comment

Choose a reason for hiding this comment

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

This looks good to me. Can you possibly rebase it? Thanks!

@andreabergia andreabergia force-pushed the generator-shorthand-method-syntax branch from 6d7b642 to 8dd633f Compare December 10, 2024 07:52
@andreabergia
Copy link
Contributor Author

Rebased

@gbrail gbrail merged commit 2fcc422 into mozilla:master Dec 11, 2024
3 checks passed
@andreabergia andreabergia deleted the generator-shorthand-method-syntax branch December 11, 2024 07:36
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.

ES2015 support shorthand generator method syntax
3 participants