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

Refining generic bounds in trait method #129251

Open
obi1kenobi opened this issue Aug 18, 2024 · 4 comments
Open

Refining generic bounds in trait method #129251

obi1kenobi opened this issue Aug 18, 2024 · 4 comments
Labels
A-traits Area: Trait system C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.

Comments

@obi1kenobi
Copy link
Member

obi1kenobi commented Aug 18, 2024

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

@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. F-refine `#![feature(refine)]`; RFC #3245 T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Aug 18, 2024
@Noratrieb
Copy link
Member

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
Copy link
Member Author

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

@compiler-errors
Copy link
Member

compiler-errors commented Aug 18, 2024

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.

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
Copy link
Member Author

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 👍

@fmease fmease added A-traits Area: Trait system T-types Relevant to the types team, which will review and decide on the PR/issue. C-discussion Category: Discussion or questions that doesn't represent real issues. and removed F-refine `#![feature(refine)]`; RFC #3245 needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system C-discussion Category: Discussion or questions that doesn't represent real issues. T-lang Relevant to the language team, which will review and decide on the PR/issue. T-types Relevant to the types team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants