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

[CP] fix precedence bug in dart2js #54697

Closed
sigmundch opened this issue Jan 22, 2024 · 3 comments
Closed

[CP] fix precedence bug in dart2js #54697

sigmundch opened this issue Jan 22, 2024 · 3 comments
Assignees
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta

Comments

@sigmundch
Copy link
Member

sigmundch commented Jan 22, 2024

Commit(s) to merge

cb16f98

Target

beta

Prepared changelist for beta/stable

https://dart-review.googlesource.com/c/sdk/+/347645

Issue Description

Dart2js can generate invalid code for the new Dart/JS interop feature.

The problem results from a precedence bug in the js_ast representation in dart2js. This bug was not exploitable from code generated from Dart, but the new constructs added by Dart/JS interop make the precedence bug highly visible.

What is the fix

Fix the precedence bug when printing js_ast's.

Why cherry-pick

May affect any user of the new Dart/JS interop feature being released as part of Dart 3.3. The issue has been identified by both by internal and external developers.

Risk

low

Issue link(s)

#54534

Extra Info

No response

@sigmundch sigmundch added the cherry-pick-review Issue that need cherry pick triage to approve label Jan 22, 2024
@sigmundch
Copy link
Member Author

/cc @srujzs @rakudrama

I'll confirm once the change rolls into our internal systems to verify that no regressions are introduced as a result of this change.

Overall I'd like to cherry-pick due to the high visibility of the bug in the new Dart/JS interop feature. Workarounds exist, but are not friendly (e.g. revert to the old JS-interop patterns).

@itsjustkevin itsjustkevin added cherry-pick-approved Label for approved cherrypick request merge-to-beta labels Jan 22, 2024
@a-siva a-siva added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Jan 23, 2024
@sigmundch
Copy link
Member Author

Just to confirm, the change reached our internal systems and the signal is green.

@itsjustkevin let me know if there are any constraints on when to land this.

@itsjustkevin
Copy link
Contributor

@sigmundch we are okay to land this based on approvals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve merge-to-beta
Projects
None yet
Development

No branches or pull requests

7 participants