Vec::push in consts MVP
#147893
Open
Vec::push in consts MVP
#147893
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
rustbot
added
S-waiting-on-author
This comment has been minimized.
This comment has been minimized.
RalfJung
reviewed
Oct 20, 2025
Member
There was a problem hiding this comment.
We used to have the plan to have a dedicated compile-time allocator type... this is an interesting alternative. Given that it also involves the library surface, we should probably involve t-libs-api.
Why have you picked Vec as the first type for this? I think it'd make more sense for Box to go first since that is the most primitive type for heap allocations.
library/alloc/src/alloc.rs
Outdated
Comment on lines
377
to
382
| let mut offset = 0; | ||
| while offset < size { | ||
| offset += 1; | ||
| // SAFETY: the pointer returned by `const_allocate` is valid to write to. | ||
| ptr.add(offset).write(0) | ||
| } |
Member
There was a problem hiding this comment.
It would be much more efficient to use write_bytes here.
library/alloc/src/vec/mod.rs
Outdated
| /// `Vec<T>` created during compile time. | ||
| #[unstable(feature = "const_heap", issue = "79597")] | ||
| #[rustc_const_unstable(feature = "const_heap", issue = "79597")] | ||
| pub const fn const_leak(mut self) -> &'static [T] { |
Member
There was a problem hiding this comment.
This is conceptually quite different from leak which IMO should be reflected in the name, so I would indeed prefer const_make_global.
RalfJung
reviewed
Oct 20, 2025
library/alloc/src/vec/mod.rs
Outdated
| /// `Vec<T>` created during compile time. | ||
| #[unstable(feature = "const_heap", issue = "79597")] | ||
| #[rustc_const_unstable(feature = "const_heap", issue = "79597")] | ||
| pub const fn const_leak(mut self) -> &'static [T] { |
Member
There was a problem hiding this comment.
This needs a where T: Freeze for soundness.
RalfJung
reviewed
Oct 20, 2025
library/alloc/src/vec/mod.rs
Outdated
| } | ||
|
|
||
| /// Leaks the `Vec<T>` to be interned statically. This mut be done for all | ||
| /// `Vec<T>` created during compile time. |
Member
There was a problem hiding this comment.
It doesn't need to be done for all Vec created during compile-time -- only for those that you want to carry over to runtime. You can use intermediate Vec as scratch space or so during compile time that you never call this method on.
Member
Author
That will either require us doing a |
This comment has been minimized.
This comment has been minimized.
rust-cloud-vms
bot
force-pushed
the
constheapheapheap
branch
from
09046cc to
1c6257e
Compare
This comment has been minimized.
This comment has been minimized.
rust-cloud-vms
bot
force-pushed
the
constheapheapheap
branch
from
1c6257e to
aded02c
Compare
Contributor
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rust-bors bot
added a commit
that referenced
this pull request
Oct 26, 2025
`Vec::push` in consts MVP
rustbot
added
the
S-waiting-on-perf
This comment has been minimized.
This comment has been minimized.
Collaborator
|
Finished benchmarking commit (b3f7ade): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 0.4%, secondary -0.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 3.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.3%, secondary 0.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 473.751s -> 473.82s (0.01%) |
rustbot
added
perf-regression
rustbot
added
the
S-waiting-on-review
fee1-dead
force-pushed
the
constheapheapheap
branch
from
7f695cf to
3cc8d08
Compare
fee1-dead
removed
the
S-blocked
Member
Author
|
Updated and the rustdoc regression should be minimal now (just things getting regrouped under different impl blocks). As this is going to be a nightly feature I think the amount of testing is okay here, let me know if you want some edge cases to be tested. |
oli-obk
approved these changes
Dec 27, 2025
Contributor
|
@bors r+ |
Collaborator
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Dec 28, 2025
`Vec::push` in consts MVP
Example:
```rust
const X: &'static [u32] = {
let mut v = Vec::with_capacity(6);
let mut x = 1;
while x < 42 {
v.push(x);
x *= 2;
}
assert!(v.len() == 6);
v.const_make_global()
};
assert_eq!([1, 2, 4, 8, 16, 32], X);
```
Oh this is fun...
* We split out the implementation of `Global` such that it calls `intrinsics::const_allocate` and `intrinsics::const_deallocate` during compile time. This is achieved using `const_eval_select`
* This allows us to `impl const Allocator for Global`
* We then constify everything necessary for `Vec::with_capacity` and `Vec::push`.
* Added `Vec::const_make_global` to leak and intern the final value via `intrinsics::const_make_global`. If we see any pointer in the final value of a `const` that did not call `const_make_global`, we error as implemented in #143595.
r? `@rust-lang/wg-const-eval`
To-do for me:
* [x] Assess the rustdoc impact of additional bounds in the method
* [x] ~~Increase test coverage~~ I think this is enough for an unstable feature.
Collaborator
This comment has been minimized.
This comment has been minimized.
Collaborator
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
Member
|
Cc @rust-lang/libs FYI |
fee1-dead
force-pushed
the
constheapheapheap
branch
from
3cc8d08 to
d0645b6
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Member
|
Interesting, seems like that test needs to be made deterministic, and also there's some sort of bug in Miri causing an ICE here. Either way it's unrelated to the changes in this PR. EDIT: rust-lang/miri#4796 should fix that |
Member
|
If you rebase, the strange Miri failure should be gone. |
fee1-dead
force-pushed
the
constheapheapheap
branch
from
d0645b6 to
0defb1d
Compare
Collaborator
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Collaborator
|
The job Click to see the possible cause of the failure (guessed by this bot) |
Member
Author
|
I don't understand what is going on with the miri differences, @RalfJung what should I change here? Bless the test? Or change something so that the test still retains the original message? |
Collaborator
|
☔ The latest upstream changes (presumably #147247) made this pull request unmergeable. Please resolve the merge conflicts. |
Member
|
Ah dang, I fixed the ICE but forgot to fix the non-determinism... |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Example:
Oh this is fun...
Globalsuch that it callsintrinsics::const_allocateandintrinsics::const_deallocateduring compile time. This is achieved usingconst_eval_selectimpl const Allocator for GlobalVec::with_capacityandVec::push.Vec::const_make_globalto leak and intern the final value viaintrinsics::const_make_global. If we see any pointer in the final value of aconstthat did not callconst_make_global, we error as implemented in addconst_make_global; err forconst_allocateptrs if didn't call #143595.r? @rust-lang/wg-const-eval
To-do for me:
Increase test coverageI think this is enough for an unstable feature.