Tracking issue for #![feature(const_precise_live_drops)]
#73255
Comments
ecstatic-morse
added
C-tracking-issue
jonas-schievink
added
the
T-lang
JohnTitor
added
the
B-unstable
|
Cc @rust-lang/wg-const-eval (which I think never happened for this feature or the PR) |
|
Is there particular reason why this was moved to be a feature? |
|
cc @rust-lang/lang I am nominating this feature for stabilization citing @ecstatic-morse from the impl PR: This isn't really a "feature" but a refinement to const-checking, and it's kind of non-obvious what the user opts in to with the feature gate, since drop elaboration is an implementation detail. A relevant precedent is This is also somewhat relevant to the stabilization of const _: Vec<i32> = {
let vec_tuple = (Vec::new(),);
vec_tuple.0
};
This code is functionally no different from the following, which currently works on stable because const X: Option<Vec<i32>> = { let x = Vec::new(); x };Const-checking only considers whole locals for const qualification, but drop elaboration is more precise: It works on projections as well. Because drop elaboration sees that all fields of the tuple have been moved from by the end of the initializer, no |
|
@oli-obk how do things look like in implementation complexity and the risk of locking us into a scheme that might be hard to maintain or make compatible with other future extensions? |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@DoumanAsh due to the halting problem, rustc will always report "incorrect errors". It is impossible to predict at compiletime if e.g. So there's always a trade-off between analysis complexity and "incorrect errors", but even the best analysis will sometimes show "incorrect errors". My impression after looking at the PR here is that the analysis complexity is totally reasonable (it mostly implicitly reuses the analysis performed by drop elaboration), but this is still a decision we should make explicitly, not implicitly. More precise drop analysis in @oli-obk Thanks. :) As you probably saw, I also left some questions in the PR that implemented the analysis. |
|
I did see these comments, just didn't have time to look more deeply yet. Cross posting from the comments on that PR: I am guessing the difference between the Of course such a change would insta-stabilize the feature gate from this PR without any reasonable way to avoid said stabilization. So I'm tempted to stabilize the feature but leave a FIXME on this function to merge its qualif checks into |
Even worse, it would change drop elaboration, which is a rather tricky part of the compiler.^^ That should be done with utmost care. |
|
Dropping the nomination here as it's waiting on Niko; that's tracked in the language team action item list. |
ghost
mentioned this issue
|
What remains to be done before this can be stabilized? |
|
Hey y'all. I'll try to summarize exactly what this feature gate does to help inform a decision on stabilization. Oli and Ralf probably already know this information, and can skip the next section. BackgroundCurrently, it is forbidden to run custom However, some
We rely on a separate MIR pass, Current Implementation
For these reasons, I chose the current implementation strategy. I realize that none of these arguments are dispositive, and I don't think it's unreasonable to gate stabilization of this feature on reimplementing the relevant bits of drop elaboration inside const-checking, although I still think it's overly conservative. Besides that big question, I think there were also some concerns from |
|
I'm not educated on the details, but it would be super nice to see this stabilized in some form. There are a comparatively large number of new APIs that rely on this. For examples, see issues #76654, #82814, and PR #84087, the last of which is an approved stabilization PR that can't be merged until this is stabilized. That's why I was checking in on the progress towards stabilization a few days ago. I'm sorry about that, by the way. I know that that sort of message can be annoying, but I wanted to know there if there was anything I could do to help move this along. |
|
The fact that the main blocker (and why this was feature gated in the first place) is the implementation makes it somewhat unusual (see #71824 (comment)). That makes it more the domain of the compiler-team rather than the lang-team. Niko reviewed #71824 and is assigned to this issue, but I'm hesitant to ping them specifically unless their expertise is required. So, if you want to see this stabilized I would figure out some process for getting consent from the compiler team. I think they use the MCP process for this exclusively? Oli is a member, so there's at least one potential sponsor. The team might require documenting the drop-elaboration/const-checking dependency in the dev-guide and maybe the module itself, which I'm happy to do if asked. After that, I can write a stabilization report with some examples and we can do lang-team FCP (assuming any lingering concerns from @rust-lang/wg-const-eval have been addressed). I'm, uh, not great at navigating bureaucratic systems with many veto points, so if you want to drive this forward your help would be greatly appreciated. However, unless we end up reimplementing drop-elaboration in const-checking like I mention above, I don't think much of the remaining work is technical. |
|
I think your summary post contains most of what we need for a stabilization report (full instructions here). We can just mark this issue as requiring sign-off from both T-compiler and T-lang and do all of this at once. |
|
Is this still blocked? It appears that #91009 has been closed. The ability to make |
|
We still have this problem: #73255 (comment). Basically exactly the thing we were worried about (depending on subtle dropck details) actually came up just after the attempt to stabilize.
|
|
Ah! Is there a dedicated issue open for that that can be linked in the description, or is that just a known issue at the moment? |
|
It's not a very concrete issue, and I don't think is tracked anywhere explicitly outside of this tracking issue. |
|
That's fair. As is expected with all these const features, something subtle and complicated lurks in the depths that makes it hard to finish up. I was kinda hopeful that this was mostly done, but alas. |
|
It might be, I am honestly not familiar enough with drop elaboration to really evaluate the trade-offs here. I hope someone else reading along has some good ideas for what can be done before we ask the lang team to discuss this again. |
|
One thing I did miss when writing the comments above is #91410: this feature no longer relies on full drop elaboration, just a lightweight version of dead drop removal. Still, doing this somewhere in the middle of our MIR pipeline does not feel great. Instead maybe it is possible to run the analysis that remove_uninit_drops performs during const-checking so that we can just ask "can this drop ever happen" rather than manifesting the result as explicit MIR? Then this could just happen during regular const checking. (Borrowck already does something like that, doesn't it? If someone knows a borrowck expert, please point them to this thread, maybe they can help :) Also see Zulip |
…lstrieb Stabilize const Atomic*::into_inner Partial stabilization for rust-lang#78729, for which the FCP has already completed. The other `into_inner` functions in that tracking issue (`UnsafeCell`, `Cell`, `RefCell`) are blocked on rust-lang#73255 for now. ```console error[E0493]: destructor of `UnsafeCell<T>` cannot be evaluated at compile-time --> library/core/src/cell.rs:2076:29 | 2076 | pub const fn into_inner(self) -> T { | ^^^^ the destructor for this type cannot be evaluated in constant functions 2077 | self.value 2078 | } | - value is dropped here ```
Rollup merge of rust-lang#123522 - dtolnay:constatomicintoinner, r=Nilstrieb Stabilize const Atomic*::into_inner Partial stabilization for rust-lang#78729, for which the FCP has already completed. The other `into_inner` functions in that tracking issue (`UnsafeCell`, `Cell`, `RefCell`) are blocked on rust-lang#73255 for now. ```console error[E0493]: destructor of `UnsafeCell<T>` cannot be evaluated at compile-time --> library/core/src/cell.rs:2076:29 | 2076 | pub const fn into_inner(self) -> T { | ^^^^ the destructor for this type cannot be evaluated in constant functions 2077 | self.value 2078 | } | - value is dropped here ```
|
These days I've been thinking that we should reformulate borrow check (and drop check) to take place on THIR. I plan to prototype this in a-mir-formality, which I suppose may have to be renamed to just formality as a result... ;'(. Seems relevant.
…On Mon, Mar 18, 2024, at 4:44 AM, Ralf Jung wrote:
One thing I did miss when writing the comments above is #91410 <#91410>: this feature no longer relies on full drop elaboration, just a lightweight version of dead drop removal.
Still, doing this somewhere in the middle of our MIR pipeline does not feel great. Instead maybe it is possible to run the analysis that remove_uninit_drops performs during const-checking so that we can just ask "can this drop ever happen" rather than manifesting the result as explicit MIR? Then this could just happen during regular const checking.
(Borrowck already does something like that, doesn't it? If someone knows a borrowck expert, please point them to this thread, maybe they can help :)
—
Reply to this email directly, view it on GitHub <#73255 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF4ZUKPPUCNG2OQHVJBPDYY2SPXAVCNFSM4N3YYLLKU5DIOJSWCZC7NNSXTN2JONZXKZKDN5WW2ZLOOQ5TEMBQGMZDCMZYGEYQ>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
Stabilize `&mut` (and `*mut`) as well as `&Cell` (and `*const Cell`) in const
This stabilizes `const_mut_refs` and `const_refs_to_cell`. That allows a bunch of new things in const contexts:
- Mentioning `&mut` types
- Creating `&mut` and `*mut` values
- Creating `&T` and `*const T` values where `T` contains interior mutability
- Dereferencing `&mut` and `*mut` values (both for reads and writes)
The same rules as at runtime apply: mutating immutable data is UB. This includes mutation through pointers derived from shared references; the following is diagnosed with a hard error:
```rust
#[allow(invalid_reference_casting)]
const _: () = {
let mut val = 15;
let ptr = &val as *const i32 as *mut i32;
unsafe { *ptr = 16; }
};
```
The main limitation that is enforced is that the final value of a const (or non-`mut` static) may not contain `&mut` values nor interior mutable `&` values. This is necessary because the memory those references point to becomes *read-only* when the constant is done computing, so (interior) mutable references to such memory would be pretty dangerous. We take a multi-layered approach here to ensuring no mutable references escape the initializer expression:
- A static analysis rejects (interior) mutable references when the referee looks like it may outlive the current MIR body.
- To be extra sure, this static check is complemented by a "safety net" of dynamic checks. ("Dynamic" in the sense of "running during/after const-evaluation, e.g. at runtime of this code" -- in contrast to "static" which works entirely by looking at the MIR without evaluating it.)
- After the final value is computed, we do a type-driven traversal of the entire value, and if we find any `&mut` or interior-mutable `&` we error out.
- However, the type-driven traversal cannot traverse `union` or raw pointers, so there is a second dynamic check where if the final value of the const contains any pointer that was not derived from a shared reference, we complain. This is currently a future-compat lint, but will become an ICE in rust-lang#128543. On the off-chance that it's actually possible to trigger this lint on stable, I'd prefer if we could make it an ICE before stabilizing const_mut_refs, but it's not a hard blocker. This part of the "safety net" is only active for mutable references since with shared references, it has false positives.
Altogether this should prevent people from leaking (interior) mutable references out of the const initializer.
While updating the tests I learned that surprisingly, this code gets rejected:
```rust
const _: Vec<i32> = {
let mut x = Vec::<i32>::new(); //~ ERROR destructor of `Vec<i32>` cannot be evaluated at compile-time
let r = &mut x;
let y = x;
y
};
```
The analysis that rejects destructors in `const` is very conservative when it sees an `&mut` being created to `x`, and then considers `x` to be always live. See [here](rust-lang#65394 (comment)) for a longer explanation. `const_precise_live_drops` will solve this, so I consider this problem to be tracked by rust-lang#73255.
Cc `@rust-lang/wg-const-eval` `@rust-lang/lang`
Cc rust-lang#57349
Cc rust-lang#80384
Rollup merge of rust-lang#129195 - RalfJung:const-mut-refs, r=fee1-dead Stabilize `&mut` (and `*mut`) as well as `&Cell` (and `*const Cell`) in const This stabilizes `const_mut_refs` and `const_refs_to_cell`. That allows a bunch of new things in const contexts: - Mentioning `&mut` types - Creating `&mut` and `*mut` values - Creating `&T` and `*const T` values where `T` contains interior mutability - Dereferencing `&mut` and `*mut` values (both for reads and writes) The same rules as at runtime apply: mutating immutable data is UB. This includes mutation through pointers derived from shared references; the following is diagnosed with a hard error: ```rust #[allow(invalid_reference_casting)] const _: () = { let mut val = 15; let ptr = &val as *const i32 as *mut i32; unsafe { *ptr = 16; } }; ``` The main limitation that is enforced is that the final value of a const (or non-`mut` static) may not contain `&mut` values nor interior mutable `&` values. This is necessary because the memory those references point to becomes *read-only* when the constant is done computing, so (interior) mutable references to such memory would be pretty dangerous. We take a multi-layered approach here to ensuring no mutable references escape the initializer expression: - A static analysis rejects (interior) mutable references when the referee looks like it may outlive the current MIR body. - To be extra sure, this static check is complemented by a "safety net" of dynamic checks. ("Dynamic" in the sense of "running during/after const-evaluation, e.g. at runtime of this code" -- in contrast to "static" which works entirely by looking at the MIR without evaluating it.) - After the final value is computed, we do a type-driven traversal of the entire value, and if we find any `&mut` or interior-mutable `&` we error out. - However, the type-driven traversal cannot traverse `union` or raw pointers, so there is a second dynamic check where if the final value of the const contains any pointer that was not derived from a shared reference, we complain. This is currently a future-compat lint, but will become an ICE in rust-lang#128543. On the off-chance that it's actually possible to trigger this lint on stable, I'd prefer if we could make it an ICE before stabilizing const_mut_refs, but it's not a hard blocker. This part of the "safety net" is only active for mutable references since with shared references, it has false positives. Altogether this should prevent people from leaking (interior) mutable references out of the const initializer. While updating the tests I learned that surprisingly, this code gets rejected: ```rust const _: Vec<i32> = { let mut x = Vec::<i32>::new(); //~ ERROR destructor of `Vec<i32>` cannot be evaluated at compile-time let r = &mut x; let y = x; y }; ``` The analysis that rejects destructors in `const` is very conservative when it sees an `&mut` being created to `x`, and then considers `x` to be always live. See [here](rust-lang#65394 (comment)) for a longer explanation. `const_precise_live_drops` will solve this, so I consider this problem to be tracked by rust-lang#73255. Cc `@rust-lang/wg-const-eval` `@rust-lang/lang` Cc rust-lang#57349 Cc rust-lang#80384
Partially stabilize const_pin Tracking issue rust-lang#76654. Eight of these methods can be made const-stable. The remainder are blocked on rust-lang#73255.
Partially stabilize const_pin Tracking issue rust-lang#76654. Eight of these methods can be made const-stable. The remainder are blocked on rust-lang#73255.
Partially stabilize const_pin Tracking issue rust-lang#76654. Eight of these methods can be made const-stable. The remainder are blocked on rust-lang#73255.
Partially stabilize const_pin Tracking issue rust-lang#76654. Eight of these methods can be made const-stable. The remainder are blocked on rust-lang#73255.
Rollup merge of rust-lang#130136 - GKFX:stabilize-const-pin, r=dtolnay Partially stabilize const_pin Tracking issue rust-lang#76654. Eight of these methods can be made const-stable. The remainder are blocked on rust-lang#73255.
This is a tracking issue for a more precise version of checking for drops in
constcontexts.The feature gate for the issue is
#![feature(const_precise_live_drops)].With this feature enabled, drops are checked on slightly elaborated MIR. This makes the analysis that prevents dropping in
const fnmore accurate for code that does not actually drop anything: specifically, if the initial MIR contains an unnecessary call todropthat may be eliminated in elaboration, it can be accepted withconst_precise_live_drops.Internally, the library can also use this on a per-function basis with
#[rustc_allow_const_fn_unstable(const_precise_live_drops)].About tracking issues
Tracking issues are used to record the overall progress of implementation.
They are also used as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.
Discussion comments will get marked as off-topic or deleted.
Repeated discussions on the tracking issue may lead to the tracking issue getting locked.
Steps
Blockers and Concerns:
The text was updated successfully, but these errors were encountered: