Skip to content

Conversation

@trossimel-sc
Copy link

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 .
  • Introduced an EnableAsyncGenerators flag to manage this functionality.

Test Plan

  • Added unit tests

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%.
---
@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Apr 3, 2025
ESTree::ProgramNode *root,
const DeclarationFileListTy &ambientDecls) {
if (astContext.getEnableAsyncGenerators()) {
transformAsyncGenerators(astContext, root);
Copy link
Author

@trossimel-sc trossimel-sc Apr 3, 2025

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.

Copy link
Contributor

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.

#1658

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.

Copy link
Author

@trossimel-sc trossimel-sc Apr 4, 2025

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 👍

Copy link
Author

@trossimel-sc trossimel-sc Apr 8, 2025

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

Copy link
Contributor

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.

Copy link
Author

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!

trossimel-sc and others added 8 commits April 3, 2025 11:24
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
@facebook-github-bot
Copy link
Contributor

@avp has imported this pull request. If you are a Meta employee, you can view this in D78378721.

facebook-github-bot pushed a commit that referenced this pull request Jul 29, 2025
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Do not delete this pull request or issue due to inactivity.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants