rust-lang / rust

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

Refining generic bounds in trait method #129251

Open obi1kenobi opened 4 weeks ago

obi1kenobi commented 4 weeks ago

This issue documents an aspect of #100706 (refined trait implementations) that to my knowledge has not been previously discussed or documented: refining generic bounds on trait methods.

It also documents the current rustc behavior, which seems to be quite surprising based on conversations I've had with a half dozen very experienced Rustaceans.

Code example

Consider the following setup in a fictional upstream crate:

pub trait Super {}

mod private {
    pub trait Marker: super::Super {}
}

pub trait Subject {
    fn method<IM: private::Marker>(&self);
}

In this case, rustc allows both of the following generic type refinements with no errors or warnings:

struct First;
impl upstream::Subject for First {
    fn method<IM: upstream::Super>(&self) {}
}

struct Second;
impl upstream::Subject for Second {
    fn method<IM>(&self) {}
}

playground

Surprising current behavior

In the above code, First refines the generic bound from IM: upstream::private::Marker to IM: upstream::Super, a supertype of Marker. Second refines the bound away completely, allowing any type.

It's quite surprising that a bare <IM> generic is accepted! It's completely unused and trivially satisfied, and unbounded generics are usually not accepted elsewhere in Rust. Consider for example that PhantomData marker fields must be added if a type declares, but does not use, a generic type.

Relying on the refinement fails at point of use, not declaration

Attempting to rely on either refinement when calling the refined function fail with the trait bound '<type>: Marker' is not satisfied errors:

fn use_first() {
    <First as upstream::Subject>::method::<()>(&First);
}

impl upstream::Super for () {}
fn use_second() {
    <Second as upstream::Subject>::method::<()>(&Second);
}

playground

A warning similar to the one for refined return types (impl trait in impl method signature does not match trait method signature) would be good. An example of that warning can be seen in this playground link.

Related to #121718, #100706

@rustbot label +F-refine +T-lang

Noratrieb commented 4 weeks ago

and unbounded generics are usually not accepted elsewhere in Rust

that's not true at all, unbounded generics are accepted everywhere? structs are a special case because their variance needs to be inferred.

obi1kenobi commented 4 weeks ago

You're right, I've just been conditioned that way I guess 😅

compiler-errors commented 4 weeks ago

This issue documents an aspect of https://github.com/rust-lang/rust/issues/100706 (refined trait implementations) that to my knowledge has not been previously discussed or documented: refining generic bounds on trait methods.

This is documented in the RFC, if not explicitly, then implicitly -- AFAICT the only examples that the RFC gives are with APIT (argument-position impl trait), but APIT is just sugar for a generic that cannot be turbofished. It's a shame that the RFC wasn't as thorough with explicitly stating that this can be done with where clauses or generic bounds as well.

Also, an impl having weaker where clauses than a trait really doesn't have anything to do with refinement. Specifically, the "Relying on the refinement fails at point of use" section is basically just the behavior that RFC 3245 wants to allow, but the fact that a generic call-site requires the where clauses of the trait to hold and not those of the impl has been how Rust works for approximately forever.

It also documents the current rustc behavior, which seems to be quite surprising

I don't consider this to be surprising, and while it's also a shame that this isn't documented in the Reference (like lots of other parts of the type system), this is very much intended behavior.

If you're curious, the place in rustc's implementation that enforces this is called compare_method_predicate_entailment. Specifically, it's tasked with ensuring that the implementation of a method is at least as general as the trait version of a method, so that we can ensure that any generic call-site can be replaced with a monomorphized instance of the method later on in codegen.

It's quite surprising that a bare generic is accepted! It's completely unused and trivially satisfied, and unbounded generics are usually not accepted elsewhere in Rust. Consider for example that PhantomData marker fields must be added if a type declares, but does not use, a generic type.

This is incorrect. "Unbounded" and "unused" generics are totally fine on free function items, on inherent methods, and in traits.

Attempting to rely on either refinement when calling the refined function fail with the trait bound ': Marker' is not satisfied errors

Yes, this is correct and expected behavior until someone implements RFC 3245 in its entirety, which I've expressed a lot of skepticism towards in the past, since it requires the type system to be able to be far more eager in determining which implementation actually satisfies a trait bound than we've needed to in the past.

obi1kenobi commented 4 weeks ago

Yes, this is correct and expected behavior until someone implements RFC 3245 in its entirety

I think it may be nice if rustc emits a warning here and in the other cases where expected behavior is surprising, just like it does for refined RPITs.

I think it's safe to narrow the scope of my post down to the above request-for-rustc-warning, given the rest appears to have been well-known and intentional behavior that I just wasn't aware of :+1: