Skip to content

interpret: add allocation parameters to AllocBytes #141513

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
Merged
merged 1 commit into from
May 26, 2025
Merged

interpret: add allocation parameters to AllocBytes #141513

merged 1 commit into from
May 26, 2025

Conversation

nia-e
Copy link
Contributor

@nia-e nia-e commented May 24, 2025

Necessary for a better implementation of rust-lang/miri#4343. Also included here is the code from that PR, adapted to this new interface for the sake of example and so that CI can run on them; the Miri changes can be reverted and merged separately, though.

r? @RalfJung

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 24, 2025

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri, @RalfJung, @oli-obk, @lcnr

@nia-e nia-e marked this pull request as draft May 24, 2025 18:43
@nia-e
Copy link
Contributor Author

nia-e commented May 24, 2025

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 24, 2025
@rustbot
Copy link
Collaborator

rustbot commented May 24, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@RalfJung
Copy link
Member

So you don't want a review yet? 🤔

RalfJung
RalfJung reviewed May 24, 2025
View reviewed changes
@nia-e
Copy link
Contributor Author

nia-e commented May 24, 2025

Feel free to go ahead, but it's not fully implemented. I realised right after opening this there was more to do, oopsie

@nia-e nia-e force-pushed the allocbytes-extend branch from 3dd9d4f to 15a3f38 Compare May 24, 2025 21:23
@nia-e
Copy link
Contributor Author

nia-e commented May 24, 2025

Ok! So I spent an hour or two panicking over some failing tests only to realise that they're failing on master, actually. Sobs. This was actually not far from ready from the start, oh well.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 24, 2025
@nia-e nia-e marked this pull request as ready for review May 24, 2025 21:28
@rustbot
Copy link
Collaborator

rustbot commented May 24, 2025

Some changes occurred to the CTFE / Miri interpreter

cc @rust-lang/miri

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE machinery

cc @RalfJung, @oli-obk, @lcnr

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nia-e nia-e force-pushed the allocbytes-extend branch from 15a3f38 to 3c71aef Compare May 24, 2025 21:43
RalfJung
RalfJung reviewed May 25, 2025
View reviewed changes
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! However, I have some questions. :)

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 25, 2025
@RalfJung
Copy link
Member

RalfJung commented May 25, 2025 •
edited
Loading

Ok! So I spent an hour or two panicking over some failing tests only to realise that they're failing on master, actually

I see no test failure in CI here? If some tests fail on your machine on an unchanged master branch, please file an issue with instructions to reproduce the problem.

@RalfJung RalfJung changed the title Specify an associated type on AllocBytes interpret: add allocation parameters to AllocBytes May 25, 2025
@nia-e
Copy link
Contributor Author

nia-e commented May 25, 2025

Thanks for the review! I'll get back to you with fixes in a few hours, but the failure was when running ./x test miri (the first round of tests was fine, the second got 40ish failures complaining about a decoder exhausting memory or something). Though that might just be my machine being weird; if it's still the case later when I can retry, I'll file an issue

@nia-e nia-e force-pushed the allocbytes-extend branch from 3c71aef to 179f5de Compare May 25, 2025 15:09
@nia-e
Copy link
Contributor Author

nia-e commented May 25, 2025 •
edited
Loading

Seems like the failing tests are just a me thing, fixed it by doing a cargo clean and rebuilding. I also pushed something that hopefully addresses the review!

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 25, 2025
RalfJung
RalfJung reviewed May 25, 2025
View reviewed changes
Copy link
Member

@RalfJung RalfJung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just some minor comments. :)

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 25, 2025
@nia-e nia-e force-pushed the allocbytes-extend branch from f0e97fd to 4406446 Compare May 25, 2025 21:47
@nia-e
Copy link
Contributor Author

nia-e commented May 25, 2025

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 25, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@nia-e nia-e force-pushed the allocbytes-extend branch from 4406446 to e388a3e Compare May 25, 2025 22:15
@RalfJung
Copy link
Member

All right, let's land this. :)
@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 26, 2025

📌 Commit e388a3e has been approved by RalfJung

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 26, 2025
@jieyouxu jieyouxu mentioned this pull request May 26, 2025
Merged
bors added a commit that referenced this pull request May 26, 2025
@bors
Auto merge of #141605 - jieyouxu:rollup-3gjqh5l, r=jieyouxu
2805e1d 
Rollup of 10 pull requests

Successful merges:

 - #140898 (minor improvements on running miri)
 - #141392 (Avoid obligation construction dance with query region constraints)
 - #141431 (Emit dummy open drop for unsafe binder)
 - #141433 (Properly analyze captures from unsafe binders)
 - #141439 (Deduplicate dyn compatibility violations due to coercion)
 - #141449 (further deduplicate ast visitor code)
 - #141513 (interpret: add allocation parameters to `AllocBytes`)
 - #141516 (speed up charsearcher for ascii chars)
 - #141526 (add a dedicated section for compiler environment variables in the unstable book)
 - #141550 (Fix `unused_braces` lint suggestion when encountering attributes)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit afb57ca into rust-lang:master May 26, 2025
6 checks passed
@rustbot rustbot added this to the 1.89.0 milestone May 26, 2025
rust-timer added a commit that referenced this pull request May 26, 2025
@rust-timer
Unrolled build for #141513
e6be81c 
Rollup merge of #141513 - nia-e:allocbytes-extend, r=RalfJung

interpret: add allocation parameters to `AllocBytes`

Necessary for a better implementation of [rust-lang/miri#4343](rust-lang/miri#4343). Also included here is the code from that PR, adapted to this new interface for the sake of example and so that CI can run on them; the Miri changes can be reverted and merged separately, though.

r? `@RalfJung`
@nia-e nia-e deleted the allocbytes-extend branch May 26, 2025 23:47
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request May 27, 2025
@bors
Auto merge of #141605 - jieyouxu:rollup-3gjqh5l, r=jieyouxu
b282c28 
Rollup of 10 pull requests

Successful merges:

 - rust-lang/rust#140898 (minor improvements on running miri)
 - rust-lang/rust#141392 (Avoid obligation construction dance with query region constraints)
 - rust-lang/rust#141431 (Emit dummy open drop for unsafe binder)
 - rust-lang/rust#141433 (Properly analyze captures from unsafe binders)
 - rust-lang/rust#141439 (Deduplicate dyn compatibility violations due to coercion)
 - rust-lang/rust#141449 (further deduplicate ast visitor code)
 - rust-lang/rust#141513 (interpret: add allocation parameters to `AllocBytes`)
 - rust-lang/rust#141516 (speed up charsearcher for ascii chars)
 - rust-lang/rust#141526 (add a dedicated section for compiler environment variables in the unstable book)
 - rust-lang/rust#141550 (Fix `unused_braces` lint suggestion when encountering attributes)

r? `@ghost`
`@rustbot` modify labels: rollup
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RalfJung RalfJung RalfJung left review comments

Assignees

@RalfJung RalfJung

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.89.0
Development

Successfully merging this pull request may close these issues.
5 participants
@nia-e @rustbot @RalfJung @rust-log-analyzer @bors