-
Notifications
You must be signed in to change notification settings - Fork 17
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
RFC for trait bounds on generic parameters of const fns #8
Conversation
From all options proposed, this appears to be the most minimal one that gets the problem solved, and it also appears to be forward compatible with all others (supporting traits with const methods, an effect system, only implementing some methods as const instead of all of them, etc.). It's a bit of a shame that all methods of an implementation have to be const, but one has to start somewhere, and even if we were to relax this in the future, we would still want "sugar" for the cases in which the user wants to implement all methods as const. So thumbs up from me. |
Would it make sense to make it obvious in the bound that the impl must be trait Bar { fn bar(&self) -> usize; }
/*not const*/ fn foo<T: const impl Bar>(x: T) {
const X: usize = <T as Bar>::bar(&x); // Ok because the impl of Bar is const
} |
Ah I have no thought about this use-case. It probably is more sensible to stay forward compatible by using the explicit annotation alternative instead of punting to an opt out in the future. I'll adjust the RFC accordingly |
Very glad to see an RFC for this finally! Looks like a good path forward to me. Effect systems (for const and other things) would be great in the future, but yes, that involves significant technical investment... this is a good compromise for now. |
I like most of it, but I think I'd prefer this alternative:
The thing is, you already kind-of have all the complexity of this alternative in your proposal when you say
I find it rather inconsistent to do automatic propagation on Moreover, I assume that your |
True. We could require manually annotating everything inside |
I don't think this provides a workable solution for deriving (short of But also, I think it should be stated more clearly what the difference is between |
Another drawback to keep in mind is the syntactic similarity to |
const-generic-const-fn-bounds.md
Outdated
## Drop | ||
|
||
Should we also allow `(SomeDropType, 42).1` as an expression if `SomeDropType`'s `Drop` impl | ||
was declared with `const impl Drop`? |
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! That's literally what it's been waiting for 🎉
There's no way around this. Imagine the following situation: struct Foo;
const impl Clone for Foo {
fn clone(&self) -> Self { Foo }
}
#[derive(Clone)]
pub struct Bar(Foo); If we automagically made the derived |
@oli-obk Right. Can you add to the RFC that an opt-in for deriving is needed? |
@Centril I think this is ready for opening a full RFC in the RFC repo. Do you want to do a review before I do this? |
@scottmcm brought up a point on discord: We have In contrast to that, So I am changing this RFC to use |
This looks pretty good; the one additional thing I'd like to see is a plan for how to write generic impls when a generic type has a impl for a const trait. For example, how do we have |
const-generic-const-fn-bounds.md
Outdated
# Unresolved questions | ||
[unresolved-questions]: #unresolved-questions | ||
|
||
## Runtime uses don't have `const` restrictions? |
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.
how do we have std::cmp::Rev impl Ord or const Ord appropriately and without breaking anyone?
This is discussed in this section. You'd write the impl as follows:
impl<T: Ord> const Ord for Reverse<T> {
#[inline]
fn cmp(&self, other: &Reverse<T>) -> Ordering {
other.0.cmp(&self.0)
}
}
compare this to the current impl in https://doc.rust-lang.org/src/core/cmp.rs.html#458-463
impl<T: Ord> Ord for Reverse<T> {
#[inline]
fn cmp(&self, other: &Reverse<T>) -> Ordering {
other.0.cmp(&self.0)
}
}
The only difference is the const
in front of Ord
. If the unresolved question about requiring const
bounds becomes canonical we'd have
impl<T: const Ord> const Ord for Reverse<T> {
#[inline]
fn cmp(&self, other: &Reverse<T>) -> Ordering {
other.0.cmp(&self.0)
}
}
If you pass a T
that is not const, you also don't get a const
impl, just like calling a const
function outside a const context gives you a regular function.
In this design, how would I go about annotating specific functions within a trait as trait Foo {
const fn do_stuff(a: usize) -> usize;
}
const fn bar<T: Foo>(a: usize) -> usize {
T::do_stuff(a)
} |
No, that is orthogonal to this RFC. Marking a function as const fn in a trait means that all impls of the trait will need to provide a const fn. While interesting, I have yet to see a case where this would be useful. This RFC does allow you to define trait Bar {
fn do_stuff(a: usize) -> usize;
}
const fn do_stuff<T: const Bar>(a: usize) -> usize {
Bar::do_stuff(a)
} which amounts to the same thing, but it's not an associated constant function. |
Thanks for the clarification. The motivation for this is a scenario where you want some items in a trait to be The RFC might actually enable something like this, but in a slightly hacky way: trait _Foo {
fn do_stuff(a: usize) -> usize;
}
trait Foo: const _Foo {
fn other_fn();
}
const fn bar<T: Foo>(a: usize) -> usize {
T::do_stuff(a)
} Would the above work? Note the lack of a |
That would not really work with just this RFC, since const bounds on trait declarations are not something discussed at all. You can still get that split by only adding a trait FooA {
fn do_stuff(a: usize) -> usize;
}
trait FooB: FooA {
fn other_fn();
}
const fn bar<T: FooB + const FooA>(a: usize) -> usize {
T::do_stuff(a)
} |
I have yet to thoroughly read through the RFC/discussions, but from a naïve intuitive perspective, First, we have an existing basic notion of types in Rust:
Now we want to consider
It makes sense then to consider a new universe/category of types, This embedding is strictly one-way. We can't go from a runtime type to a const type. Built-in types like This essentially means that const types are those that are made up solely of const types, whereas "runtime" types are those made up of const types or other runtime types (the base case for these definitions are in the functions). (It's critically important that non-const types can't make their way into const types, because otherwise we would introduce type dependency into the type system.) The crux is this: with this model, const traits are (const) type-level functions on the universe of const types, Therefore, const types, const functions and const traits all make sense as notions to consider, because the corresponding non-const notion is well-defined. But a "const impl" only makes sense if you consider it as "an implementation of a const trait". The implementation itself can't be const: it's just the definition of a value. This also applies to the bounds of a type: In summary, I don't think it makes sense to have any notion of "const impl" without first having concrete notions of "const trait". (Aside: I'm fairly sure the constness of everything but functions can be inferred without issue in a model like the one I describe, as a side point.) The RFC as it stands has very little in the way of theoretical motivation and (even if I've misread the RFC entirely and this does all work out) I think that's an important consideration. |
Can you give an example of behaviour you want, but only works if trait methods can be |
Sorry, to clarify I mean |
To clarify my comment #8 (comment), I think if you want a trait Trait {
fn foo();
}
default impl<T> Trait for T {
const fn foo() { ... }
} The problem with adding This means |
I fully agree with the motivation, but not with the solution. As long as we are usually specifying (non-const) default method bodies direclty in the trait, we should keep on doing so and do it for default const method bodies, too. IIRC that's also not how specialization works, you either specialize all methods, or none. So it would be inappropriate to use on existing traits like |
That's definitely the ideal solution. It's just a bit tricky to nail down the syntax.
I got the syntax wrong in my example (corrected now), but I think you may only specialise some methods (i.e. the ones included in the |
Neat, I did not know that. I went with the attribute and added the syntax question to the unresolved questions. |
One of the alternatives in rust-lang/rfcs#2626 could benefit of enforcing that a trait method is always a |
const-generic-const-fn-bounds.md
Outdated
|
||
2. Opt out bounds are ugly | ||
|
||
I don't think it's either intuitive nor readable to write the following |
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 is quite funny given that it behaves exactly like the ?const
trait bound. Why do you think it is intuitive or readable there, but not here?
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.
To me it feels like ?const fn() -> i32
is ?(const fn() -> i32)
and not (?const) fn() -> i32
. But I don't care very much either way. It would certainly be more consistent.
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.
To me it feels like ?const fn() -> i32 is ?(const fn() -> i32) and not (?const) fn() -> i32.
But you don't interpret ?const Trait
as ?(const Trait)
-- why?
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.
Not sure, probably because const fn
is a pattern burned into my mind by now, any variance on it is unusual.
Actually I've started to read ?const fn()
correctly over time, so it's fine, and mostly orthogonal to this RFC I'd think
|
||
The const requirement is inferred on all bounds of the impl and its methods, | ||
so in the following `H` is required to have a const impl of `Hasher`, so that | ||
methods on `state` are callable. |
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 requirement is in place only when the code is called from const context, right? The text makes it sound like just writing this impl imposes some requirements, but I don't think that is the case.
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.
Indeed, the requirement only exists for const contexts.
for it. | ||
|
||
These rules for associated types exist to make this RFC forward compatible with adding const default bodies | ||
for trait methods. These are further discussed in the "future work" section. |
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 rules would also otherwise be needed, would they not? After all, a const fn
could use these bounds to call methods on data that has the associated type.
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.
Without const default bodies, we'd only need the bounds if used by a const
method. This way we need it even if nothing uses the bound in a const way.
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.
We could opt out of this by writing the trait def as
trait Foo {
type Bar: ?const Add;
}
const-generic-const-fn-bounds.md
Outdated
These follow the same rules as bounds on `const` functions: | ||
|
||
* all bounds are required to have `impl const` for substituted types if the impl is used in a const context | ||
* except in the presence of `?const` (see below) |
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 fact that this exception is first mentioned when it comes to impl blocks makes it sound like it does not apply for what got previously discussed.
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 don't understand, can you elaborate? Should I mention ?const
earlier?
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 find the overall structure here a bit confusing. You first have some text outside any subsection that seems to give an overview, cutting across a bunch of features, and then you start to systematically explore const Trait
in some areas -- but there is no subsection specifically for "const Trait
in function bounds". Hence this is the first time you are summarizing, but you are summarizing for a new area that we didn't look at, and you are also adding a new concept while summarizing. Now it is unclear how much of this summary also applies to the previously discussed area of bounds for functions.
Or maybe this isn't supposed to be a summary? It sounds like one though, because it says "follow the same rules as [previously discussed]".
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.
It's not supposed to be a summary, but just a terse explanation of the new feature. Due to the similarity to bounds on const
functions I wanted to highlight the differences and similarities in a bullet point way.
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 the structure would be improved by extending this subsection to cover generic bounds in both fn
and impl
contexts. The rules are the same, after all.
Thanks everyone for the critical reviews and constructive commentary. I believe this RFC is ready now and I'll open a regular rust RFC some time today |
Rendered
cc @Centril @RalfJung @eddyb
Once we have gone through the process here, I'll post the final result on the regular RFC repo
Previous discussion: #1
Glossary: