rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.35k stars 12.72k forks source link

Partially const traits #103265

Open the8472 opened 2 years ago

the8472 commented 2 years ago

102225 attempts to constify Iterator which would be a stepping stone towards for _ in iter loops in const fns.

The trouble there is that #[const_trait] requires that all trait methods are const and Iterator has an ever-growing set of methods which now all would have to be constified even though we only really need Iterator::next to be const to enable iterator loops. And not only do those methods themselves have to be const but their return types must be const-constructable. So this property is quite viral.

Since most of the const trait syntax is still experimental and full of placeholders this may result in an unnecessarily large maintenance burden for the standard library. Additionally it can also hinder future API evolution because it rules out adding impossible-to-constify methods.

From a libs perspective partially const traits would allow us to constify the minimal necessary API surface to unlock further experimentation.

Prior zulip discussion in which the const_trait hackmd was brought up. My reading of it suggests that partially const traits were primarily removed due to open design questions, not due to fundamental impossibility.

oli-obk commented 2 years ago

They were removed due to, once a trait is public and stable, it being a breaking change to add a non-const defaulted method to a trait. Just like adding a non-defaulted method to a trait is a breaking change today.

That said, until we stabilize the constness of Iterator, this is not an issue. We could add an attribute that can be added to defaulted methods that makes them not const (and thus also not require their bodies to be const).

We originally wanted to avoid this because it complicates the logic and is incompatible with the const system rewrite currently in progress https://github.com/rust-lang/rust/pull/101900 without resorting to a few moderately complex hacks.

As an experimental flag for moving things along, I think this is doable.

Fwiw: we never actually supported this in previous iterations. What was previously supported was to force all implementations of the trait to supply a const impl for the defaulted methods that weren't const. This would not really help here, as we'd have to come up with a const method body at each impl const Iterator instead of once at the trait.

We could for now also just not return constified iterators from things like map, thus allowing invoking it in const eval, but not using the result. That should probably alleviate the "infectious" part of the issue, even if all method bodies must still be const.

the8472 commented 2 years ago

That said, until we stabilize the constness of Iterator, this is not an issue. We could add an attribute that can be added to defaulted methods that makes them not const (and thus also not require their bodies to be const).

Yeah, that'd be a start to let the PR move forward, but eventually a proper solution will be needed for stabilization.

They were removed due to, once a trait is public and stable, it being a breaking change to add a non-const defaulted method to a trait. Just like adding a non-defaulted method to a trait is a breaking change today.

Counter-example: in dyn traits you can have non-object-safe methods with a where Self: Sized bound, Iterator has a bunch of those. By that analogy the non-const functions shouldn't be available from a const context even on a const trait. So how would they break const contexts?

We could for now also just not return constified iterators from things like map, thus allowing invoking it in const eval, but not using the result. That should probably alleviate the "infectious" part of the issue, even if all method bodies must still be const.

It at least requires const constructors for those types since the default impls of those methods have to be able to construct them. This already makes up a portion of the PR

oli-obk commented 2 years ago

in dyn traits you can have non-object-safe methods with a where Self: Sized bound, Iterator has a bunch of those. By that analogy the non-const functions shouldn't be available from a const context even on a const trait. So how would they break const contexts?

I'll have to think on this. T: ~const Trait implies T: Trait so it's not as simple as adding some bound to all non-const methods. I also don't know what bound could be added to next that would only permit next to be called from const contexts.

I guess we'd want a negative trait bound on all nonconst methods. Like Self: ! ~const Iterator (lots of sigils, but not user-writable syntax anyway). I'll check if the trait solver can actually do that

cuviper commented 2 years ago

It might make more sense as an opt-in, like if trait Iterator wrote ~const fn next() and nothing else, then that method would be the only thing where I: ~const Iterator could use. Implementors would have to match the same level of constness to join in, and we'd need to spell out what that means for stability / breaking changes.

the8472 commented 2 years ago

Additional methods will likely be made const later, but at least in the Iterator case all of them would have default bodies and a const context could fall back to those.

oli-obk commented 2 years ago

I will give this a shot after #101900

fee1-dead commented 1 year ago

Here are some more thoughts:

First, let's consider a partially const trait.

trait Foo {
    ~const fn a() {} // const fn - must be const in a `const` implementation.
    fn b() {} // non const
}

Because the absence of ~const on b means that it is not allowed to be called in const contexts, its defaulted body and downstream implementations could all be non-const.

But a downstream implementation that looks like this reveals a deeper issue:

impl Foo for Bar {
    ~const fn a() {}
    fn b() { /* some code here.. */ }
}

The problem arises from here that promoting b on Foo to a ~const fn is now a breaking change. Even if we switched to the defaulted method on the Foo trait after this change, it would still be very surprising.

In addition, to allow this promotion to ~const not be a breaking change, we must allow examples such as this:

trait AAAA {
    ~const fn lol() { /* breaking your computer.. */ } 
}

impl AAAA for IHaveNoIdeaWhatIAmDoing {
    fn lol() {
        // innocent code owo
    }
}

To fall back to the ~const default code when called in a const context. Now there is a way to mitigate the example above, and that is to differentiate the two behaviors (one in const context vs when in runtime) such that existing uses cannot observe any change in behavior should we upgrade to ~const fn. But this opens up a hole in the language which feels really sloppy.

the8472 commented 1 year ago

If/when we have super calls then a caller using the default impl instead of the overridden one would be less surprising.

fee1-dead commented 1 year ago

If/when we have super calls then a caller using the default impl instead of the overridden one would be less surprising.

No? The problem is that we need to ensure that changing a trait function from fn a to ~const fn a is not a breaking change in partially const traits.

Downstream implementations might not even have const implementations because they could be written before that change. Therefore, we would have to either panic if that function is called in compile time, or magically rewrite to the const default body if impl body is non-const.

Now (1) does not need to be a language feature; I said on the iterator PR that this could be achieved using const_eval_select.

(2) would be a big hole in the language. Because we can't distinguish between functions that have just transitioned from non-const to const from functions that have been const since the beginning. That means any downstream implementation that specifies a non-const body will always redirect to the upstream const default body.

Sure, we could make a diagnostic for this, e.g. a hard warning, but that doesn't make sense in terms of language design. If we are warning users instead of erroring when we know 100% sure that the user did not mean what they have written, then we have failed as a language.

cuviper commented 1 year ago

The problem arises from here that promoting b on Foo to a ~const fn is now a breaking change. Even if we switched to the defaulted method on the Foo trait after this change, it would still be very surprising.

I agree that this would be a breaking change, and I don't think it should fall back to the defaulted method. So that means that once a trait is partially ~const-ified at all (with #[stable] in the case of the standard library), its current methods will be frozen in their ability to be const or not. New methods could still be added ~const right away.

That sounds painful, but it's still more flexible than the alternative where a trait is all-or-nothing const.