Pre-RFC: pub(crate) impl Trait for Type


#1

(This is pretty unbaked, but feels concrete enough to ask for feedback on.)

Summary

Allows you to write

pub(crate) impl From<dependency::Type> for crate::Type { ... }

to benefit from having the trait implemented internal to your crate without “leaking” the dependency as public.

Motivation

regex@0.2 accidentally had a public dependency on regex_syntax due to a From implementation. In parsing applications, it’s not uncommon for your parsing library to have a Span representation different from the Span that you want to expose to users.

Even in non-From cases, it can be useful to implement traits that would otherwise leak a private dependency for local use only.

Guide-level explanation

If you have a dependency on crate foo, and that crate exposes a type Foo, implementing From<Foo> for your type makes foo a public dependency of your crate. This means breaking changes to foo are also breaking changes to your crate, as foo is part of your public API.

If that From<Foo> implementation is not meant to be used as public API, you can instead write pub(crate) impl to scope the trait implementation to your local crate only. Users of your library crate will have no access to the trait implementation, so foo remains a private dependency, able to be bumped in a semver-compatible manner.

Reference-level explanation

The grammar is changed to take a visibility marker before impl in impl Trait for Type. pub is implied, and pub(crate) is the only other allowed visibility [that the RFC author has bothered to consider].

The trait implementation is not shared with consumers of the crate as a library. Within the crate, however, the type is treated as implementing the trait. Dependencies of the crate can observe the implementation if the type is passed to them. Because of this, the normal orphan rules must be followed.

The scoped trait implementation is not allowed to be converted to a dyn Trait trait object or returned as impl Trait, as that would trivially allow leaking the concrete type’s implementation of the trait to library consumers.

Drawbacks

As all language additions, it adds complexity to the language.

[Further drawbacks TODO]

Rationale and alternatives

  • TODO

Prior art

[None?]

Unresolved questions

  • This would seem to require preventing the type from being turned into a trait object (or abstract type) in order to enforce that it doesn’t leak. Is this sustainable? Is it actually possible that, assuming orphan rules are followed, that leaking trait implementation this way wouldn’t break the “local” reasoning of the implementation? Leaking the implementation to library users would require the dependency be public anyway, in which case the implementation might as well be public (unless for some reason it’s unstable).
  • With further restrictions, could this syntax be used to allow selectively breaking the orphan rules? If nobody else sees you break the rules, is it problematic?

#2

Piror arts:

Thin Traits by @aturon

He mentioned that they expect to have the trait limited to implement on types in the same crate. So it is relevant. But this is much more ambitious and answered your question:

Instead, his proposal implies a more powerful way to create trait objects. I think this can work directly when you require where Self:Sized.


#3

What if you call into some dependencies function that takes a trait object instead of being generic? Could the compiler statically determine if you leak a trait object externally, rather than just internal usage?


#4

I’ve written a full draft RFC about this that is fully baked, https://github.com/Centril/rfcs/pull/7, which has been blocked on reviews for some time. It has some minor differences, and I can integrate some of your motivation, but I would like to go with this draft.


#5

If you limit it only to within your crate (or even module/scope where declaration occurs) it would be the functional equivalent of C#'s “Extension Methods” I believe. I can’t see why that couldn’t work/be useful. Leaking it out as “Trait Objects” seems a little more likely to run into problems, but, I’ll let the experts be the better judge of that.


#6

I’m having trouble imagining how this could work with the orphan rules. Could you walk through some examples?


#7

Am I misreading/misunderstanding or is your proposal about private implementations only for structs, enums, etc. that your crate defines of traits not defined by your crate whereas the OP’s proposal is about private impl’s of traits defined by other crates for structs, enums, etc. defined by other crates where said impl of traits is not already defined?

EDIT: Upon further re-reading, yes, I think I am misunderstanding the OP’s proposal and it is in fact in-line with your proposal:


#8

!!Bikeshedding warning!!

IIRC priv is still a keyword for historical reason, and priv impl is read naturally as private implementation which is what this RFC is intended. Doesn’t it fits more for this case?


#9

It is being unreserved in the 2018 edition (IIRC). That being said, it could probably be un-un-reserved. :slight_smile:


#10

It is not.


#11

Thanks for the correction. I thought that I recalled seeing an RFC that was approved for this, but, it must’ve been rejected.


#12

I believe you are referring to my RFC 2421 which was accepted, but in a modified form from the original (as usually happens with the RFC process ^,-).


#13

A library has a routine which puts Foo in a hashmap, using a pub(crate) implementation of Hash. I implement Hash for Foo with a pub(crate) impl as well. I can observe the difference in hash implementation by accessing the same hash table from routines in my library vs theirs, breaking the invariants of the hash table.


#14

So, this is an example that demonstrates how it wouldn’t work then, right? I guess I was looking for an example of how it could work, but, this counter-example seems to demonstrate that it could not work, right?


#15

I don’t think it could work.


#16

@Centril’s RFC draft is indeed a more baked version of this, and contains reasoning around why erasing the concrete type into the trait is fine. I’d like to see public/private dependency control mentioned (since that’s one of the biggest reasons I see for scoping trait implementations), but otherwise it is exactly what I had in mind for this.


#17

And we’re live folks!