Make trait methods callable in const contexts

[oli-obk]

Please remember to create inline comments for discussions to keep this RFC manageable and discussion trees resolveable.

Rendered

Related:

• Tracking issue for closed RFC 2632, impl const Trait for Ty and [const] (conditionally const) syntax (const_trait_impl) rust#67792

[juntyr]

I love it and I’m very excited to get to replace the outlandish const fn + associated const hacksworkarounds I’ve joyfully come up over the last years :D

Thank you for all of the hard work that everyone working on the various implementation prototypes and around has put into const traits!

[letheed]

I didn’t see const implementations in alternatives. Is there a reason they can’t be considered ?

Eg. const impl PartialEq for i32 { … } where only this impl is const, no changes to the trait.

[Jules-Bertholet]

Maybe this should just be ~const Drop? Drop bounds in their present form are completely useless, so repurposing them would make sense. Drop would be implemented by every currently existing type, and ~const Drop only by ones that can be dropped in const contexts.

(Overall, very impressed by this RFC. It addresses essentially all the concerns I thought I might have going in. Thank you @oli-obk and team for all your hard work!)

[oli-obk]

Yea that would be neat. But it needs an edition and giving the ppl that needed T: Drop bounds the ability to still do whatever they were doing

[Jules-Bertholet]

the ppl that needed T: Drop bounds

Are there any such people at all? Making more types implement a trait should not be breaking or require an edition, no? Unless there is some useful property (for e.g. unsafe code) that only types that are currently Drop have—and there isn’t, AFAICT. (Plus, removing an explicit Drop impl from a type is usually not considered breaking.)

[compiler-errors]

I still think it's very useful conceptually to split Destruct and Drop since the former is structural and the latter really isn't -- it's more like an "OnDrop" handler. If we moved to ~const Drop, then in order to write a well-formed ~const Drop impl, you need to write where {all of my fields}: ~const Drop in the where clause.

That is to say, there's a very good reason we split ~const Destruct out of ~const Drop in the first place :)

[Jules-Bertholet]

it's more like an "OnDrop" handler.

Yeah, that’s what it is right now, but we could expand its meaning.

in order to write a well-formed ~const Drop impl, you need to write where {all of my fields}: ~const Drop in the where clause.

The bound could always be made implicitly inferred. Drop is extremely magic already, why not a little more?

But actually, I think it’s a good thing that these bounds can be specified explicitly, because it enables library authors to leave room for adding or changing private fields in the future. I could see allowing impl Drop/impl const Drop blocks with no fn drop() method, that serve only to add restrictions on dropping in const contexts. (In today’s Rust, you could use a ZST field for this.)

[fee1-dead]

It would still be possible to change the meaning of : Drop over an edition

Right, so before we have a consensus on how that would look like, having both Destruct and Drop feels completely fine for me. Since we just want to know whether something can be dropped (T: ~const Destruct) and we know that we will accommodate any existing uses of the Drop bound and impls, any reformulating of how that works can still be done through an edition even if we choose to add Destruct here.

[workingjubilee]

Changing how random trait bounds of otherwise typical traits are presented, even over an edition, is not useful. It's not Fn, FnMut, FnOnce, or Sized. The mistake isn't that you can write a Drop bound, it's that Drop was handled by a typical trait, despite having atypical needs, and was not given special treatment to begin with. That is something you cannot simply change over an edition. Otherwise, introducing a magical special case too-late to help is not really for the best.

[Jules-Bertholet]

@workingjubilee Can you elaborate? To be clear, my suggestion is that Drop should be like a normal trait (at least in terms of its trait bounds). The “magical special case” I suggested would be only for old editions, to preserve compatibility for the small number of people relying on the current not-like-a-normal-trait behavior (where a type that satisfies the Drop bound is less capable than one that does not).

[workingjubilee]

??? Perhaps I misunderstood something?

[Jules-Bertholet]

Current Drop: trait bound satisfied only when an explicit impl exists. Such an impl must contain an fn drop(). Adding such an impl makes the type less capable.

Proposed Drop: trait bound always satisfied on new editions (like this RFC’s Destruct). Bare : Drop bounds retain their current behavior on old editions (with a warning), for compatibility. ~const Drop bounds behave like this RFC’s ~const Destruct on all editions. Conceptually: when implementing Drop manually, you override the default impl (like with an auto trait). An explicit impl may specify ~const bounds, or an fn drop() handler. Adding such a handler implicitly (a) makes the type ineligible for destructuring, and (b) unimplements auto trait TrivialDrop.

[RalfJung]

Overall I am really excited about this; with &mut out of the door, traits are the next big frontier for const. I fully agree we shouldn't block this on the async/try effects work; const is quite different since there's no monadic type in the language reifying the effect, and I also don't want to wait another 4 years before const fn can finally use basic language features such as traits.

My main concern is the amount of ~const people will have to add everywhere. I'm not convinced it's such a bad idea to make that the default mode for const fn. However that would clearly need an edition migration so it doesn't have to be part of the MVP. I just don't agree with the way the RFC dismisses this alternative.

[RalfJung]

For the reference-level section, this seems more understandable than the <T as Default>::k#host = Conditionally thing above, but maybe that's just because I have already through about the "be generic over constness" formulation quite a bit.

[Jules-Bertholet]

Does this not contradict later parts of the RFC that seem to explicitly allow const in RPIT?

[Jules-Bertholet]

Suggested change

	Methods that are declared as `(const)` on a trait can now be made `const` in an impl, if that impl is marked as `impl cosnt Trait`:
	Methods that are declared as `(const)` on a trait can now be made `const` in an impl, if that impl is marked as `impl const Trait`:

[Jules-Bertholet]

Suggested change

The implementation correctness argument is partially due to our history with `cosnt fn` trait bounds (see https://github.com/rust-lang/rust/issues/83452 for where we got "reject all trait bounds" wrong and thus decided to stop using opt-out), and partially with our history with `?` bounds not being great either (https://github.com/rust-lang/rust/issues/135229, https://github.com/rust-lang/rust/pull/132209). An opt-in is much easier to make sound and keep sound.

The implementation correctness argument is partially due to our history with `const fn` trait bounds (see https://github.com/rust-lang/rust/issues/83452 for where we got "reject all trait bounds" wrong and thus decided to stop using opt-out), and partially with our history with `?` bounds not being great either (https://github.com/rust-lang/rust/issues/135229, https://github.com/rust-lang/rust/pull/132209). An opt-in is much easier to make sound and keep sound.

[Jules-Bertholet]

This impl shouldn’t compile, right?

[Jules-Bertholet]

const_derive or derive_const?

[Jules-Bertholet]

Missing body

[Jules-Bertholet]

It’s just a future possibility, so we can always figure out these details later, but I still don’t understand why this might not be possible to do. Is it just an implementation difficulty concern?

[traviscross]

Oli said:

The RFC requires const trait Trait and impl const Trait again after discussing it with niko.

Rationale is

• Pro: When you change this, you have a "one shot" chance to adjust the effects of methods in the trait.

• Pro: More clear what your intent is, allows for better diagnostics; if you add a (const) to a method without modifying the trait, we error and have a chance to explain what's happening a bit, because that may indicate you have a confused mental model.

• Pro: Syntax mirrors usage in other places:

  • T: const Trait
  • const fn foo<T: (const) Default> and const trait Default

• Con: Redundant. (But we could always not require it.)

Regarding the marking of the trait, I'm OK with explicit syntactic indication in the current edition. In fact, I somewhat prefer it for the reasons you mention. But I see those reasons (and in particular the number 1 and 2 "pros" that you mention) as transitional, and over time, using editions, I'd like to wash out this distinction, and that leads me to prefer using an attribute for this.

If we mark the impls with const Trait, that makes a certain sense, as it does match the bound syntax, and the const in that position means something analogous. In one place, it's setting (or asserting) the value of the associated effect, and in the other, it's bounding it. That makes sense.

On the trait, though, we'd be overloading const to mean something rather different. Probably I'd frame what it's doing as "allowing const / (const) bounds on the trait and allowing the setting or asserting of the associated effect on impls of the trait." For that one, I'd rather just have an attribute to this effect and later have an attribute that disallows those things that we'd employ when flipping the default over an edition.

[oli-obk]

I'll add it to future possibilities that we can remove the const from the trait decl and turn it into a lint or sth if you didn't add any const methods

[RalfJung]

I think there's a good semantic reason to mark the trait: it indicates the presence of an associated effect variable in the trait. non-const traits do not need an associated effect variable and thus ideally shouldn't have one to avoid mysterious issues due to a hidden degree of freedom.

[traviscross]

trait Tr {
    fn f();
}

impl Tr for () {
    const fn f() {}
}

fn test1<T: Tr>() {
    // `f` can't be used as `const` in a generic context.
    const { <T as Tr>::f() }; //~ ERROR
}

fn test2() {
    // Since `f` is refined as `const` for `()`, though, we can use it
    // as `const` when we know the concrete type.
    const { <() as Tr>::f() }; //~ OK
}

If we ever accept this, which I think we should, then the most sensible way to explain that, I'd suggest, is still with associated effects and the "impls of associated types/effects are always refining" rule.

That's among the set of reasons why I don't think it's prudent for us to bifurcate the space of traits long-term.

[traviscross]

Here's the parallel, by the way, that I draw for why that should work. This works today:

trait Tr {
    async fn f();
}

impl Tr for () {
    fn f() -> impl Future<Output = ()> + Send {
        core::future::ready(())
    }
}

fn spawn<T: Send>(_: T) {}

fn test1<T: Tr>() {
    // `f(..)` can't be used as `Send` in a generic context.
    spawn(<T as Tr>::f()); //~ ERROR
}

fn test2() {
    // Since `f(..)` is refined as `Send` for `()`, though, we can use
    // it as `Send` when we know the concrete type.
    spawn(<() as Tr>::f()); //~ OK
}

[Jules-Bertholet]

Put simply, once we stabilize this, I'm planning to opt-in all of my traits and to require that in my projects for new ones.

What about traits for doing heap allocations, or I/O? For some traits, it doesn’t make sense to make them const.

[matthieu-m]

What about traits for doing heap allocations, or I/O? For some traits, it doesn’t make sense to make them const.

How do you know that the implementation is performing a system call?

For example, have a look at the Store API (#3446) which would aim at replacing the Allocator API. It was specifically designed to allow "in-line" memory, ie for [T; N] to be used as the Store type and raw memory arena both, in which case even though it's an "allocation" API, no system call is required and it should be perfectly possible to call and return from a const context.

Similarly, even if the trait is FileSystem, there's no reason that a fake/mock implementation cannot be supplied in a const context.

As such, as long as a trait can be used in a const context even if some methods cannot -- for example because they return a raw file descriptor -- there's no reason not to allow a user to provide a const implementation, and allow them to use this const implementation in a const context.

[Jules-Bertholet]

Yes, there are cases where, with extra thought and design, you might be able to design a sensible const version of the trait even in such domains. But there are also cases where you can’t (like your example of raw file descriptors, but also something as simple as “this returns a Vec”). And even where it’s possible, the trait author might choose not to do so yet, as they may not be certain of the right API, or they may want to wait for upcoming const features in the language or in their library dependencies.

[traviscross]

But it seems easier to just not allow refinement and avoid all this complexity.

People made this argument about refinement with RPITIT as well. We defended it on the basis that "RPITIT is just hidden associated types, and associated types are always refining." I don't believe that RPITIT would have been better without this kind of refinement. And having defended it with RPITIT, I'm probably especially skeptical that we should foreclose or plan to foreclose doing the same thing with hidden associated effects when there's a clear parallel.

Yes, until and unless we have more features there will be some limitations. We often choose to live with those for awhile while keeping doors open.

To the point of your example about blanket impls over wrapper types, there's no current way to express this either, even after we ship RTN:

trait Tr {
    async fn f(&self);
}

impl<T: Tr<f(..): (Copy)>> Tr for MyBox<T> {
    fn f(&self) -> impl Future<Output = ()> + (Copy) {
        (**self).f()
    }
}

And of course, until we ship RTN, even the version without conditional bounds isn't and hasn't been possible to express (without desugaring the trait away from AFIT/RPITIT, at which point you have to start writing manual Future impls to implement the trait since we haven't shipped ATPIT either).

Also wrt the default being const. The same could be said for const fn in general. So maybe these should be considered together.

These are not comparable. Flipping the default on fn means making all function bodies const contexts by default and then making runtime an explicit effect somehow. Flipping the default on trait doesn't mean anything like that, especially now that we're marking all items. It doesn't require us to (un-)invert an effect and all that entails.

One of my concerns about extending const to this position is that it overloads const in a different and unrelated way. The fact that even we're tempted to equivocate between these very different semantics in our own discussions just because of the shared keyword does not lessen my concern about this.

I think all this is out of scope of this RFC though

If we're going to require syntax to opt-in traits, we should settle that syntax in this RFC.

[RalfJung]

If we ever accept this, which I think we should, then the most sensible way to explain that, I'd suggest, is still with associated effects and the "impls of associated types/effects are always refining" rule.

I don't think so. The trait has no associated effect in this case. We just change how we typecheck trait method calls if we can already resolve to the impl, basically entirely bypassing the trait and treating this as if it was an inherent method call. I think we have to do that anyway, otherwise we can't explain e.g. unsafe refinement or weaker trait bounds.

So, accepting that code makes no statement at all about the trait, since the trait is being bypassed in the relevant call. We can still meaningfully say that this trait simply does not have an associated effect. And my feeling is still that since an associated effect is a new degree of freedom with tangible consequences, we need some syntax to indicate its presence.

If I try to phrase your position in this framework, I think you'd say that the associated effect doesn't actually have any consequences, we cannot observe it doing anything unless there's a const impl (i.e., unless there's an impl that sets the associated effect to anything non-default) and there you already agreed to having syntax? Hm... I'll have to think more about this.

[traviscross]

Oli said:

The RFC requires const trait Trait and impl const Trait again after discussing it with niko.

Rationale is

• Pro: When you change this, you have a "one shot" chance to adjust the effects of methods in the trait.

• Pro: More clear what your intent is, allows for better diagnostics; if you add a (const) to a method without modifying the trait, we error and have a chance to explain what's happening a bit, because that may indicate you have a confused mental model.

• Pro: Syntax mirrors usage in other places:

  • T: const Trait
  • const fn foo<T: (const) Default> and const trait Default

• Con: Redundant. (But we could always not require it.)

For the marking of the impl, I'm OK with impl const Trait (if we decide we want to mark impls), as it matches the bound syntax and means an analogous thing. In one place, it's setting (or asserting) the value of the associated effect, and in the other, it's bounding it. That makes sense.

Unlike with marking the trait, I don't see this one as transitional.

On the question of whether we want to mark the impls, I'll say that not doing so feels approximately equivalent to me to how one has to consider all of the fields of a type to know what auto traits that type implements, and marking it feels similar to #[derive(Copy)], for all of the good and bad that each of those entail.

Another element to consider is that if we don't mark the impl, then conceivably we could allow people to constify their impls ahead of the trait being updated. Then, those impls would automatically qualify for the bound when the trait was marked for that. I don't know how important that is. I could imagine maybe people caring who want their thing to work with both newer and older versions of some upstream crate for a trait and want their own downstreams to be able to take advantage of the const bound when using the new upstream crate version. Marking the impl means this ecosystem transition has to be serialized. Maybe or maybe not that matters, but it's something to think about.

If we were to see impl const Trait as essentially just a static assertion on the associated effect, maybe that opens another door of syntactic possibilities for this.

[Jules-Bertholet]

If we were to see impl const Trait as essentially just a static assertion on the associated effect

I’ve argued in favor of this previously, but it doesn’t strictly need to be in the MVP. However, if we want to be forward-compatible with this, we need a special case now for const Traits with no non-defaulted (const) fns—they should probably be forbidden for now. (We don’t want people relying on certain types only implementing non-const Trait, and then a rustc upgrade makes them implement const Trait also)

[Jules-Bertholet]

Can you give an example of RPIT/describe the semantics?

[oli-obk]

Please remember to create inline comments for discussions to keep this RFC manageable and discussion trees resolveable.

[traviscross]

Over in rust-lang/rust#139858 (comment), there was an ask to start a thread here about this (const) fn syntax on methods in the impl.

Requiring this (const) fn syntax in cases like the one above, where the method's constness as implemented necessarily depends on the constness of a generic from the impl header -- and consequently on the associated effect in the desugaring -- is where @nikomatsakis and I had landed back at the start of March. We had discussed it both ways, but we liked this one better in our our discussion. Among other things, we liked how this emphasizes the connection to the associated effect, and this syntax allows for copying signatures from the trait def to the impl.

Niko mentioned this in his write-up here, and I elaborated on this earlier in this thread in #3762 (comment) and in #3762 (comment). Please see those comments, particularly the first, for more details.

[oli-obk]

This I'd a copy paste error. I have never intentionally written this syntax

[oli-obk]

This is just more syntax salt solely for the purpose of some language purity. There is no meaningful difference unless we stop marking impls which would be a mess

[RalfJung]

This is just more syntax salt solely for the purpose of some language purity.

No, it is using syntax to convey a conceptual difference: is this function's constness tied to the constness of the trait (and therefore dependent on (const) bounds in the impl header) or not?

At least, that's what I understood it to do.

[oli-obk]

sure, but is this difference actually useful? We need to check whether const fn refers to a (const) fn in the trait or a const fn in the trait anyway. This is just a random thing users will need to learn without actually caring. Only from a language purity this is nice, but practically this has neither an effect on the compiler (beyond being annoying to implement) nor on users' ability to understand the code

[RalfJung]

It seems quite relevant to me to understand which bounds I have in context. A const fn cannot use (const) Trait bounds of the impl block, right?

[oli-obk]

if it was declared as (const) fn in the trait it can afaict. Or I still don't understand the TC/niko model so maybe someone else should write this RFC because I'm lost honestly and it seems bad to have someone who dislikes a model (the one allowing const, (const) and normal fns in one trait) being the one to write the RFC just to walk into mines like these.

see also my confusion in #3762 (comment)

[RalfJung]

the one allowing const, (const) and normal fns in one trait

Yeah fair, on that one I am with you, I don't think we need that for the MVP -- though we should keep it in mind as a future possibility.

[Jules-Bertholet]

It seems quite relevant to me to understand which bounds I have in context. A const fn cannot use (const) Trait bounds of the impl block, right?

If there are no bounds on the impl block, then this is a distinction without a difference. IMO we should require (const) fn only when there actually are such bounds.

[fee1-dead]

Note: A lot of the discussion around the language design is happening on Zulip, under the t-lang/effects stream. The latest major proposal discussion can be found at https://rust-lang.zulipchat.com/#narrow/channel/328082-t-lang.2Feffects/topic/All.20that's.20old.20is.20new.20again. It would be nice if before commenting on this RFC one could check if this topic has been discussed on the Zulip stream before :) When there appears to be a consensus forming from community members + the lang team, the proposal should then be updated.

[SamB]

Note: the following issue is marked deprecated (and locked)

• Tracking issue for closed RFC 2632, impl const Trait for Ty and [const] (conditionally const) syntax (const_trait_impl) rust#67792

It says the tracking issue for the rewrite is:

• Tracking Issue for removing impl const and ~const in the standard library rust#110395

but that obviously only tracks the removal of the old version.

I don't really know what the feature is supposed to be, but it sorta seems like this RFC should have a new tracking issue, and maybe the old one should link to the new one instead of to the removal tracking issue?

[RalfJung]

This RFC will get a new tracking issue once it gets accepted.

[oli-obk]

the jury is still out on this one, as similarly to const fn there is no benefit of choosing [const] trait over const trait, as the latter can't mean anything different

[fee1-dead]

Yeah, that's also true

[farnz]

This looks like a typo - you talk below (consistently) about derive_const, but this is const_derive.

[clarfonthey]

Going to share this comment I made here: rust-lang/rust#90080 (comment)

Specifically, I'd like to point out that rust-lang/rust#67792 is a currently locked issue still being added as a tracking issue for several things depending on this RFC, with the caveat that "a new issue will be opened when the RFC is accepted."

We all know that this RFC is going to be accepted in some form or another, just, not sure what its final form will be. There are currently plenty of discussions happening in the background behind closed doors that have been influencing implementations (for example, rust-lang/rust#139858 mentions that the lang team decided on a new syntax but there is no link to where that discussion happened), and currently, they are all stuck on this old, locked tracking issue where nobody else is allowed to participate except those directly under the rust-lang org.

This should be resolved properly. Either, there needs to be a consensus that new stuff should not be done with the const traits (syntax, making traits const, etc.), or it should be acknowledged that things are being done under the assumption this RFC will be accepted in some form, and either the old tracking issues should be unlocked, or new ones should be made.

EDIT: I tried to follow your "only threadable discussions", "reminder" (it's not a reminder, it's a request, since this is not how the GitHub UI is designed to be used), but apparently leaving a review without linking it directly to a line in the source doesn't accomplish this. Sorry; feel free to create a new "thread," or just resolve this in a single comment so there's no need to have a thread.

[traviscross]

There are currently plenty of discussions happening in the background behind closed doors that have been influencing implementations (for example, rust-lang/rust#139858 mentions that the lang team decided on a new syntax but there is no link to where that discussion happened)

When @oli-obk said, over in rust-lang/rust#139858, that,

Afaict the syntax with which to move forward was settled as much as it could be in the last lang team design meeting. The lang team wants to get hands-on experience with the feature to see how it feels, so this PR is ready to be merged now.

he was referring to:

• Design meeting: "Marker effects" lang-team#317

In that meeting, we didn't decide on the syntax. But, as Oli said, we need hands-on experience here, and along with the new proposal presented in that meeting and the lack of immediate strong objections, that's enough to move forward the experimental implementation work, as Oli did.

The doors aren't closed. Our meetings are open, and we keep extensive minutes that you can find linked from there.

[RalfJung]

Nevertheless, having proper tracking issues would be helpful. The current situation of reusing either 5-year old locked issues or 5-year-old closed issues as tracking issues is very confusing.

[traviscross]

Agree of course we should properly track things. In my view, rust-lang/rust#67792 is still the proper tracking issue for this work (as with many of our tracking issues, of course, it could use a lot of love to be brought up to date). I've gone ahead and unlocked it. I'd expect the reason we needed to lock it was temporary and hopefully the need for that has passed. If that's not the case, we can always lock it again.

[clarfonthey]

The doors aren't closed. Our meetings are open, and we keep extensive minutes that you can find linked from there.

I should clarify, I wasn't saying that the information wasn't technically available, just that it wasn't immediately obvious where to look. For example, even among lang team meetings, it's not immediately clear which one it took place in, and the more time that passes between when a decision was made and when the decision is noticed by someone else, the harder it is to find which specific discussion occurred if it's not linked. It's entirely plausible that the discussion happened multiple meetings before the PR and it was only at that point where someone got around to implementing it.

This is kind of the reason why tracking issues exist: instead of linking every micro-discussion that happened elsewhere when it comes to important changes with respect to a thing, the unified tracking issue tracks any relevant decisions that happened. Precisely so you don't have to look through a bunch of irrelevant discussions in meeting notes, you can just have a bullet point in the tracking issue pointing out a PR that was made, which mentions in its description that it was after a discussion in a lang team meeting.

(Thank you for unlocking the issue, by the way; I'm mostly just adding a bit more context since I'm not 100% sure my point was clear.)