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

Abstracting type limits (numeric and not only). #2252

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
133 changes: 133 additions & 0 deletions text/0000-add-limits-trait.md
@@ -0,0 +1,133 @@
- Feature Name: Limits trait for the rust types
- Start Date: 2017-12-20
- RFC PR:
- Rust Issue:

# Summary
This is an RFC to add a universal trait for the type limits.
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels a bit thin, expanding this section a bit would be helpful.

Copy link
Author

Choose a reason for hiding this comment

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

Okay. :) I will expand it.


# Motivation
Copy link
Contributor

Choose a reason for hiding this comment

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

The language in this section could be improved a bit =)

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately I am not an english native speaker. Could you correct me please?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd write it as (with some slight semantic changes and clarifications and added Haskell as an example):

The motivation is simple: By providing the methods in a trait, user code is able to require with a bound X: Limit<Y> that X has certain limits of type Y, which enables generic reasoning and simplifies code.

Another motivation is that we already have inherent methods .max_value() and .min_value() on all primitive numeric types. Generalizing those methods as a trait simplifies the code and avoids duplication.

Looking at other languages, C++ provides std::numeric_limits, while Haskell provides the Bounded typeclass. This provides precedent that such a facility belongs in the standard library.

The motivation is quite simple: [make an ability to accept template types with with limits by requiring a trait](https://stackoverflow.com/questions/47904954/rust-finding-the-maximum-allowable-value-for-generic-data-type-t)
so that it simplifies and generalizes the code. Another motivation is that we have all that `max_value()` and `min_value()` implemented as
usual methods of a type implementation, generalizing this to a trait makes the code simplier and avoids duplicating code. Also, looking at
C++ template of `std::numeric_limits` tells us we must have this thing too because it is easier to use.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please elaborate on why this has to be in the standard library and not in a separate crate.

Copy link
Author

@iddm iddm Dec 21, 2017

Choose a reason for hiding this comment

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

Because if we have it standardized, the standard library types should implement it as all of them already have it implicitly by providing min_value() and max_value() methods. One more reason is that such a trait is really not enough to be a separate crate.

Are these reasons enough?

Copy link
Member

Choose a reason for hiding this comment

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

Historically no, the same way there isn't a trait for wrapping_add, next_power_of_two, rotate_left, etc, all of which are implemented already on the core types without a trait.

Copy link
Author

Choose a reason for hiding this comment

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

@scottmcm Do you think it should be in a separate crate? Could you tell why you think so please?

Copy link
Member

Choose a reason for hiding this comment

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

@vityafx Personally, I like traits for things, to be able to write stuff as generics instead of macros. But the standard library tends to be more conservative about adding traits, leaving that to the num crate and friends. So this RFC needs to address that.

Copy link
Author

Choose a reason for hiding this comment

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

I think that moving this to the num crate and removing these static methods from the std's types will cause compilation failures for everyone who uses it.

Copy link
Member

Choose a reason for hiding this comment

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

Removing, sure, but you don't need to remove the std implementations to have them on a trait: http://rust-num.github.io/num/num_traits/bounds/trait.Bounded.html

Copy link
Author

@iddm iddm Jan 22, 2018

Choose a reason for hiding this comment

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

Yes, but why do we need the implementations for the standard types then in a separate crate? And also, we talked about requiring PartialOrd and not removing these methods will violate this rule.

# Detailed design
The design is quite simple: put everything related to the limits what can be generalized into a separate trait in the standard library:

```rust
trait Limits<T> {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably be <T = Self> if we allow a type parameter T there to make the use more ergonomic.
See std::cmp::PartialEq and (a lot of) friends for precedent.

Copy link
Author

Choose a reason for hiding this comment

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

Good notice!

fn min_value() -> T;
fn max_value() -> T;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

In all of your examples below, you've used constant which is known at compile time.
As the function is like () -> T, this means that an associated constant can be used instead as shown below.
Have you considered this design and if so, why did you discard it?

trait Limits<T> {
    const MIN_VALUE: T;
    const MAX_VALUE: T;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Also.. Why a combined trait instead of one trait for min and one for max?

Copy link
Author

Choose a reason for hiding this comment

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

Nice! Don't know why I forgot we are able to do this in traits! This looks nice, but the question then: why do we have min_value() and max_value() methods for all primitive types for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

My guess is that these inherent methods precede the stabilization of associated constants, but I could be wrong.

Copy link
Member

Choose a reason for hiding this comment

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

@Centril I don't quite understand. Do you mean that before we can have a constant, we need a method? Or simply that the associated constants feature didn't exist when the methods were introduced?

Copy link
Member

Choose a reason for hiding this comment

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

Also, I would prefer MAX and MIN so we can reuse std::*::MAX, etc...

Copy link
Contributor

Choose a reason for hiding this comment

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

If there is a use case for runtime-determined (non-const) max/min values, it is perhaps best if this RFC blocks on #2237. Then you can have opt-in const-ness.

Copy link
Author

Choose a reason for hiding this comment

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

About combined trait: we may do even better: we may have Limits trait which just requires MinValue and MaxValue traits.

Copy link
Author

Choose a reason for hiding this comment

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

About function vs constants. I prefer both the flexibility and simpliness. However, neither constants nor functions have both of them:

  1. Constants are constant. They are written once and just used later. This is simple, but is not flexible: we can't write complex code for calculating it without magic.
  2. Functions are not simple but flexible: you may write both constant value and complex logic there, but as a language entity it is more complex, that just constants. The also reason why functions in a trait are worse is that if you really just want to return a constant you still can't write const fn because it is unstable yet.

So, the question is open, I guess. If I am wrong anywhere, correct me please.

Copy link
Member

Choose a reason for hiding this comment

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

My guess is that these inherent methods precede the stabilization of associated constants, but I could be wrong.

min_value and max_value were always intended to be turned into constants, but they were added pre-1.0, long before associated constants were added! We used to have a Bounded trait, but they were rolled back after we simplified the numeric API for conservative reasons before stabilizing the standard library. I think it now lives in the num crate.


#[derive(Debug, Copy, Clone)]
struct A {
field: u64,
}
impl Limits<A> for A {
fn min_value() -> A {
A {
field: 0u64,
}
}

fn max_value() -> A {
A {
field: 5u64,
}
}
}
impl Limits<u32> for u32 {
fn min_value() -> u32 {
0
}
fn max_value() -> u32 {
10u32
}
}

fn get_limits<T: Limits<T> + std::fmt::Debug>(_t: T) {
println!("Minimum value: {:?}", T::min_value());
println!("Maximum value: {:?}", T::max_value());
}

fn main() {
let a = A { field: 6u64 };
let num = 10u32;
get_limits(a);
get_limits(num);
}

```

Here we have a generalized function `get_limits` which accepts its argument with requirement for trait `Limits` implementation. As long
as a type implements this trait, this function will succeed and will produce expected results. It's worth mentioning that a type can implement
Copy link
Contributor

Choose a reason for hiding this comment

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

this function will succeed

Do you mean to say "this function will type check" or "this function will successfully compile" ?

Copy link
Author

Choose a reason for hiding this comment

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

This function will successfully compile, thanks for this :)

different limits type, not only for itself: `struct A` can have both `Limits<A>` and `Limits<u32>` implementations: we may simply add another
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the use case for Limits<A> for B where A != B ?

Copy link
Author

Choose a reason for hiding this comment

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

  1. A "shorthand" for inner type of strong types: struct Id(pub u64) and impl Limits<u64> for Id. But this is arguably since we made strong type exactly for hiding the inner type and we will violate the strong type's meaning by exposing its internal type in a trait.
  2. Well, I thought I had a reason for this, but I don't remember it. Could you think any reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

On 2. I'm afraid not =)

Copy link
Author

Choose a reason for hiding this comment

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

So as I also don't remember why I did it so (what I was thinking about during writing), we may turn it into a simple trait then.

implementation and use it in our generalized function appropriately:

```rust
impl Limits<u32> for A {
fn min_value() -> u32 {
0u32
}

fn max_value() -> u32 {
5u32
}
}


/// Will be called only if a type implements Limits with u32 value type.
fn get_limits<T: Limits<u32> + std::fmt::Debug>(_t: T) {
println!("Minimum value: {:?}", T::min_value());
println!("Maximum value: {:?}", T::max_value());
}
```

Another option is to use the [`Bounded`](http://rust-num.github.io/num/num/trait.Bounded.html) trait of `num` crate:

```rust
trait Limits {
fn min_value() -> Self;
fn max_value() -> Self;
}
```

This will reduce the ambiguity of types and in most cases will be enough too.

# How We Teach This
I think the `Limits` name of a trait is appropriate:
- `numeric_limits` is incorrect since the trait may be implemented for any type and it may be not numerical.
- `Bounds` does not seem to be appropriate (personally for me).

This feature does not involve anything into the language itself, but adds a trait into the standard library. All the primitive types
and anything else what has `min_value()` and `max_value()` methods must implement this trait. Removing the type method is not required
Copy link
Contributor

Choose a reason for hiding this comment

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

typo: what has => that has.

Copy link
Contributor

Choose a reason for hiding this comment

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

Removing the type method is not required

Did you mean: Removing the inherent methods on the types is not required ?
An inherent method is one that has the form (including with &self, &mut self, type parameters, and where clauses):

impl MyType {
    fn inherent_method(self, arguments) -> return_type {..}
}

Removing those methods would not only not be required but would also break backwards compatibility if done.

Copy link
Author

Choose a reason for hiding this comment

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

I meant removing const fn u64::min_value() -> u64 is not required because it works for me even so (I have implemented this trait and when I call Type::min_value which is a method of my trait it calls it instead of the static method of a type).

(why does it work with having both a trait implementation and a type method - I don't know).

This feature can be introduced as `Limits` trait for the generalized contexts.

# Drawbacks
I don't know why we should not do this.
Copy link
Contributor

Choose a reason for hiding this comment

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

A simple reason: This is possible as a crate. The RFC should show why this needs to be in the standard library.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, going to think how to write it there.


# Alternatives
Another option is to simply add macros for this:

```rust
macro_rules! max_value {
($type: ident) => {
$type::max_value()
}
}

macro_rules! min_value {
($type: ident) => {
$type::min_value()
}
}
```

This helps in generalizing the code too, but not in a way that the trait does.

# Unresolved questions
The trait design is arguable and is ready to accept any critic, namely what is better: generic trait or a simple one.
Copy link
Contributor

Choose a reason for hiding this comment

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

If you can show why the "generic version" is needed and what use cases are enabled by it, then we are closer to solving this question.