-
Notifications
You must be signed in to change notification settings - Fork 718
Add async generators support via AST transformation #1657
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
base: static_h
Are you sure you want to change the base?
Conversation
This PR adds support for the `for await` loop through Intermediate Representation (IR). **Testing:** - Using the Test262 suite. Currently, `for-await-of` and async generators implementations achieve approximately a 98.46% success rate on `for-await-of` tests. This result is based on 1232 total tests - 20+ hand-made tests involving different scenarios were all passing - By manually reviewing the failing tests in the suite, many issues are related to missing features or other failing tests (e.g., arrow functions, non-async generators not working in all cases). Currently, Hermes is not 100% compliant and some functionalities do not pass Test262 test cases. However, most features are available and functioning correctly. ---
This PR introduces support for async generators through AST transformations, aligning with the design and rules of ES6 classes transformations. Key changes: - Added an `AsyncGenerator` class that converts any async generator to a normal generator. This follows the same logic implemented by [Babel](https://babeljs.io/docs/babel-plugin-transform-async-generator-functions). - Added helper functions as part of the internal bytecode. Tested by executing multiple scripts involving: - nested async/generator functions, - `for await...of` loop with and without generators - `for await...of` loop with array of promises **Test262 results:** 98.46%. ---
lib/Sema/SemResolve.cpp
Outdated
| ESTree::ProgramNode *root, | ||
| const DeclarationFileListTy &ambientDecls) { | ||
| if (astContext.getEnableAsyncGenerators()) { | ||
| transformAsyncGenerators(astContext, root); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to add AST transformation in lazy mode? I tried by running transformAsyncGenerators in resolveASTLazy, but it does not let me change the tree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't really started to review this PR yet (working on getting for-await-of committed right now) but I've put up a PR for a new compiler stage that should work with lazy and eager compilation. It does the transformation in a single place so that things can be kept track of in a principled manner.
Can you try it out and see if it works for your use case here? You just need to fill out TransformAST.cpp so that it returns a node of the same type as was passed in (it can be the same node). If it works, I'll commit it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works, but not In lazy compilation:
Assertion failed: (saveNode == node && "node must not be replaced in-place with a no-replace visitor"), function visitESTreeNodeNoReplace, file RecursiveVisitor.h, line 568.
Edit: I see, I need to use visitESTreeNode as in lazy mode it may replace the root. It still fails but it's not related to your PR, so you can go on and merge it 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I disabled the test in lazy compilation for now.
I am not sure of the issue, may it be related to transformASTForCompilation being called twice just in lazy compilation? 🤔
The AST transformation itself seems correct to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
called twice just in lazy compilation
If you're visiting the lazy function (empty body block) during the first call to the AST transformation then you will visit it twice - once with an empty body and once with a real (parsed) body. The transformation likely needs to account for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed and enabled unit tests in lazy compilation, thanks!
Summary: This is a compiler stage that is invoked from every place we compile JS, allowing arbitrary AST transformations between parsing and semantic resolution. Differential Revision: D72414034
Summary: This PR introduces support for async generators through AST transformations. In detail: - Added AsyncGenerator.cpp to transform asynchronous generators into standard generators using AST transformations. This implementation mirrors the logic found in [Babel ](https://babeljs.io/docs/babel-plugin-transform-async-generator-functions). - Introduced an EnableAsyncGenerators flag to manage this functionality. Pull Request resolved: #1657 Test Plan: - Added unit tests Reviewed By: tmikov Differential Revision: D78378721 Pulled By: avp fbshipit-source-id: cb67810823e2a1478b562ad3f12c5a5dfbf3f754
Summary
This PR introduces support for async generators through AST transformations. In detail:
Test Plan