-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Abstracting type limits (numeric and not only). #2252
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
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm sympathetic to the core idea behind this RFC. Here are some hopefully helpful comments =)
text/0000-add-limits-trait.md
Outdated
|
||
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 | ||
different limits type, not only for itself: `struct A` can have both `Limits<A>` and `Limits<u32>` implementations: we may simply add another |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- A "shorthand" for inner type of strong types:
struct Id(pub u64)
andimpl 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. - Well, I thought I had a reason for this, but I don't remember it. Could you think any reason?
There was a problem hiding this comment.
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 =)
There was a problem hiding this comment.
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.
text/0000-add-limits-trait.md
Outdated
trait Limits<T> { | ||
fn min_value() -> T; | ||
fn max_value() -> T; | ||
} |
There was a problem hiding this comment.
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;
}
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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:
- 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.
- 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.
There was a problem hiding this comment.
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.
text/0000-add-limits-trait.md
Outdated
- Rust Issue: | ||
|
||
# Summary | ||
This is an RFC to add a universal trait for the type limits. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
text/0000-add-limits-trait.md
Outdated
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. | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
text/0000-add-limits-trait.md
Outdated
# Summary | ||
This is an RFC to add a universal trait for the type limits. | ||
|
||
# Motivation |
There was a problem hiding this comment.
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 =)
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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>
thatX
has certain limits of typeY
, 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 theBounded
typeclass. This provides precedent that such a facility belongs in the standard library.
text/0000-add-limits-trait.md
Outdated
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. |
There was a problem hiding this comment.
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.
text/0000-add-limits-trait.md
Outdated
``` | ||
|
||
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 |
There was a problem hiding this comment.
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" ?
There was a problem hiding this comment.
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 :)
text/0000-add-limits-trait.md
Outdated
- `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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
text/0000-add-limits-trait.md
Outdated
This feature can be introduced as `Limits` trait for the generalized contexts. | ||
|
||
# Drawbacks | ||
I don't know why we should not do this. |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
text/0000-add-limits-trait.md
Outdated
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> { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good notice!
On-going discussion should mainly happen in comments on the PR itself rather than the file you're proposing we merge. To the extent possible, prefer line comments and replying to them as it is the closest we can get to threaded discussion, on GitHub, which tends to be more readable as it allows us to discuss a single issue at a given place rather than mixing all issues together. Whenever you feel that you've changes to make that elaborate, change design decisions, adds alternatives, and so on, just add additional commits. |
Can associated constants be used instead? EDIT I mean this: https://doc.rust-lang.org/1.15.0/book/associated-constants.html |
Also, this RFC seems to be using the older format, rather than the newer format. |
@mark-i-m Heh, did not think the format could change somehow. Should I rewrite it using new template? |
@vityafx TBH, I don't know. I guess it would be nice if it's not too much trouble. |
Can you clarify what the expected semantics of |
@scottmcm Awesome question. But it needs a small decomposition:
Now it is difficult for me to answer these questions. The possible drawbacks in my opinion are:
What do others think? |
This feature exists in Haskell as
|
Proof by counterexample: fn main() {
fn proof_partial_ord<T: PartialOrd>() {}
proof_partial_ord::<f64>();
assert_eq!(std::f64::MIN <= std::f64::NAN, false);
assert_eq!(std::f64::NAN <= std::f64::MAX, false);
} |
Haskell doesn't define
|
So, we end the ordering question with not requiring it, am I right? |
Or by getting everyone to stop using floating point... |
@mark-i-m In any case |
I agree about not requiring Regardless whether there's a trait bound or not, the "what's the expected behaviour?" question still needs to be answered. Perhaps floating point just can't implement a general Edit: for clarity after an IRC discussion, I'm not proposing adding these properties to trait Bounded<T = Self> : PartialOrd<T> {
/// ∀x : Self, Self::MIN <= x
const MIN: T;
/// ∀x : Self, Self::MAX >= x
const MAX: T;
}
Reminder just how many things are on numeric_limits: http://en.cppreference.com/w/cpp/types/numeric_limits#Member_constants. It argues more for separate |
Okay, we stopped on requiring
|
I'd do the same. I guess we can do nothing better than that. |
Update 0000-add-limits-trait.md
I'd kinda imagine this'll go like In that vein, I'd suggest someone read up on, or even survey, related features like refinement types in languages designed for formal verification of production software, like F* (paper) and ProVerif. I donno if all their theory makes sense in a language with mutability, etc. but maybe enough does. As for floats, I do not buy @Centril argument that Anyways, I think examining F*, ProVerif, etc. will provide a far better answer to the floats question than Haskell, like maybe nobody even included floats when doing formal verification, or maybe you need refinement types than can clearly express all the allowed special float values. |
Yup. If there's one thing I learned in my explorations on numeric libraries, it's that Haskell really messed up on the Haskell 98 numeric type classes. So take what they do with a big grain of salt! |
Unfortunately, that loses uniqueness—that expression is also true if you set both |
@scottmcm This is technically not true since NaN != NaN... |
@mark-i-m But the definition didn't mention anything about |
@scottmcm what I mean is that you can have MAX = NaN and MIN = NaN and still have uniqueness because MAX != MIN. This strikes me as absolutely ridiculous, but it is technically true :P |
@mark-i-m Ah, sorry I misunderstood. I meant non-uniqueness in the sense that |
Yes, minimal and maximal elements aren't unique in a general partial order, but floating-point types are just one example of that. (Ignoring here that floating-point values aren't technically partially ordered because ¬(NaN ≤ NaN).) The floating-point types simply have no greatest and smallest value, but both NaN and +∞ are maximal values, and both NaN and −∞ are minimal values. How about creating newtypes which would be able to contain any floating-point value except NaN? I am preparing an RFC proposing a feature like that (I'm not sure when I'll be able to submit it, though). For such a type, implementing a type limit trait would be straightforward. I'd also suggest splitting limits into two traits. Some orders may have one limit, but not the other. Consider the prefix order; in Rust terms, an ordering for |
A question for those who knows the rfc process better than me: is it really bad if it will be a breaking change? I mean, if we move those implementation to a trait and remove them from the types implementations, we will require the user code to add Another way would be to deprecate using type's static methods. |
One more question: I'd agree with using associated constants if it were able to use methods/functions for their initialization. Now it is impossible and even |
@vityafx
Yes, but it depends on how extensive the breakage is, and that would have to be measured with a crater run... If the breakage isn't too extensive, then it can be done with an epoch.
Not necessarily. If
That is probably a good idea. |
@Centril so what if we add it to a prelude and deprecate using of static methods? Sounds like the best option at this moment to me. |
@vityafx
I agree. You can get back the constness of the methods in
So I'd recommend taking a look at the other things that are in the prelude. In particular, the documentation states:
Does |
Well, I agree.
Should I wait for the #2237 being merged before continuing? |
MIRI is coming soon, so I think sticking to
There's no requirement to remove them from the types' implementations just because the trait exists. They shouldn't do different things, because that would be confusing, but the inherent method will take priority in resolution, so there's no need for a breaking change discussion here. And there's no reason to remove a method from, say, |
@scottmcm what is "MIRI"? :) |
@vityafx
I don't think that is necessary, and #2237 will probably not be merged in its current form, I'll have to rework it.. Whether or not
|
Totally agree with this and I told this a month ago here. So, I can't guarantee that someone would want the value to be evaluated at runtime. So I'd use the most generic solution which can be tuned to be computed both at compile time and runtime. Could you tell how this can help? Don't quite understand.. |
Can you give an example with runtime limits, so we have something concrete to discuss?
I think all that was meant by the miri-comment is that miri allows you to compute many things that were previously out of scope of const eval and needed to be done at runtime. It doesn't help for inherently runtime things. |
The thing is I can't be responsible for each user. I am not a god. Even if I don't provide an example, I can't guarantee there is no case for that. Could you? |
I think the value of putting limits in types diminishes greatly when they're not |
It's the new const evaluation system in rustc. It's been under construction for a while, but it's now making it into nightly: rust-lang/rust#46882 |
Why should we *not* do this? | ||
|
||
# Rationale and alternatives | ||
[alternatives]: #alternatives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This RFC could live entirely in a crate, could it not? Or is there a use case where libstd could make use of it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanted to get rid of u64::max_value() methods and for other types, because it will duplicate the functionality. Getting rid of these types and making limits be a separate crate is a breaking change. Also, it is not clear for me, if it is a separate crate, who owns it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, I have not finished the proposal yet, because there are questions still.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vityafx We don't have to settle all questions now. Most RFCs do have an "unresolved questions" section for questions that should be decided before stabilization but after we have some experience using a feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I may go with my own vision and just put all the unresolved questions in the appropriate section of the rfc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, generally we want some kind of consensus about what we want to try, and part of that is reaching consensus about what questions we think can be resolved later... A large part of that is the OP's (your) original vision, but most RFCs do take a few edits and changes along the way :)
There is of course a small ergonomic cost to having both, since the const uses are now |
Sure; On scale I think I can personally live with that small cost =) |
This RFC proposes to add a unified way of introducing type limits, for both numeric and non-numeric types (custom).
Rendered