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] [vm/aot/tfa] Fix handling of type parameter nullability in factory constructors #50412

Closed
alexmarkov opened this issue Nov 8, 2022 · 5 comments
Closed

[CP] [vm/aot/tfa] Fix handling of type parameter nullability in factory constructors #50412

alexmarkov opened this issue Nov 8, 2022 · 5 comments
Assignees
@vsmenon @whesse @itsjustkevin @mit-mit @athomas
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request

Comments

@alexmarkov
Copy link
Contributor

Commit(s) to merge

8ef9c6a

Target

stable

Prepared changelist for beta/stable

https://dart-review.git.corp.google.com/c/sdk/+/268500

Issue Description

Type flow analysis (part of AOT compiler) ignored nullability of type parameter types (T?) in factory constructors. This caused incorrect results of analysis and incorrect optimizations. Only factories of generic classes which use type parameters as nullable (T?) are affected.

What is the fix

Added a new ApplyNullability operation to a summary in order to apply any extra nullability ('?' or '*') on top of the type argument passed to a factory constructor.

Why cherry-pick

This bug caused incorrect optimizations which may lead to a crash or a silent incorrect behavior which may be hard to diagnose.

Risk

low

Issue link(s)

#50392

Extra Info

No response
@alexmarkov alexmarkov added the cherry-pick-review Issue that need cherry pick triage to approve label Nov 8, 2022
@alexmarkov
Copy link
Contributor Author

/cc @a-siva @mraleph

@a-siva a-siva added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Nov 9, 2022
@mraleph
Copy link
Member

mraleph commented Nov 9, 2022

I support cherry picking.

@a-siva
Copy link
Contributor

a-siva commented Nov 9, 2022

lgtm.

@itsjustkevin
Copy link
Contributor

Approving this cherry-pick.

@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 Nov 9, 2022
@alexmarkov
Copy link
Contributor Author

Cherry-pick landed in 203d14e.

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-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. cherry-pick-approved Label for approved cherrypick request
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@mraleph @vsmenon @whesse @a-siva @itsjustkevin @mit-mit @athomas @alexmarkov