From ecf185b433d0373ffad656c775733cbd8f2db036 Mon Sep 17 00:00:00 2001 From: Adrian Taylor Date: Tue, 28 Jan 2025 12:37:35 +0000 Subject: [PATCH] Arbitrary self types v2: shadowing semver break. This is a pre-existing risk of semver breakage for types implementing Deref. It's made more apparent by the arbitrary self types v2 change, and will now generate errors under some circumstances. This PR documents the existing risk and the new ways it can be triggered. If it's seen as desirable to document the existing risk before Arbitrary Self Types v2 is stabilized, we can split this commit up into two. Part of https://github.com/rust-lang/rust/issues/44874 Stabilization PR https://github.com/rust-lang/rust/pull/135881 --- src/doc/src/reference/semver.md | 123 ++++++++++++++++++++++++++++++++ 1 file changed, 123 insertions(+) diff --git a/src/doc/src/reference/semver.md b/src/doc/src/reference/semver.md index 232f7f3ac50..6f2c3c2295f 100644 --- a/src/doc/src/reference/semver.md +++ b/src/doc/src/reference/semver.md @@ -88,6 +88,7 @@ considered incompatible. * [Minor: generalizing a function to use generics (supporting original type)](#fn-generalize-compatible) * [Major: generalizing a function to use generics with type mismatch](#fn-generalize-mismatch) * [Minor: making an `unsafe` function safe](#fn-unsafe-safe) + * [Major: adding a potentially shadowing method](#fn-add-potentially-shadowing-method) * Attributes * [Major: switching from `no_std` support to requiring `std`](#attr-no-std-to-std) * [Major: adding `non_exhaustive` to an existing enum, variant, or struct with no private fields](#attr-adding-non-exhaustive) @@ -1883,6 +1884,128 @@ Making a previously `unsafe` associated function or method on structs / enums safe is also a minor change, while the same is not true for associated function on traits (see [any change to trait item signatures](#trait-item-signature)). +### Major: add a potentially shadowing method {#fn-add-potentially-shadowing-method} + +If you have a type which implements `Deref`, you must not add methods +which may "shadow" methods in `T`. This can lead to unexpected changes in +program behavior. + +```rust,ignore +/////////////////////////////////////////////////////////// +// Before +pub struct MySmartPtr(T); + +impl core::ops::Deref for MySmartPtr { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +/////////////////////////////////////////////////////////// +// After +pub struct MySmartPtr(T); + +impl core::ops::Deref for MySmartPtr { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl MySmartPtr { + fn method(&mut self) { + println!("A"); + } +} + +/////////////////////////////////////////////////////////// +// Example usage that will break. +struct SomeStruct; + +impl SomeStruct { + fn method(&self) { + println!("B"); + } +} + +fn main() { + let mut ptr = MySmartPtr(SomeStruct); + ptr.method(); // prints B before, A after +} +``` + +Note that the shadowing and shadowed methods receive `self` +slightly differently: `&self` and `&mut self`. +That's because Rust searches for methods first by value, then by `&`, then +by `&mut T`, then by `*const T`. Rust stops the search +when it encounters a valid method, and so methods later in this order may +be shadowed by methods encountered earlier. + +This is only a compatibility risk if the `Deref` target is +beyond your control. If your type implements `Deref` to another type where +you can fix the available methods, you can ensure no shadowing +occurs. An example is that `PathBuf` implements +`Deref`. + +For types which do implement `Deref` with an arbitrary target, +it's bad practice to add methods: add associated functions instead. This is +the pattern used by Rust's standard library smart pointer types, such as +`Box`, `Rc` and `Arc`. + +Similar shadowing risks occur for a type implementing +`Receiver`. If you have a type which implements either +`Receiver` or `Deref` it may be used as a method receiver +by `T`'s methods. If your type then adds a method, you may shadow methods in +`T`. For instance: + +```rust,ignore +// MAJOR CHANGE + +/////////////////////////////////////////////////////////// +// Before +pub struct MySmartPtr(T); + +impl core::ops::Receiver for MySmartPtr { // or Deref + type Target = T; +} + +/////////////////////////////////////////////////////////// +// After +pub struct MySmartPtr(T); + +impl core::ops::Receiver for MySmartPtr { // or Deref + type Target = T; +} + +impl MySmartPtr { + fn method(self) { + + } +} + +/////////////////////////////////////////////////////////// +// Example usage that will break. +struct SomeStruct; + +impl SomeStruct { + fn method(self: &MySmartPtr) { + + } +} + +fn main() { + let ptr = MySmartPtr(SomeStruct); + ptr.method(); // error: multiple applicable items in scope +} +``` + +When types like this are being used as method receivers, Rust endeavours to +do additional searches and present errors in simple cases, e.g. shadowing of +`&self` by `self` with inherent methods. This is better than invisible +behavior changes - but either way it's a compatibility break. Avoid adding +methods if you implement `Deref` or `Receiver` to an arbitrary target. + ### Major: switching from `no_std` support to requiring `std` {#attr-no-std-to-std} If your library specifically supports a [`no_std`] environment, it is a