Initial start at summarizing unstable const discussion #46
Conversation
| **Replacing a derive with a manual impl**: generally not permitted, see | ||
| [#102371](https://github.com/rust-lang/rust/issues/102371) for context. We are | ||
| waiting on const-derive being implemented to avoid an increase in manual | ||
| implementations throughout the standard library. |
There was a problem hiding this comment.
does rust-lang/rust#102049 mean this section can be removed
| Does making this API const require that code be hand-written which was previously automatic? | ||
|
|
||
| Does the new code require duplicating, for example through `#[cfg(bootstrap)]`, | ||
| increasing the risk of diverging between the two implementations (and |
There was a problem hiding this comment.
afaik this has never happened before. tests are run on not(bootstrap) mode, and that is the only relevant mode afaict.
There was a problem hiding this comment.
I think it's pretty clear that duplicating ~100+ lines of code is a recipe for editing only one of those copies. We don't always have a regression test (e.g., for performance improvements), and due to git these edits may happen entirely in parallel in terms of PRs.
There was a problem hiding this comment.
I agree, but we have never seen this happen to my knowledge.
| example, if a trait is made const, that likely means all implementations in the | ||
| standard library have to be const too. It may not be the case that this is easy |
There was a problem hiding this comment.
This is never and will never be true. A const trait can have non-const impls, otherwise we'd not be able to mark any traits as const.
There was a problem hiding this comment.
Okay, maybe a poor example. I suspect there are some things like this though? e.g., my understanding is that today a const impl is either all const (for every method). I'm happy to reword with help on where the limitation actually is.
(Obviously future work may remove those constraints, but this document is very much emphasizing current reality - or even cfg(bootstrap) reality).
There was a problem hiding this comment.
ah, yes, that makes a lot more sense ^^. Either all default methods on a trait are const or none.
While I disagree with the policy (we could just make them all const for now and revert the constness if it becomes a problem), I'm fine following it to save everyone headaches
| When making an internal (or external) API `const`, it becomes harder to | ||
| evaluate whether a given standard library function is safe to stabilize as | ||
| const. One of its dependencies may take a long time to stabilize for | ||
| const-evaluation, even though its API doesn't obviously reference unstable | ||
| surface area. | ||
|
|
||
| For example, a function like `[T]::get_many_mut` may rely on | ||
| sorting to guarantee distinct indices. If sorting is made unstably const, it is | ||
| difficult to know when looking at `get_many_mut` whether it is OK to | ||
| const-stabilize it: you have to inspect the full dependency tree with knowledge | ||
| of the "fully determined" const surface area. |
There was a problem hiding this comment.
we have logic to automate this. You cannot stabilize a const fn's constness if it calls unstable const fns unless you explicitly add #[rustc_allow_const_fn_unstable(foo, bar)]
There was a problem hiding this comment.
Are all transitively called const fns required to be either stable or unstable by annotation? It was my impression that private helpers are sometimes const but not tagged with a const stability marker. Maybe bad memory on my part though.
There was a problem hiding this comment.
untagged functions are const stable by default, so you can't call unstable functions in the body unless you tag it
There was a problem hiding this comment.
hmm... I should add tests, but I'm fairly certain you can only avoid the stability marker if you're already unstable.
There was a problem hiding this comment.
This compiles fine today (just threw this into library/core/src/lib.rs). To me that looks like it's exposing in a const-stable function functionality that might depend on const-eval-select (obviously this example doesn't but it's no guarantee).
Even if we forbid this, we'll want some language here about the thought process for adding const_stable to internal functions -- I suspect we're not very good at having rules there yet.
const fn foo() {
unsafe { crate::intrinsics::const_eval_select((), comp, run) }
}
const fn comp() {}
fn run() {}
#[stable(feature = "unreachable", since = "1.27.0")]
#[rustc_const_stable(feature = "const_unreachable_unchecked", since = "1.57.0")]
pub const fn hidden_danger() {
foo();
}There was a problem hiding this comment.
ah yes. I remember that there was some issue with not being able to add those attributes to functions that don't already have stable/unstable attributes, but that seems to have gotten fixed: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=9df5e879c896456abcf21973709242b8 so we could now start checking that internal functions have const stability attributes
There was a problem hiding this comment.
Cc rust-lang/rust#103193 - looks like a practical example of this case, presumably there's more hiding.
| When making an internal (or external) API `const`, it becomes harder to | ||
| evaluate whether a given standard library function is safe to stabilize as | ||
| const. One of its dependencies may take a long time to stabilize for | ||
| const-evaluation, even though its API doesn't obviously reference unstable | ||
| surface area. |
There was a problem hiding this comment.
I am a bit confused by this point here. If an API isn't const in the first place, then we don't need to evaluate whether it is okay to stabilize as const. If it is, then we can always split the feature up if unsure.
I think this is a decent initial start, but might be a bit longer than we want -- maybe could use a TL;DR or checklist of some kind.
r? @m-ou-se