Skip to content

Constify implementations of (Try)From for int types #86840

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.

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

Merged
merged 3 commits into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions library/core/src/convert/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Copy link
Member

Choose a reason for hiding this comment

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

Stability attributes do not usually work on trait impls. Do they work for impl const?

Copy link
Member

Choose a reason for hiding this comment

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

Yes. See this.

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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?

Copy link
Member

Choose a reason for hiding this comment

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

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.

impl const From<$Small> for $Large {
// Rustdocs on the impl block show a "[+] show undocumented items" toggle.
// Rustdocs on functions do not.
#[doc = $doc]
Expand Down Expand Up @@ -172,7 +173,8 @@ impl_from! { f32, f64, #[stable(feature = "lossless_float_conv", since = "1.6.0"
macro_rules! try_from_unbounded {
($source:ty, $($target:ty),*) => {$(
#[stable(feature = "try_from", since = "1.34.0")]
impl TryFrom<$source> for $target {
#[rustc_const_unstable(feature = "const_num_from_num", issue = "87852")]
impl const TryFrom<$source> for $target {
type Error = TryFromIntError;

/// Try to create the target number type from a source
Expand All @@ -190,7 +192,8 @@ macro_rules! try_from_unbounded {
macro_rules! try_from_lower_bounded {
($source:ty, $($target:ty),*) => {$(
#[stable(feature = "try_from", since = "1.34.0")]
impl TryFrom<$source> for $target {
#[rustc_const_unstable(feature = "const_num_from_num", issue = "87852")]
impl const TryFrom<$source> for $target {
type Error = TryFromIntError;

/// Try to create the target number type from a source
Expand All @@ -212,7 +215,8 @@ macro_rules! try_from_lower_bounded {
macro_rules! try_from_upper_bounded {
($source:ty, $($target:ty),*) => {$(
#[stable(feature = "try_from", since = "1.34.0")]
impl TryFrom<$source> for $target {
#[rustc_const_unstable(feature = "const_num_from_num", issue = "87852")]
impl const TryFrom<$source> for $target {
type Error = TryFromIntError;

/// Try to create the target number type from a source
Expand All @@ -234,7 +238,8 @@ macro_rules! try_from_upper_bounded {
macro_rules! try_from_both_bounded {
($source:ty, $($target:ty),*) => {$(
#[stable(feature = "try_from", since = "1.34.0")]
impl TryFrom<$source> for $target {
#[rustc_const_unstable(feature = "const_num_from_num", issue = "87852")]
impl const TryFrom<$source> for $target {
type Error = TryFromIntError;

/// Try to create the target number type from a source
Expand Down
1 change: 1 addition & 0 deletions library/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
#![feature(const_slice_from_raw_parts)]
#![feature(const_slice_ptr_len)]
#![feature(const_swap)]
#![feature(const_trait_impl)]
#![feature(const_type_id)]
#![feature(const_type_name)]
#![feature(const_unreachable_unchecked)]
Expand Down
2 changes: 2 additions & 0 deletions library/core/tests/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#![feature(const_ptr_read)]
#![feature(const_ptr_write)]
#![feature(const_ptr_offset)]
#![feature(const_trait_impl)]
#![feature(const_num_from_num)]
#![feature(core_intrinsics)]
#![feature(core_private_bignum)]
#![feature(core_private_diy_float)]
Expand Down
25 changes: 25 additions & 0 deletions library/core/tests/num/const_from.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#[test]
fn from() {
use core::convert::TryFrom;
use core::num::TryFromIntError;

// From
const FROM: i64 = i64::from(1i32);
assert_eq!(FROM, 1i64);

// From int to float
const FROM_F64: f64 = f64::from(42u8);
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, we should really work towards getting rid of "things that are allowed in const but forbidden in const fn"...

assert_eq!(FROM_F64, 42f64);

// Upper bounded
const U8_FROM_U16: Result<u8, TryFromIntError> = u8::try_from(1u16);
assert_eq!(U8_FROM_U16, Ok(1u8));

// Both bounded
const I8_FROM_I16: Result<i8, TryFromIntError> = i8::try_from(1i16);
assert_eq!(I8_FROM_I16, Ok(1i8));

// Lower bounded
const I16_FROM_U16: Result<i16, TryFromIntError> = i16::try_from(1u16);
assert_eq!(I16_FROM_U16, Ok(1i16));
}
2 changes: 2 additions & 0 deletions library/core/tests/num/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ mod u64;
mod u8;

mod bignum;

mod const_from;
mod dec2flt;
mod flt2dec;
mod int_log;
Expand Down