Decide about generics in arbitrary self types

[traviscross]

Over here, @adetaylor gave an update about arbitrary self types that included this bit:

During the preparation of the RFC it was broadly felt that we should ban "generic" self types but we didn't really define what "generic" meant, and I didn't pin it down enough.

Some of the commentary:

• Arbitrary self types v2 rfcs#3519 (comment)
• Arbitrary self types v2 rfcs#3519 (comment)
• Arbitrary self types v2 rfcs#3519 (comment)
• A zulip comment
• I've a feeling there's more which I've been unable to find, including the initial impetus to ban generics from arbitrary self types.

It seems to be widely felt that:

impl SomeType {
 fn m<R: Deref<Target=Self>>(self: R) {}
}

would be confusing, but (per those comments) it's actually pretty hard to distinguish that from various legitimate cases for arbitrary self types with generic receivers:

impl SomeType {
  fn method1(self: MyBox<Self, impl Allocator>) {}
  fn method2<const ID: u64>(self: ListArc<Self, ID>) {}
}

I played around with different tests here on the self type in wfcheck.rs but none of them yield the right sort of filter of good/bad cases (which is unsurprising since we haven't quite defined what that means, but I thought I might hit inspiration).

From those comment threads, the most concrete proposal (from @joshtriplett) is:

just disallow the case where the top-level receiver type itself is not concrete.

I plan to have a crack at that, unless folks have other thoughts. cc @Nadrieril who had opinions here.

If we do this, I might need to revisit the diagnostics mentioned in the bits of RFC removed by this commit.

We never made a decision about this. Perhaps if we could offer more clarity, it would save @adetaylor some time here, so it may be worth us discussing.

@rustbot labels +T-lang +I-lang-nominated -needs-triage +C-discussion

cc @rust-lang/lang @adetaylor

[adetaylor]

Thanks for raising this @traviscross .

[adetaylor]

Current status: investigating whether I can improve the diagnostics here that can occur when generics are used and the self type doesn't unify cleanly. The canonical failure case is here and the suggestion to improve diagnostics here is from this comment by @compiler-errors.

Outcomes might be:

• (the best case) we can find checks we put in place within wfcheck.rs to eliminate some of these classes of type mismatch - I'm not sure if this will turn out to be possible.
• maybe it turns out that these mismatched types errors can only be achieved by use of a turbofish, and perhaps there is somewhere that turbofishes are checked for correctness and this can be enhanced - I think I'm ruling this out because it would need to understand that the generic type parameter is used for the self type, and that information is presumably hard to find at the call site (thoughts welcome though)
• (the worst case) we can't do anything to stop these errors being emitted where they are, but we can probably at least replace the generic "mismatched type" error with a new, slightly more helpful "mismatched self type" error.

Once I've got to the bottom of this, I'll report back, with some thoughts on all the reasons we might have wanted to exclude generics (not just diagnostics).

[adetaylor]

Here's a summary of the concerns and considerations about why we might want to block generic arbitrary self
types.

Also, I finally found the original suggestion to ban generics from @RalfJung , which led to the commit editing the RFC to do so.

• Diagnostics. With this sort of code we end up with a mismatched type error. This comment from @compiler-errors says we should try to restrict the feature such that users can't end up in that circumstance quite as often.

  • Having looked into this a bit, it's pretty hard to trigger such a case by accident. This code example uses a turbofish to explicitly insist on a different type.
  • Without a turbofish, I can't think of any ways to trigger such a mismatched type error except by using the type of some other function parameter to bound a receiver like in this pretty contrived example (the actual method call is down here). Other cases like this don't trigger this error because the nature of a Self type can't easily be represented; trait bounds get a bit recursive.
  • I therefore think these mismatched type errors will be sufficiently rare that we don't need to worry about constraining this feature just to avoid that confusing diagnostic.
  • However, we can add better diagnostics in this case. As there are all sorts of type errors, I think the best approach is just to add an extra note like this to such type errors. (Note that more complex cases won't flow through this path so won't get this additional diagnostic, but we may as well show it in simple cases.)

• Complex interaction with anti-shadowing algorithms

  • An earlier draft of the RFC proposed a fairly complex algorithm to detect shadowed methods, which would allow us to use types like raw pointers, Weak<T> or NonNull<T> as self types. During the process of agreeing the RFC, we agreed that the complexity of this deshadowing approach was not worth its value - for now. Instead, such types need to be wrapped in newtype wrappers if they are to be used as self types.
  • This deshadowing algorithm sometimes favored "inner" methods - e.g. in a case like A<B> it would pick B::foo rather than A::foo. This was surprising (which is the reason we dropped the idea) and the surprise levels might have been greater for generic types, where A::Target didn't simply and always point to B.
  • This complexity has fallen away from the RFC now, so isn't a concern. If we put it back in future, we can consider excluding certain kinds of generic types from that algorithm.

• Avoiding "unknown unknowns".

  • The original reason to ban generics was simply because of unknown unknowns.

IMO the only remaining reason to avoid generic receiver types is the desire to avoid "unknown unknowns" per this third point. This is a good reason, but there are counterarguments:

• We don't really know what we mean by a "generic" receiver, since by definition all the types of receiver we really care about are partially generic: Box<Self>, Rc<Self>, SomeCppPtr<Self>, etc. In fact some are even more generic (Box<Self, A>).
• Even if we did have a clear definition, implementing it might cause more surprise in users. For example if a user had been happily using Foo<A> as their self type, then (for example) an upstream crate changes it to Foo<A,B> and things break. It's hard to think of any definition of "generic" where we don't run into this sort of risk.

As such: I am now firmly of the opinion that we should just allow generic receivers, as the current nightly arbitrary self types v1 does.