Constify implementations of (Try)From for int types
#86840
Conversation
|
r? @m-ou-se (rust-highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
usbalbin
changed the title
(Try)From for number types(Try)From for int types
|
☔ The latest upstream changes (presumably #87768) made this pull request unmergeable. Please resolve the merge conflicts. |
|
r? @oli-obk Please remove the commented out attribute and create a tracking issue, then we can merge this |
|
Ok, on it |
|
Tracking issue added and code updated. I also added a test specific to floats since that will also be enabled by this |
|
@bors r+ rollup |
|
📌 Commit c8bf5ed has been approved by |
bors
added
S-waiting-on-bors
| assert_eq!(FROM, 1i64); | ||
|
|
||
| // From int to float | ||
| const FROM_F64: f64 = f64::from(42u8); |
There was a problem hiding this comment.
cc @rust-lang/wg-const-eval this is an interesting edge case around floats. We forbid floats in const fn, but those checks only happen at the declaration site, and libcore uses the feature gate that allows them. So now we can invoke const fns (only within items, not within other const fn) that work with floats.
There was a problem hiding this comment.
Yeah, we should really work towards getting rid of "things that are allowed in const but forbidden in const fn"...
|
⌛ Testing commit c8bf5ed with merge 8bb9ff63ed0125748c04b13a40ef468b97b84708... |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
@bors retry |
bors
added
S-waiting-on-bors
Constify implementations of `(Try)From` for int types I believe this to be one of the (many?) things blocking const (Range) iterators. ~~If this is to be merged maybe that should wait until `#![feature(const_trait_impl)]` no longer needs `#![allow(incomplete_features)]`?~~ - Done
Constify implementations of `(Try)From` for int types I believe this to be one of the (many?) things blocking const (Range) iterators. ~~If this is to be merged maybe that should wait until `#![feature(const_trait_impl)]` no longer needs `#![allow(incomplete_features)]`?~~ - Done
| @@ -45,7 +45,8 @@ impl_float_to_int!(f64 => u8 u16 u32 u64 u128 usize i8 i16 i32 i64 i128 isize); | |||
| macro_rules! impl_from { | |||
| ($Small: ty, $Large: ty, #[$attr:meta], $doc: expr) => { | |||
| #[$attr] | |||
| impl From<$Small> for $Large { | |||
| #[rustc_const_unstable(feature = "const_num_from_num", issue = "87852")] | |||
There was a problem hiding this comment.
Stability attributes do not usually work on trait impls. Do they work for impl const?
There was a problem hiding this comment.
That tests the case where we call a trait fn on a concrete type.
But there is another way to use a trait impl: to satisfy a bound.
const fn const_context_generic<T: MyTrait>() {
T::func();
}
const_context_generic::<Unstable>();Is that properly checked?
There was a problem hiding this comment.
That tests the case where we call a trait fn on a concrete type.
But there is another way to use a trait impl: to satisfy a bound.
const fn const_context_generic<T: MyTrait>() {
T::func();
}
const_context_generic::<Unstable>();Is that properly checked?
There was a problem hiding this comment.
You are right, those are probably not. I guess it is fine to let this PR pass and I will come up with a fix in the future. (and we need to wait for #87375, as that introduces a check on const bounds to error on previous code that should not be accepted) Trait bounds on const functons are gated behind a feature anyways, so even if this would allow one to use an unstable library feature without having it enabled, it would still require users to use a nightly compiler anyways.
Rollup of 14 pull requests Successful merges: - rust-lang#86840 (Constify implementations of `(Try)From` for int types) - rust-lang#87582 (Implement `Printer` for `&mut SymbolPrinter`) - rust-lang#87636 (Added the `Option::unzip()` method) - rust-lang#87700 (Expand explanation of E0530) - rust-lang#87811 (Do not ICE on HIR based WF check when involving lifetimes) - rust-lang#87848 (removed references to parent/child from std::thread documentation) - rust-lang#87854 (correctly handle enum variants in `opt_const_param_of`) - rust-lang#87861 (Fix heading colours in Ayu theme) - rust-lang#87865 (Clarify terms in rustdoc book) - rust-lang#87876 (add `windows` count test) - rust-lang#87880 (Remove duplicate trait bounds in `rustc_data_structures::graph`) - rust-lang#87881 (Proper table row formatting in platform support) - rust-lang#87889 (Use smaller spans when suggesting method call disambiguation) - rust-lang#87895 (typeck: don't suggest inaccessible fields in struct literals and suggest ignoring inaccessible fields in struct patterns) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
|
Any suggestions for what to put under |
|
I don't think the default libs api issue template applies here. |
|
I'd probably put the |
There is quite a massive amount of impls :) also note that I have not added any impls just constified the existing ones |
|
Yeah, with these macros there's a lot of them... maybe there is a good way to summarize them? |
I believe this to be one of the (many?) things blocking const (Range) iterators.
If this is to be merged maybe that should wait until- Done#![feature(const_trait_impl)]no longer needs#![allow(incomplete_features)]?