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

RFC for trait bounds on generic parameters of const fns #8

Merged
merged 26 commits into from Feb 5, 2019

Conversation

@oli-obk
Copy link
Collaborator

commented Oct 5, 2018

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:

@gnzlbg

This comment has been minimized.

Copy link

commented Oct 12, 2018

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.

@gnzlbg

This comment has been minimized.

Copy link

commented Oct 12, 2018

Would it make sense to make it obvious in the bound that the impl must be const ? For example using T: const impl Foo ? We might want to allow requiring const impls in non-const functions in the future:

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
}
@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 12, 2018

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

@alexreg

This comment has been minimized.

Copy link

commented Oct 23, 2018

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.

@RalfJung

This comment has been minimized.

Copy link
Collaborator

commented Oct 31, 2018

I like most of it, but I think I'd prefer this alternative:

One could require no const on the bounds (e.g. T: Trait) and assume constness for all bounds. An opt-out via T: ?const Trait would then allow declaring bounds that cannot be used for calling methods. This design causes discrepancies with const fn pointers as arguments (where the constness would be needed, as normal function pointers already exist as the type of constants). Also it is not forward compatible to allowing const trait bounds on non-const functions

The thing is, you already kind-of have all the complexity of this alternative in your proposal when you say

The const requirement is propagated to all bounds of the impl or its methods,

I find it rather inconsistent to do automatic propagation on const bounds on const impl but not on const fn.

Moreover, I assume that your triple_add is callable at run-time even with a non-const impl, right? So the bound is also somewhat misleading.

@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 2, 2018

I find it rather inconsistent to do automatic propagation on const bounds on const impl but not on const fn.

True. We could require manually annotating everything inside const impls for now to stay consistent and forward compatible.

@eddyb

This comment has been minimized.

Copy link

commented Nov 4, 2018

I don't think this provides a workable solution for deriving (short of #[derive(const Clone)] - which I don't hate, but I don't know if it fits in with everything - cc @petrochenkov @alexcrichton).

But also, I think it should be stated more clearly what the difference is between const fn callers versus runtime fn callers, and how supertraits and implied bounds interact with this proposal.

@eddyb

This comment has been minimized.

Copy link

commented Nov 4, 2018

Another drawback to keep in mind is the syntactic similarity to const X: T params.

## Drop

Should we also allow `(SomeDropType, 42).1` as an expression if `SomeDropType`'s `Drop` impl
was declared with `const impl Drop`?

This comment has been minimized.

Copy link
@eddyb

eddyb Nov 4, 2018

Yes! That's literally what it's been waiting for 🎉

@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Nov 5, 2018

I don't think this provides a workable solution for deriving (short of #[derive(const Clone)] - which I don't hate, but I don't know if it fits in with everything

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 Clone impl for Bar const, and internally changed the const impl Clone for Foo to impl Clone for Foo, we'd be breaking users of the crate that used Bar::clone in const contexts. This is exactly why we have const fn instead of inferring the constness from the body: semver compatibility.

@eddyb

This comment has been minimized.

Copy link

commented Nov 5, 2018

@oli-obk Right. Can you add to the RFC that an opt-in for deriving is needed?
My first choice was #[derive(const Clone)], but it's bikeshed material I think.

@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 14, 2018

@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?

@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 15, 2018

@scottmcm brought up a point on discord:

We have unsafe impl Foo for Bar {}, which means that it's an impl for an unsafe trait Foo, not that all methods being implemented are unsafe.

In contrast to that, unsafe fn means the function can only be called in unsafe blocks and const fn means the function can be called in const contexts.

So const impl Foo for Bar {} gives a new meaning to a modifier in a place where another modifier has a different meaning.

I am changing this RFC to use impl const Foo for Bar {}, which has the nice symmetry of having the const infront of the trait name like in bounds (T: const Trait).

oli-obk added 2 commits Dec 15, 2018
@scottmcm

This comment has been minimized.

Copy link

commented Dec 18, 2018

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 std::cmp::Rev impl Ord or const Ord appropriately and without breaking anyone?

# Unresolved questions
[unresolved-questions]: #unresolved-questions

## Runtime uses don't have `const` restrictions?

This comment has been minimized.

Copy link
@oli-obk

oli-obk Dec 19, 2018

Author Collaborator

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.

@Pratyush

This comment has been minimized.

Copy link

commented Dec 24, 2018

In this design, how would I go about annotating specific functions within a trait as const? Is there some way to do the following?

trait Foo {
    const fn do_stuff(a: usize) -> usize;
}

const fn bar<T: Foo>(a: usize) -> usize {
    T::do_stuff(a)
}
@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 27, 2018

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.

@Pratyush

This comment has been minimized.

Copy link

commented Dec 27, 2018

Thanks for the clarification. The motivation for this is a scenario where you want some items in a trait to be const, but not necessarily others, and all reasonable impls of the trait would be able to satisfy const-ness for the relevant items. I have some code that could benefit from this sort of fine-grained const-ness.

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 const bound in T: Foo.

@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 28, 2018

Would the above work? Note the lack of a const bound in T: Foo.

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 const bound for the trait where you actually need the bounds:

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)
}
@varkor

This comment has been minimized.

Copy link
Contributor

commented Jan 8, 2019

I have yet to thoroughly read through the RFC/discussions, but from a naïve intuitive perspective, impl const Trait doesn't seem like the right abstraction when we think about const type theoretically. I'll just jot down some notes and try to elaborate afterwards.

First, we have an existing basic notion of types in Rust:

  • We have a universe/set of types (called Type or * or whatever) that we can roughly split into "function types" and "non-function types" (that is, Type is a category).
  • Traits are type-level functions: they take a type (Self) and return a type, which is the signature of the trait with Self specialised to the input type.
  • Implementations of traits are values of traits applied to types. E.g. impl Trait for Foo defines a value, <Foo as Trait>, which has the type Trait(Foo). (I'm overloading notation here a little, but hopefully it's clear what I mean.)

Now we want to consider const (in its various forms). What do we want from a model of const?

  • There are strictly more fns than const fns (because every const fn can be used as a normal function, but there are some functions inexpressible as const fn). So the set of const fns must embed into the set of fns.
  • We must be able to convert a const fn to a fn (as pointed out above). (It doesn't matter here whether that's implicit or explicit.)

It makes sense then to consider a new universe/category of types, const Type, which is analogous to Type, but with const types (whatever they are) and const functions. This is a smaller universe than Type. We have an embedding map (strictly speaking, a functor) taking const types to their "runtime" equivalents and const functions to their runtime equivalents.

This embedding is strictly one-way. We can't go from a runtime type to a const type. Built-in types like u8, (), bool, etc. are const types, and we can use them in const fn and normal fns. An example of something that isn't a const type would be a type making use of something we know isn't const: e.g. a pointer to a non-const function.

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, const Type. They take a const type and return a const type. Implementations of const traits are values of applications of const traits.

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: A: const Trait doesn't make sense, because traits are inherently either const or not-const: the constness is part of the Trait definition itself. It doesn't make sense to switch between them.

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. const can be quite subtle, and we want to make sure we get this design right.

@varkor

This comment has been minimized.

Copy link
Contributor

commented Jan 15, 2019

To clarify my comment #8 (comment), I think if you want a const default method implementation that doesn't force implementers to const-implement that method, you should have to use specialisation:

trait Trait {
	fn foo();
}

default impl<T> Trait for T {
	const fn foo() { ... }
}

The problem with adding const to mean const-default in trait signatures is that it's solely an implementation detail, not a signature detail, so it makes what traits represent blurry (this is already true in some respect for default implementations, but this can be explained as syntactic sugar). You can either use const in traits to refer to signature or implementation and I think it's important that we keep const signatures in traits free for potential use in the future, as they're a really clean way to deal with abstraction.

This means const default is dependent on default being stabilised first, but this gives us some leeway to play with the design later. I think it would be very unfortunate to lose the opportunity to define const signatures in the future, and I think it's unintuitive with respect to the model of a trait.

@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 15, 2019

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 Iterator, because we'd end up with a breaking change.

@varkor

This comment has been minimized.

Copy link
Contributor

commented Jan 15, 2019

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.

That's definitely the ideal solution. It's just a bit tricky to nail down the syntax.

IIRC that's also not how specialization works, you either specialize all methods, or none.

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 default impl). This is what makes default trait method definitions sugar for specialisation (e.g. as described in this section of the specialisation RFC).

@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 15, 2019

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 default impl). This is what makes default trait method definitions sugar for specialisation

Neat, I did not know that.

I went with the attribute and added the syntax question to the unresolved questions.

@gnzlbg gnzlbg referenced this pull request Jan 22, 2019
@gnzlbg

This comment has been minimized.

Copy link

commented Jan 22, 2019

One of the alternatives in rust-lang/rfcs#2626 could benefit of enforcing that a trait method is always a const fn on all implementations.


2. Opt out bounds are ugly

I don't think it's either intuitive nor readable to write the following

This comment has been minimized.

Copy link
@RalfJung

RalfJung Jan 22, 2019

Collaborator

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?

This comment has been minimized.

Copy link
@oli-obk

oli-obk Jan 23, 2019

Author Collaborator

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.

This comment has been minimized.

Copy link
@RalfJung

RalfJung Jan 23, 2019

Collaborator

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?

This comment has been minimized.

Copy link
@oli-obk

oli-obk Jan 23, 2019

Author Collaborator

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.

This comment has been minimized.

Copy link
@RalfJung

RalfJung Jan 22, 2019

Collaborator

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.

This comment has been minimized.

Copy link
@oli-obk

oli-obk Jan 23, 2019

Author Collaborator

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.

This comment has been minimized.

Copy link
@RalfJung

RalfJung Jan 22, 2019

Collaborator

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.

This comment has been minimized.

Copy link
@oli-obk

oli-obk Jan 23, 2019

Author Collaborator

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.

This comment has been minimized.

Copy link
@oli-obk

oli-obk Jan 23, 2019

Author Collaborator

We could opt out of this by writing the trait def as

trait Foo {
    type Bar: ?const Add;
}
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)

This comment has been minimized.

Copy link
@RalfJung

RalfJung Jan 22, 2019

Collaborator

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.

This comment has been minimized.

Copy link
@oli-obk

oli-obk Jan 23, 2019

Author Collaborator

I don't understand, can you elaborate? Should I mention ?const earlier?

This comment has been minimized.

Copy link
@RalfJung

RalfJung Jan 23, 2019

Collaborator

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]".

This comment has been minimized.

Copy link
@oli-obk

oli-obk Jan 23, 2019

Author Collaborator

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.

This comment has been minimized.

Copy link
@RalfJung

RalfJung Jan 23, 2019

Collaborator

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.

@RalfJung

This comment was marked as resolved.

Copy link
Collaborator

commented on const-generic-const-fn-bounds.md in 9c715d7 Jan 23, 2019

Should be c: constness. We don't use "type variable" anywhere in Rust.

@oli-obk oli-obk referenced this pull request Jan 24, 2019
0 of 18 tasks complete
@oli-obk

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 5, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.