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

deps: v8 backport 9689b17687b #37865

Closed

Conversation

guybedford
Copy link
Contributor

Original commit from https://chromium-review.googlesource.com/c/v8/v8/+/2667772, fixing a TLA execution ordering spec bug:

[top-level-await] Implement spec fix for cycle root detection

There is a bug in the top-level await spec draft such that async
strongly connected components are not always evaluated before their
depending modules.

See https://github.com/tc39/proposal-top-level-await/pull/161 for full
discussion and spec fix.

Bug: v8:11376
Change-Id: I88bf06afb2e9a5d8d0b757de8276f1d1242a875e
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2667772
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72508}

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. v14.x v8 engine Issues and PRs related to the V8 dependency. labels Mar 22, 2021
@guybedford guybedford requested a review from targos March 22, 2021 16:50
@targos
Copy link
Member

targos commented Mar 22, 2021

Please add Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0 to the commit message

@targos
Copy link
Member

targos commented Mar 22, 2021

Note to whoever lands this: the embedder string should be incremented.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 22, 2021

@aduh95
Copy link
Contributor

aduh95 commented Mar 22, 2021

This is targeting v14.x. Did you mean to target v14.x-staging or master instead?

@targos
Copy link
Member

targos commented Mar 23, 2021

master already contains the commit. I guess it's okay to target v14.x. We just need to land on staging.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@guybedford
Copy link
Contributor Author

It would be great to get another approval here to land this TLA bug fix for 14.x.

@guybedford guybedford changed the base branch from v14.x to v14.x-staging March 29, 2021 21:25
@nodejs-github-bot
Copy link
Collaborator

@guybedford
Copy link
Contributor Author

Switched the branch to rerun CI on 14.x-staging. Will land tomorrow.

guybedford added a commit that referenced this pull request Mar 29, 2021
[top-level-await] Implement spec fix for cycle root detection

Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0
PR-URL: #37865
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@guybedford
Copy link
Contributor Author

Landed in fe5e2e3.

@guybedford guybedford closed this Mar 29, 2021
guybedford added a commit to guybedford/node that referenced this pull request Mar 30, 2021
[top-level-await] Implement spec fix for cycle root detection

Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0
PR-URL: nodejs#37865
Reviewed-By: Michaël Zasso <targos@protonmail.com>
ruyadorno pushed a commit that referenced this pull request Mar 31, 2021
[top-level-await] Implement spec fix for cycle root detection

Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0
PR-URL: #37865
Backport-PR-URL: #37985
Reviewed-By: Michaël Zasso <targos@protonmail.com>
MylesBorins pushed a commit that referenced this pull request Apr 6, 2021
[top-level-await] Implement spec fix for cycle root detection

Refs: http://github.com/v8/v8/commit/9689b17687b21c800c3f7400df4255c55b9c6ec0
PR-URL: #37865
Reviewed-By: Michaël Zasso <targos@protonmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-ci PRs that need a full CI run. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants