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.

Sign up for GitHub

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

Jump to bottom

[CP] fix dart2js crash for rare combination of local functions + generics + records #52216

Closed
sigmundch opened this issue Apr 28, 2023 · 4 comments
Closed

[CP] fix dart2js crash for rare combination of local functions + generics + records #52216

sigmundch opened this issue Apr 28, 2023 · 4 comments
Assignees
@vsmenon @whesse @itsjustkevin @mit-mit @athomas
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-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable web-dart2js

Comments

@sigmundch
Copy link
Member

sigmundch commented Apr 28, 2023 •
edited
Loading

Commit(s) to merge

8ee2b8b

Target

stable

Prepared changelist for stable

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

Issue Description

dart2js crashes in a rare combination of local functions + generics + records. This was caused by a bug: dart2js processed some data out of order.

What is the fix

The fix was to restore the correct processing order in the compiler.

Why cherry-pick

Impact is fairly low, and would be a reason to skip this cherry-pick (it is also why we decided not to cherry-pick in beta before the stable release, but are considering this for a patch release after 3.0 goes out)

That said, the way it is exposed is a crash, which we generally want to avoid in our stable releases.

The fix is really small and considered low risk.

Risk

low

Issue link(s)

#51899

Extra Info

This is a cherry-pick for 3.0.1 stable (not the 2.19 stable). We haven't created the gerrit CL yet because the stable branch doesn't have 3.0 changes yet.
@sigmundch sigmundch added the cherry-pick-review Issue that need cherry pick triage to approve label Apr 28, 2023
@sigmundch
Copy link
Member Author

FYI @rakudrama

@mraleph mraleph added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Apr 28, 2023
@itsjustkevin itsjustkevin added merge-to-stable cherry-pick-hold Hold off on this cherry-pick labels May 1, 2023
@itsjustkevin
Copy link
Contributor

@sigmundch do we still desire this cherry-pick? Based on the low risk and impact on users, this seems to meet the criteria for cherry-pick approval.

@sigmundch
Copy link
Member Author

Yes, I'm inclined to cherry-pick. I just created the CL for it and linked it above.

@itsjustkevin
Copy link
Contributor

I am marking this as approved.

@itsjustkevin itsjustkevin added cherry-pick-approved Label for approved cherrypick request and removed cherry-pick-hold Hold off on this cherry-pick labels May 11, 2023
copybara-service bot pushed a commit that referenced this issue May 12, 2023
@sigmundch
[cp] Cherry-pick 8ee2b8b to stable.
2794288 
This cherry-picks a fix for a dart2js crash onto the stable channel.

For more information see #52216

=== original commit message ===
[dart2js] Convert recordTypes after closureData

Bug #51899

Change-Id: Id9108e6cf13aee409b127fe6cd007bef8f0fe609
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/298460
Reviewed-by: Sigmund Cherem <[email protected]>
Reviewed-by: Mayank Patke <[email protected]>
Commit-Queue: Stephen Adams <[email protected]>
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/302801
Commit-Queue: Sigmund Cherem <[email protected]>
@sigmundch sigmundch added the cherry-pick-merged Cherry-pick has been merged to the stable or beta branch. label May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@vsmenon vsmenon

@whesse whesse

@itsjustkevin itsjustkevin

@mit-mit mit-mit

@athomas athomas

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-merged Cherry-pick has been merged to the stable or beta branch. cherry-pick-review Issue that need cherry pick triage to approve merge-to-stable web-dart2js
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@mraleph @fishythefish @sigmundch @vsmenon @whesse @itsjustkevin @mit-mit @athomas