Tweaks to the object-safety rules

[nikomatsakis]

Amend object safety RFC to include the complete set of rules, along with an exemption that permits individual methods to require that Self:Sized.

Implemented on the following branch:

https://github.com/nikomatsakis/rust/tree/object-safe-sized-methods

[solson]

Should be "the new method"? Also on line 167.

[sfackler]

Didn't we also add a restriction making static methods (like the new below) non-object safe?

[nikomatsakis]

ah, yes. Though I don't think of it as "add" so much as "forget" the first time.

[alexcrichton]

Does this obsolete the need for an IteratorExt trait? (along with a number of other FooExt traits in the standard library). It looks like we could move all methods into the Iterator trait while adding where Self: Sized bounds to each of them while still allowing Iterator itself to be object-safe.

[nikomatsakis]

@alexcrichton I believe that is correct, yes. Ideally, though, we'd find a way for those methods to be usable on iterator objects as well! I guess the major problem is that they frequently take self by value.

[alexcrichton]

@aturon and I discussed this a little more today, and it looks like we won't actually want to remove the IteratorExt trait. The any method has a type parameter (so it is not object self) but only requires &mut self so it doesn't need Self: Sized. This means that to be fully general this method at least needs to be provided in an extension trait (so IteratorExt still needs to exist).

From what we talked about though, any consuming method no longer necessarily needs to live in an extension trait as they can all just be tagged with where Self: Sized.

[P1start]

cc rust-lang/rust#22031

I’d rather like a built-in trait which is automatically implemented for all types except trait objects, allowing things like fn any<F>(&mut self, f: F) -> bool where Self: NotObject without needing an extension trait.

@alexcrichton IteratorExt already has a Sized supertrait bound on it, so while adding where Self: Sized to IteratorExt::any would ideally not be needed, we already enforce that on any anyway, so it wouldn’t worsen the current situation. It would, of course, be slightly less flexible than keeping IteratorExt and removing the Sized bound, but I personally would rather not fully support unsized iterators (which are practically unheard of as far as I know) than keep the messy (and very confusing to people who can’t find the iterator methods) hack that is the IteratorExt trait. And if we got a trait like what I described above, we could backwards-compatibly update any to use the new trait, I think.

---

On the topic of object safety rules, but slightly off-topic from the actual RFC, I’ve often wanted to tweak the ‘must not use Self’ rule to something more general like ‘must only refer to Self once in the input parameter list’. It seems to me that methods like fn frob(&mut self) -> Wrapper<Self> (where Wrapper<T> accepts T: ?Sized) and fn moo(x: i32, y: &Self) are theoretically object safe: the former would return a Wrapper<Trait>, and the latter would be equivalent to a normal method, but with its arguments reordered. (I should probably file an RFCs issue for this, rather than just commenting on an RFC…)

[huonw]

I'm in favour.

If we ever get type inequalities this would be "optimal" as trait Foo { fn ... where Self != Foo; } AFAICT.

[nikomatsakis]

I just pushed a slight adjustment that permits by-value self methods as well. The reasoning is two-fold:

1. Calling such a method is (currently) illegal if Self is not sized (or could easily be made so).
2. I would like to support by-value self methods for unsized values in the future.

In particular, this allows us to define FnOnce as fn call_once(self, ...) and permit trait FnMut : FnOnce without making FnMut be not object-safe. I'd prefer not to add a where Self : Sized requirement to FnOnce because if we permitted passing unsized values as arguments, it'd prevent that usage.

I also included some text highlighting that there is room to make the rules more liberal in the future.

[nikomatsakis]

(Note also that the question of IteratorExt is related but orthogonal)

[P1start]

How exactly could by-value self be object-safe? I thought the entire point of object safety was to disallow trait objects that can’t automatically implement their own trait. By-value self methods not only can’t be called on unsized types (as the RFC notes), they can’t even be implemented on unsized types, and frankly I’m surprised that they can even be defined on the trait in a context where Self isn’t necessarily Sized.

If we ever do allow unsized by-value self methods, then it of course makes sense to allow them on trait objects. But allowing by-value self methods to be object-safe to me seems like it should wait on such a feature.

[theemathas]

What about type parameters or associated types that default to Self?

See rust-lang/rust#22040 and rust-lang/rust#18956

[aturon]

@P1start

The key here is a realization we had a while back about where clauses on trait methods: you can interpret them as saying when the method must be implemented if the clause applies to type variables beyond those in the method. For example, in IteratorExt we have:

fn cycle(self) -> Cycle<Self> where Self: Clone

This is a method that only needs to be implemented on Self types that happened to be Clone.

The same principle is being applied here, with Self: Sized.

[aturon]

@nikomatsakis

I was talking with @wycats and realized that the use of where clauses here actually implies something deeper, which I think should cause us to re-evaluate object safety altogether.

In particular, since where clauses mean that not all trait objects need to be implemented in all contexts, we can achieve the initial goal of impl Trait for Trait universally for any trait ("object safe" or not). Basically, methods that aren't usable from trait objects act as if they had Self: Sized automatically -- a form of implied bounds.

Put differently, given the above, the only reason to have the object safety check is to force people to manually annotate with Self: Sized to say: "yes, I realize I'm using this trait as an object but not all of its methods are usable". I personally don't think that's a very good reason to force an annotation here.

[P1start]

@aturon I understand the where Self: Sized part of the RFC, and I’m all for it. What I don’t understand is the more recent change to the RFC that allows by-value self methods to be object-safe even if they don’t specify that Self: Sized. For example:

trait Foo {
    fn foo(self);
}

Trait Foo here, according to the updated RFC, is object-safe. That should mean that we should be able to impl Foo for Foo, right?

impl Foo for Foo {
    fn foo(self) { self.foo() } // error: the trait `core::marker::Sized` is not implemented for the type `Foo`
}

So I don’t see how this could be considered object-safe.

[aturon]

@P1start Ah, sorry! I think the point is that by-value self is interpreted as an implicit Self: Sized for now but would be changed later once you can use by-value self with objects.

[nikomatsakis]

@aturon interesting thought. One downside is that, as @huonw has pointed out, Self:Sized is actually a little imprecise. Perhaps Self:!TraitObject (if that existed) would be better or something like that. In particular, for things like generic fns, one could have Self=[int] -- I am pretty sure we make use of this in traits implemented on slices.

[nikomatsakis]

(I guess @huonw's suggestion of Self != SomeTrait (for the trait SomeTrait) is sort of the most precise one, but it doesn't cover subtraits, so it doesn't seem quite right.)

[aturon]

@nikomatsakis Yes. @huonw and I chatted about this on IRC -- in my proposal, I don't literally mean interpreting as Self: Sized, but something internally in the compiler that's a more precise version of it (even if you can't write it yourself). I suppose this does affect what you get to assume inside of default impls for such methods.

[nikomatsakis]

@aturon well, you have to be able to write it out, because we need to be able to use it as a bound on generic fns like fn foo<T:Trait>(...), right? And in particular, we need the methods to not be callable if you have fn foo<T:Trait+?Sized>(...), but that seems overly strict in general.

[nikomatsakis]

(Or rather, we need them to not be callable if T may be a trait object, but we have no way to say that it is not other than Sized vs ?Sized right now.)

[nikomatsakis]

Basically I guess you can interpret the object safety rules as an additional way of ruling out that for T:?Sized, T could be a trait object.

[aturon]

@nikomatsakis

you have to be able to write it out, because we need to be able to use it as a bound on generic fns like fn foo<T:Trait>(...), right? And in particular, we need the methods to not be callable if you have fn foo<T:Trait+?Sized>(...), but that seems overly strict in general.

So, to be clear, we'll want to eventually be able to write it out to take full advantage of the flexibility. But in the meantime, interpreting as something compiler-internal should be enough to (1) provide impl Trait for Trait while (2) not promising that Self: Sized for object-unsafe methods.

(But maybe I'm confused?)

[nikomatsakis]

@aturon my point is that it cripples traits that do not need to be object safe. For example, if I wrote a trait MapToVec:

trait MapToVec {
    fn map_to_vec<U>(&self) -> Vec<U>;
}

impl<T> MapToVec for [T] { ... } 

and I wanted to use that trait:

fn use_map_to_vec<T:MapToVec+?Sized>(t: &T) -> Vec<i32> {
    t.map_to_vec::<i32>()
}

The compiler would presumably have to disallow the call to t.map_to_vec because, for that call, Self would be T which is not known to be Sized and hence which might be MapToVec.

That example is a bit odd in that I left out some type parameters and other things one would need to make it realistic, but doesn't seem especially far-fetched to me.

[nikomatsakis]

I'd like to point out though that we can easily evolve object safety in the direction you propose, which I consider promising, though of course we won't be able to go back and fix traits in the meantime.

[aturon]

@nikomatsakis Thanks, I understand the worry better now.

But yes, I'm a little worried about precisely what we'll freeze into e.g. Iterator. Maybe we do need a way to write the constraint sooner than later.

[nikomatsakis]

The conservative path seems to be keeping IteratorExt around. We can always move (default) methods into Iterator and remove IteratorExt from the prelude, but we can never go the opposite way.

While I find the idea that all traits are object-safe very appealing, it's not clear to me how much that change would give us forwards compatibility. In particular, if we allows fns that use banned features to assume that "Self is not a trait object" (and hence use methods with other banned features), then if we expand the set of things that are possible to do with objects -- e.g., I'd like to include by-value self -- we're in a certain amount of trouble, since existing impls might be relying on that guarantee. But if we make them right it out specifically in the form of a where clause, then the trait is still locked into not support object type receivers when it could.

On the other hand, I don't see how we could ever support generic methods through virtual dispatch, at least without dynamic code generation or some additional list (on the trait) of pre-defined types. So that's a fairly safe case where you can assume that Self is not the object type. However, in reality the vast majorty of those helper methods take by-value self, so for the moment they must require that Self:Sized anyway. So it's not clear how much we're gaining here if we rush through some kind of syntax vs just using this proposal that is centered around Sized.

[aturon]

We can always move (default) methods into Iterator and remove IteratorExt from the prelude, but we can never go the opposite way.

Strictly speaking, it's possible to be relying on IteratorExt (the name) being in the prelude, though we can always introduce a new prelude. But there are lots of good reasons to merge ASAP, given the centrality of Iterator.

I think going with Self: Sized right now is probably OK, though I'd like to land something like my proposal above quickly if we can. We can probably relax the bounds on Iterator from Self: Sized to something more precise without any breakage in practice.

[nikomatsakis]

@aturon agreed. I'd also really like to move forward with this RFC because it allows motion on fn trait inheritance.

[nikomatsakis]

Just FYI, impl is rust-lang/rust#22301 :)

[aturon]

FWIW I'm happy to land this RFC for now and separately follow up on the more implicit design later.

[sfackler]

👍

I was able to do some nice cleanup of parts of rust-postgres using @nikomatsakis's branch: sfackler/rust-postgres@aa4cad7.
The fact that a default implementation of to_sql_checked won't work is a bit of a bummer but it seems like that can't really be helped unless we add conditionally implemented default impls.

[aturon]

This RFC makes it possible to have your cake and eat it too: we get impl Trait for Trait, and object safety in general, but make it possible to "opt out" for a given method by requiring Self: Sized. As the discussion shows, this is likely still not the final step for object safety (e.g., Self: Sized could probably be inferred in many cases), but it's a definite step forward. The RFC has been merged.