Skip to content
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

Add more const constructors and various convenience functions to Number for generic conversions #47

Merged
merged 10 commits into from
Dec 13, 2024

Conversation

danlehmann
Copy link
Owner

@danlehmann danlehmann commented Aug 25, 2024

In particular:

  • from_u8, from_u16, ..., from_u128 which allow creating an arbitrary int without type conversion, e.g. u5::from_u32(123)
  • from_() which allows any Number argument to be passed through generics
  • as_() which easily converts any Number to another
  • as_u8(), as_u16() for more control (and to implement the others)

With const_convert_and_const_trait_impl enabled (which the Rust compiler broke years ago), the trait functions do not work, so as_(), from_() and masked_new() are not available

@danlehmann danlehmann requested a review from mist64 August 25, 2024 07:40
@danlehmann danlehmann force-pushed the more_ways_to_convert_with_simple_types branch 4 times, most recently from 3e1744b to 5c1aaa2 Compare August 25, 2024 07:57
src/lib.rs Outdated
fn try_new(value: Self::UnderlyingType) -> Result<Self, TryNewError>;

fn value(self) -> Self::UnderlyingType;

#[cfg(not(feature = "const_convert_and_const_trait_impl"))]
fn new_<T: Number>(value: T) -> Self;
Copy link

Choose a reason for hiding this comment

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

What's the role of the trailing underscore here? Just to differentiate from ::new?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Pretty much. Unfortunately, new is still more useful when dealing with literals, e.g. u5::new(1) works but u5::new_(1) doesn't, as Rust will treat 1 as an i32. So you'd need u5::new_(1u32) which is verbose.

src/lib.rs Outdated
}

fn as_u8(&self) -> u8 {
self.value as _
Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, nice, TIL!

src/lib.rs Outdated
@@ -57,11 +58,37 @@ pub trait Number: Sized {
/// Maximum value that can be represented by this type
const MAX: Self;

/// Creates a number from the given value, throwing an error if the value is too large
Copy link
Collaborator

@mist64 mist64 Aug 26, 2024

Choose a reason for hiding this comment

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

do you want to mention that the argument has to be the underlying type, i.e. the next power of two number of bits, which is fine in case your data is already exactly this, otherwise you should use new_()?

@danlehmann danlehmann force-pushed the more_ways_to_convert_with_simple_types branch 2 times, most recently from ddd894a to a75b424 Compare December 2, 2024 22:30
danlehmann and others added 8 commits December 3, 2024 14:05
…er for generic conversions

In particular:
- new_u8, new_u16, ..., new_u128 which allow creating an arbitrary int without type conversion,
  e.g. `u5::new_u32(123)`
- new_() which allows any Number argument to be passed through generics
- as_() which easily converts any Number to another
- as_u8(), as_u16() for more control (and to implement the others)
First step in making .into() more usable
@danlehmann danlehmann force-pushed the more_ways_to_convert_with_simple_types branch from c17a722 to cb6b171 Compare December 3, 2024 22:05
Comment on lines 16 to 21
# Supports const trait implementation through const_convert and const_trait_impl. Support for those was removed
# from more recent Rust nightlies, so this feature requires an older Rust compiler
# (2023-04-20 is broken, 2022-11-23 works. The exact day is somewhere inbetween)
# (2023-04-20 is broken, 2022-11-23 works. The exact day is somewhere inbetween).
# As of 12/2/2024, this also uses inline_const. This has been stabilized but is required for current code to work
# on the old compiler.
const_convert_and_const_trait_impl = []

Choose a reason for hiding this comment

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

@danlehmann With 1.3.0 being incompatible to previous versions anyway and inline_const having been stabilized in Rust 1.79.0, may it be a good time to raise the MSRV to 1.79 and just remove this optional feature?

Copy link
Owner Author

Choose a reason for hiding this comment

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

This feature is still used by https://crates.io/crates/bilge (which itself puts it behind an optional feature). While I really dislike the code duplication, I don't want to break them. I think the right time to remove the feature is when const_convert/const_trait become finalized.

In terms of breakage, I think 1.3.0 doesn't break much. I'd like to keep it at a minimum.

@danlehmann danlehmann merged commit c90fcf3 into main Dec 13, 2024
8 checks passed
@danlehmann danlehmann deleted the more_ways_to_convert_with_simple_types branch December 13, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: new_with_u32
4 participants