Noticed some unexpected behavior today. Unpin seems to be auto implemented on the return value of functions returning impl Trait.
use std::{any::Any, marker::PhantomPinned};
fn assert_unpin(_: impl Unpin) {}
// Imagine a crate exports this function.
pub fn get_any() -> impl Any {}
// Now a consumer may rely on the return value being Unpin.
// No complaints from rustc.
assert_unpin(get_any());
// But then the crate author changes the implementation.
// Note the type signature has not changed.
pub fn new_get_any() -> impl Any {
PhantomPinned
}
// It's an accidental breaking change.
assert_unpin(new_get_any());
// error[E0277]: `PhantomPinned` cannot be unpinned
// --> src/main.rs:20:18
// |
// 15 | pub fn new_get_any() -> impl Any {
// | -------- within this `impl Any`
// ...
Swap out Unpin and PhantomPinned with Send and Rc<()> an you'll see similar behavior.
It seems a bit unnerving that:
assert_unpin(get_any()); works in the first place.
Downstream code can be broken without editing to the function signature.
Do I misunderstand auto-traits? Do I misunderstand return position impl trait?
Return position impl Trait does indeed leak auto traits, including Unpin but perhaps even more notably also Sync and Send. It's a deliberate design decision to make impl Trait easier to use, on one hand.
On the other hand, it can indeed be a backwards compatibility hazard/foot-gun, and might require some trickery to work around (like, throw in some marker types to remove unwanted implementation of Unpin/Send/Sync/... and writing tests against accidental unwanted change (adding or removal of such an impl) is quite tricky, too.
Notably, too, something worth mentioning given your example features Any (though this works generally as soon as a 'static bound is present): users can also observe / rely on the precise type used in impl Trait by downcasting to that type through the Any trait. Just to name another way in which the abstraction of impl Trait return types can be a bit weakened / broken.
Note that nominal structs also leak auto-traits, so updating them can also break downstream code without editing the struct header or any function signatures. [1] Granted, you're probably more likely to make such a change with something cobbled together in an RPIT function.
If you take the defensive route (proactive PhantomPinned and whatnot), you'll probably gravitate towards some local struct which you forward traits through. Once you do that, you might as well keep it private, which would mitigate the Any aspect.
(Though IMO any reasonable person using Any for this purpose should accept that breakage due to a change of type only is on them.)
They also leak variance and non-'static outlives bounds, and can cause breakage in other ways without #[non_exhaustive]. ↩︎
@steffahn@quinedot Thank you for explaining auto-trait leakage with RPIT. I think I've realized a couple things now, and wanted to confirm:
RPIT is useful internally because of the flexibility, but probably not a good idea in public APIs since it is easy to cause incompatibilities.
I had assumed that the std APIs that return trait impls (e.g., all those Iterator adapters) would have used RPIT, but it wasn't available until after the APIs were published. Now I assume they would not have used RPIT even it was available.
But:
When precise capturing is available, RPIT could be used without incompatibility concerns in public APIs?
What do you mean by "cause incompatibilities"? My interpretation would normally be "cause a breaking change", but if it's a new API, it's not a major breaking change. If you mean "hazard of accidentally breaking something later by changing something", well, nominal structs leak more so have a bigger surface area, but arguably it's easier to break RPIT because changing function body code is "easier" or seems more natural than changing a struct implementation, and we're used to having freedom to change function bodies at will.
Anyway, some tradeoffs are
Nominal structs have names but RPITs cannot easily / directly be named yet
Nominal structs leak variance while RPITs are invariant
Nominal structs "leak" all the traits they implement, not just auto traits and those you name
Nominal structs can have inherent methods and public fields while RPITs cannot[1]
Downstream can implement local traits on upstream nominal structs but not on RPITs
RPITS have over/under capture problems (largely going away soon, we hope) which nominal structs do not
RPITs can contain local unnameable types without type erasure while nominal structs cannot
RPITs only became possible in traits recently and make a trait non-object-safe which nominal structs do not
Until we get TAIT, every RPIT site is a unique type whereas nominal structs can be resued
For now I feel that nominal structs are still best for public APIs, that is, I feel they have the better pro/con story for consumers of an API. Writing RPITs is often easier for the implementor (more rapid development and less boilerplate), but is sometimes unacceptable, e.g. due to being non-trait-safe.
Do you mean, could the existing APIs switch to using them? That will be a breaking change for the foreseeable future due to things like invariance and downstream not being able to implement traits on opaque types.[2]
I feel that RPITs' best use cases are those that have long-standing with nominal structs, like returning unboxed futures and closures. It's no coincidence that async is a major driver of RPIT development and is at the center of most official posts about them.
other than indirectly, e.g. you think of some useful method and have to add a custom trait so you can attach it do your RPIT ↩︎
At least, it would be odd and a lot of work to put so many restrictions on a nominal struct that changing it to an opaque type could not break anything. ↩︎
That's what I meant, I was referring to creation of new APIs and possible future breakage and in particular the part about "we're used to having freedom to change function bodies at will".
I should have been more explicit. Thanks for the details! I'll spend some time digesting them. The situation is more complex than I thought.
I understand the part about the existing APIs.
But actually I was just saying that with precise capturing we will be able to create APIs using RPIT that are not as vulnerable to future breakage (due to method body changes) as RPIT is today. That seems true based on your response.
Thanks for the comparison with nominal structs, that's very useful.
This is theoretically where semver checking tools should solve the problem. Unfortunately, the two that I know of (cargo semver-checks and cargo public-api) are currently unable to catch breaking changes with auto traits in impl Trait.
cargo-semver-checks maintainer here! Thanks for the ping
From an architectural perspective, I believe cargo-semver-checks could catch this since all the necessary information is present.
The major limiting factor is funding. I've been finding it very hard to get sponsorships even in "one coffee per month" amounts, let alone anything more substantial. While I still work on and maintain cargo-semver-checks, progress is slow because I have to pay my rent via other work.
If this breakage is painful enough to be worth catching & preventing once and for all, I'd appreciate help in getting funding for it!
