nutype icon indicating copy to clipboard operation
nutype copied to clipboard

Add const getter for Copy types

Open helgoboss opened this issue 1 year ago β€’ 4 comments

I want to propose optionally adding a const getter method to newtypes that wrap a Copy value.

Motivation

Given the following newtype:


/// Represents a tempo measured in beats per minute.
#[nutype(
    new_unchecked,
    validate(finite, greater_or_equal = 1.0, less_or_equal = 960.0),
    derive(
        Copy,
        Clone,
        Eq,
        PartialEq,
        Ord,
        PartialOrd,
        Debug,
        Default,
        Display,
        FromStr,
        Into,
        TryFrom
    ),
    default = 1.0
)]
pub struct Bpm(f64);

impl Bpm {
    /// The minimum possible value.
    pub const MIN: Self = unsafe { Self::new_unchecked(1.0) };

    /// The maximum possible value.
    pub const MAX: Self = unsafe { Self::new_unchecked(960.0) };
}

(Side note: Being able to call Self::new_unchecked from a const context needs this)

Now I would like to do things like this:

const BPM_SPAN: f64 = Bpm::MAX.into_inner() - Bpm::MIN.into_inner();

Above code is currently not possible because into_inner() is not const:

            pub fn into_inner(self) -> #inner_type {
                self.0
            }

Thoughts

Just making it const in general unfortunately doesn't work. Because it takes self by value (which is the correct choice for a method of this name). And that fails to compile if the wrapped type is heap-allocated ("the destructor for this type cannot be evaluated in constant functions").

I see 2 possibilities.

a) Making into_inner const, but only for Copy types

            pub const fn into_inner(self) -> #inner_type {
                self.0
            }

b) Introducing a separate const get method, but only for Copy types

            pub const fn get(self) -> #inner_type {
                self.0
            }

Personally, I prefer b. There are quite many examples in the standard library where a wrapper exposes its wrapped Copy value in a get method, so it appears to be a good practice. But ultimately I don't mind.

Do you think it's somehow possible to do a or b? Just for Copy types? And if yes, would you consider it? I would be happy to write a PR.

helgoboss avatar Feb 20 '24 21:02 helgoboss

Addition: I have the feeling that it would be hard doing something special just for Copy types. If this is too hard or even impossible, adding the const getter just to integers and floating point wrappers would be the alternative - that would cover most of the use cases I guess.

helgoboss avatar Feb 20 '24 21:02 helgoboss

Here's a prototypical implementation that adds pub const fn get(self) -> #inner_type if the newtype itself derives Copy. What do you think about this approach? I could turn it into a proper PR including tests if you are fine with it.

https://github.com/helgoboss/nutype/commit/e186e09268b5f411d502b0afaa6678532aabf970

helgoboss avatar Feb 20 '24 22:02 helgoboss

@helgoboss Hi, thanks for your report!

I think it's partially overlaps with https://github.com/greyblake/nutype/issues/35

The problem with adding new method .get() is that it typically returns a reference, not a value itself. I don't also want to have some extra methods for not real reason. If you need a reference, you could use AsRef trait, but year it won't work with const I guess.

So I'd prefer rather option A, meaning implementing const fn into_inner() for integer and float types. If you want you can go ahead and open a PR for that. Just please make sure to add proper tests.

greyblake avatar Feb 25 '24 10:02 greyblake

Ok, I understand. I hope to open a PR soon.

helgoboss avatar Feb 26 '24 15:02 helgoboss