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] Avoid CFE crash when input contains invalid super parameters usage #50052

Closed
mraleph opened this issue Sep 26, 2022 · 11 comments
Closed

[CP] Avoid CFE crash when input contains invalid super parameters usage #50052

mraleph opened this issue Sep 26, 2022 · 11 comments
Assignees
@vsmenon @whesse @itsjustkevin @mit-mit @athomas
Labels
area-front-end Use area-front-end for front end / CFE / kernel format related issues. cherry-pick-approved Label for approved cherrypick request merge-to-stable

Comments

@mraleph
Copy link
Member

mraleph commented Sep 26, 2022 •
edited
Loading

Commit(s) to merge

bbdb9e4

Target

stable

Issue Description

CFE crashes when input contains incorrect usage of super parameters feature instead of reporting a syntax error.

What is the fix

Fix global type inference to propertly handle situations where incorrect super parameters are involved.

Why cherry-pick

Users get an uninformative CFE crash with a stack trace instead of a readable syntax error and can't really figure out what is wrong with their code on their own.

Risk

low

Issue link(s)

#49641
flutter/flutter#111833
#47470 (comment)

Extra Info

No response
@mraleph mraleph added the cherry-pick-review Issue that need cherry pick triage to approve label Sep 26, 2022
@mraleph mraleph mentioned this issue Sep 26, 2022
Closed
@itsjustkevin
Copy link
Contributor

@johnniwinther @vsmenon @athomas @sigmundch thoughts on this cherry-pick?

@johnniwinther
Copy link
Member

LGTM

@vsmenon
Copy link
Member

vsmenon commented Sep 26, 2022

lgtm

@mit-mit
Copy link
Member

mit-mit commented Sep 26, 2022

SGTM

@itsjustkevin
Copy link
Contributor

itsjustkevin commented Sep 26, 2022 •
edited
Loading

Marking this as approved. If anyone has a major reason why this shouldn't happen, please note it here.

@itsjustkevin itsjustkevin added cherry-pick-approved Label for approved cherrypick request and removed cherry-pick-review Issue that need cherry pick triage to approve labels Sep 26, 2022
@kevmoo kevmoo added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Sep 26, 2022
@mraleph mraleph added area-front-end Use area-front-end for front end / CFE / kernel format related issues. and removed area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. labels Sep 26, 2022
@athomas
Copy link
Member

athomas commented Sep 27, 2022

We should also merge this fix to beta, it missed the branchout.

copybara-service bot pushed a commit that referenced this issue Sep 27, 2022
@whesse
[release] Update changelog for 2.18.2
2826323 
Bug: #50052
Change-Id: I1d9aedac81b2fdb05f04a0246d1088ffa66192ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261300
Reviewed-by: Alexander Thomas <athom@google.com>
copybara-service bot pushed a commit that referenced this issue Sep 27, 2022
@whesse
[release] Update changelog for 2.18.2
0d9d9b2 
Bug: #50052
Change-Id: I1d9aedac81b2fdb05f04a0246d1088ffa66192ef
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/261300
Reviewed-by: Alexander Thomas <athom@google.com>
@whesse
Copy link
Contributor

whesse commented Sep 28, 2022

Merged to stable 2.18.2, not closing until it is fixed on beta as well.

@itsjustkevin
Copy link
Contributor

@athomas and @mraleph is there an immediate need to submit this to beta already or can it wait until the next release window?

@athomas
Copy link
Member

athomas commented Sep 28, 2022

@itsjustkevin If we don't plan another beta before the beta 2 branchout, this issue alone doesn't seem enough to warrant a new beta build (given that beta 2 is not too far in the future).

@itsjustkevin
Copy link
Contributor

Fair point @athomas if it turns out to be a big callout, we can pivot.

@whesse
Copy link
Contributor

whesse commented Oct 4, 2022

Closing issue - it is landed in stable, and will be in the new beta branchout created today.

@athomas athomas closed this as completed Oct 4, 2022
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-front-end Use area-front-end for front end / CFE / kernel format related issues. cherry-pick-approved Label for approved cherrypick request merge-to-stable
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@kevmoo @mraleph @vsmenon @whesse @itsjustkevin @johnniwinther @mit-mit @athomas