Skip to content

[CP] [dart2wasm] Only perform compile-time lookup in constant list if index is in-bounds #55853

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
Closed
mkustermann opened this issue May 28, 2024 · 5 comments
Closed

[CP] [dart2wasm] Only perform compile-time lookup in constant list if index is in-bounds #55853

mkustermann opened this issue May 28, 2024 · 5 comments
Assignees
@mraleph @vsmenon @itsjustkevin @mit-mit @athomas
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve

Comments

@mkustermann
Copy link
Member

Commit(s) to merge

827a7c4

Target

stable

Prepared changelist for beta/stable

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

Issue Description

Compiler crashes when compiling list index access code when it can infer the list to be a constant list and the index to be constant but the index is out-of-bounds.

What is the fix

Only perform the optimization if the list index is in-bounds.

Why cherry-pick

Users have hit this issue.

Risk

Low

Issue link(s)

#55817

Extra Info

No response
@mkustermann mkustermann added the cherry-pick-review Issue that need cherry pick triage to approve label May 28, 2024
@mkustermann mkustermann mentioned this issue May 28, 2024
Closed
@athomas
Copy link
Member

athomas commented May 28, 2024

@mraleph please review.

@mraleph
Copy link
Member

mraleph commented May 28, 2024

LGTM

@mit-mit mit-mit added the area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). label May 28, 2024
@athomas athomas added the cherry-pick-approved Label for approved cherrypick request label May 29, 2024
copybara-service bot pushed a commit that referenced this issue May 29, 2024
@mkustermann
[stable] [dart2wasm] Only perform compile-time lookup in constant lis…
73baf6c 
…t if index is in-bounds

We have an optimization that will do list lookups at compile time when
the receiver is a constant list and the index is a constant integer.

=> We should only perform this optimization if index is in-bounds.
=> If it's out-of-bounds it should be a [RangeError] thrown at runtime
   (if that code is ever executed)

Bug: #55817

Cherry-pick: https://dart-review.googlesource.com/c/sdk/+/368302
Cherry-pick-request: #55853
Change-Id: I6a8b8ebd4ec0917d963e425fb0202aaef8d19bbd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/368304
Reviewed-by: Slava Egorov <[email protected]>
Reviewed-by: Ömer Ağacan <[email protected]>
Commit-Queue: Martin Kustermann <[email protected]>
@mkustermann
Copy link
Member Author

The CL has been merged to stable channel

@mkustermann
Copy link
Member Author

It seems 3.4.2 was released. @athomas can the CP github issues (like this) be closed now?

@itsjustkevin
Copy link
Contributor

Thank you, @mkustermann! Confirmed that this landed in 3.4.2, closing issue.

For future cherry-picks, prefer linking the CHANGELOG updates back to the cherry-pick issue as these issues provide more succinct context and link to the original issue either way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mraleph mraleph

@vsmenon vsmenon

@itsjustkevin itsjustkevin

@mit-mit mit-mit

@athomas athomas

Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). cherry-pick-approved Label for approved cherrypick request cherry-pick-review Issue that need cherry pick triage to approve
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@mraleph @vsmenon @mkustermann @itsjustkevin @mit-mit @athomas