Permit impl methods whose bounds cannot be satisfied to have no body

Opened by Niko Matsakis at 2022-09-26 21:37:38

There is a curious case with where clauses where sometimes we can show that a method in an impl could not possibly be called. This is because the impl has more precise information than the trait. Here is an example:

    trait MyTrait<T> {
        fn method(&self, t: &T) where T : Eq;
    }

    struct Foo;
    struct Bar; // note that `Bar` does not derive `Eq`

    impl MyTrait<Bar> for Foo {
        fn method(&self, t: &T) where Bar : Eq { // <-- `Bar : Eq` cannot be satisfied!
        }
    }

We should permit the method body to be omitted in such a case. As a workaround, once #20020 is fixed, I imagine it would be possible to write an impl like this:

impl MyTrait<Bar> for Foo {
    fn method(&self, t: &T) { // no where clause at all
        panic!("Bar : Eq could not be satisfied");
    }
}

However, it is unfortunate to require that of the user. For one thing, perhaps it happens later that an impl of Eq is added for Bar -- now we have this method hanging around that will panic. It'd be nice to detect that statically.

The plan then would be to permit:

impl MyTrait<Bar> for Foo {
    fn method(&self, t: &T); // <-- no body or where clauses needed
}

This serves as a declaration that you believe this method could never be called. At trans time, we will generate a body that simply does the equivalent of panic!("unsatisfiable methodmethodinvoked").

I plan to open an amendment to the where clause RFC describing this particular case.

It would be helpful to mark (or even erase) these uncallable methods in the "Trait Implementations" section of the rustdocs for the type implementing the trait.
Tom Jakubowski at 2014-12-19 16:15:42
@tomjakubowski I think I am the one who will be working #20020 and most likely this issue unless @nikomatsakis beats me to it. I will try to keep that in mind :).
Jared Roesch at 2014-12-20 08:13:20

Triaging this issue.

#20020 has already been closed, but yet we can't use unimplemented methods without where.

This is the code I used to see if it was still an issue:

trait MyTrait<T> {
    fn method(&self, t: &T) where T : Eq;
}

struct Foo;
struct Bar; // note that `Bar` does not derive `Eq`

impl MyTrait<Bar> for Foo {
    fn method(&self, t: &Bar);
}
fn main() {}

Rust version

rustc 1.7.0-nightly (b4707ebca 2015-12-27)

Bruno Tavares at 2015-12-28 12:24:42

In a similar situation, despite being object-safe, the following trait cannot be implemented for [T]:
```
trait Foo {
    fn foo(self);
}

impl<T> Foo for [T] {
    fn foo(self) { unimplemented!() }
    //~^ ERROR the trait `core::marker::Sized` is not implemented for the type `[T]`
}
```
The foo method has an implicit where Self: Sized as a result of taking self by value. Adding an explicit where Self: Sized bound would help if this issue is fixed, but the fix has to be more general than method-level where clauses.

As an unsatisfactory workaround, one can default foo as follows:
```
trait Foo {
    fn foo(self) where Self: Sized { unimplemented!() }
}

impl<T> Foo for [T] {}
```
This allows the impl to compile, but is nonsensical because it is exactly the Sized types that we want to provide a foo method.
Andrew Paseltiner at 2016-01-10 17:10:11
On Sun, Jan 10, 2016 at 09:10:54AM -0800, Andrew Paseltiner wrote:

In a similar situation, despite being object-safe, the following trait cannot be implemented for [T]:

FWIW, this is intentional, as we still aim to allow DST moves at some point (or at least I do). So I would not want to make it possible to implement that trait with [T] until that point is settled.
Niko Matsakis at 2016-01-11 19:55:02
@nikomatsakis Ah, I hadn't considered that. Are you still planning to amend the where clause RFC for this issue?
Andrew Paseltiner at 2016-01-13 16:15:45
This is also arising in the context of https://github.com/rust-lang/rfcs/pull/1216. Recent PR https://github.com/rust-lang/rfcs/pull/1699 is also relevant.

The specific issue is that the ! type allows us to identify cases where methods could not ever really be called (e.g., because the type of one of their parameters is !, and there can never be an instance of such a type). It'd be nice to omit such methods.
Niko Matsakis at 2016-08-10 13:53:31
Nominating for triage. Add some tags please. Last work item in https://github.com/rust-lang/rust/issues/17657
Brian Anderson at 2016-08-25 17:12:16
@nikomatsakis These two are qualitatively different things though. In the case that this issue is about, no call to the method could ever type-check. In the case that https://github.com/rust-lang/rfcs/pull/1699 is about, a call would type-check just fine, it's just that the method body would never actually be entered at runtime.
Gábor Lehel at 2016-08-25 18:55:24
Discussed in @rust-lang/lang meeting. A couple of notes:
- We should be careful here -- this is a form of negative reasoning, so we have to apply the "coherence criteria" to avoid subtle breakage where people get to elide methods because a trait is unimplemented, but then a trait is added in the future.
- Agreed, @glaebhoerl, there is a distinction between this and rust-lang/rfcs#1699.
triage: P-medium
Niko Matsakis at 2016-08-25 21:30:25
Ah, reviewing this though -- I'm not sure we ever settled on a syntax. It may be that we need an RFC here? Should review the existing RFC :)

EDIT: Just did, it offers no guidance here.
Niko Matsakis at 2016-08-25 21:31:20
Triage: not aware of any specific updates here.
Steve Klabnik at 2019-03-11 17:26:14
I recently asked about exactly this on StackOverflow where I was referred to this issue. This issue is very relevant for the design of a library I'm developing right now, so I'm very interested in resolving this.

Here is a short real life example:
<details>
I'm working on a polygon mesh processing library that abstracts over meshes. The important part for this example is that there are some mesh data structures that are restricted to triangular faces while some other data structures support all kinds of faces (in particular, mixed faces in one instance).
```
// Dummy
struct Vertex;


// Mesh kind marker types. Can be replaced with enum once const generic 
// lands. This trait is "sealed", i.e. it is only implemented by these two
// types.
trait FaceKind {}
enum TriFace {}
enum PolyFace {}
impl FaceKind for TriFace {}
impl FaceKind for PolyFace {}


trait Mesh {
    type FaceKind: FaceKind;
    
    // All meshes can add triangles, but only some meshes can add 
    // arbitrary faces.
    fn add_triangle(&mut self, vertices: &[Vertex; 3]);
    fn add_face(&mut self, vertices: &[Vertex])
    where
        Self: Mesh<FaceKind = PolyFace>;
}


struct MyTriMesh {}
impl Mesh for MyTriMesh {
    type FaceKind = TriFace;
    
    fn add_triangle(&mut self, vertices: &[Vertex; 3]) {
        // stuff
    }
    
    fn add_face(&mut self, vertices: &[Vertex]) {
        unreachable!() // <-- this is annoying and not robust
    }
}
```
This is only a small part of my API: there are many other places where this is relevant.
</details>
This thread has been rather quiet and it doesn't seem like a lot has happened in this regards in the last 5 years. So: what would be next steps to resolve this issue?

Probably writing a new RFC, right? Or is there some other work on the way that will have significant impact on this issue (e.g. the chalkification) and thus writing an RFC now doesn't make sense? If not and someone would be willing to start writing an RFC, what are important topics to discuss in the RFC? @nikomatsakis already mentioned something about negative reasoning and coherence. Something else?
Lukas Kalbertodt at 2019-05-14 20:48:30
(Copying what I said in https://github.com/rust-lang/rust/issues/48214#issuecomment-1258611721)

I would actually argue against this feature, for the following reason: I believe that Self bounds (or bounds on type parameters of the trait) on individual trait methods with no default implementation are an anti-pattern. Such bounds should be discouraged, in favor of separate traits. This code is questionable because it means that if type A starts implementing trait T, that can suddenly invalidate its implementation of trait U because U had a where Self: T trait method with no default impl. It's also a problem for traits wanting to take advantage of expansions to object safety in future versions of the language. I don't think complicated negative-reasoning rules are worth it just to accommodate this questionable pattern.

But maybe there is a use-case I am missing, where separate traits are not adequate?
Jules Bertholet at 2022-09-26 21:36:12