From 1f44f8e5bdbe6c78631afc817d2d2041393bf41c 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. One of the two code examples here is marked "skip" because it depends on the nightly feature which is under discussion for stabilization. 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 | 132 ++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/src/doc/src/reference/semver.md b/src/doc/src/reference/semver.md index 232f7f3ac50..ff1148b11d5 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,136 @@ 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,run-fail +// MAJOR CHANGE + +/////////////////////////////////////////////////////////// +// Before +#[derive(Clone, Copy)] +pub struct MySmartPtr(pub T); + +impl core::ops::Deref for MySmartPtr { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +/////////////////////////////////////////////////////////// +// After +#[derive(Clone, Copy)] +pub struct MySmartPtr(pub T); + +impl core::ops::Deref for MySmartPtr { + type Target = T; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl MySmartPtr { + pub fn method(self) -> usize { + 2 + } +} + +/////////////////////////////////////////////////////////// +// Example usage that will break. +use updated_crate::MySmartPtr; + +struct SomeStruct; + +impl SomeStruct { + fn method(&self) -> usize { + 1 + } +} + +fn main() { + let mut ptr = MySmartPtr(SomeStruct); + assert_eq!(ptr.method(), 1); +} +``` + +Note that the shadowing and shadowed methods receive `self` +slightly differently: `self` and `&self`. +That's because Rust [searches for methods] first by value, then by `&`, then +by `&mut 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,skip +// MAJOR CHANGE + +/////////////////////////////////////////////////////////// +// Before +#![feature(arbitrary_self_types)] +pub struct MySmartPtr(pub T); + +impl core::ops::Receiver for MySmartPtr { + // or Deref + type Target = T; +} + +/////////////////////////////////////////////////////////// +// After +#![feature(arbitrary_self_types)] +pub struct MySmartPtr(pub T); + +impl core::ops::Receiver for MySmartPtr { + // or Deref + type Target = T; +} + +impl MySmartPtr { + pub fn method(self) {} +} + +/////////////////////////////////////////////////////////// +// Example usage that will break. +#![feature(arbitrary_self_types)] +use updated_crate::MySmartPtr; + +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 @@ -2317,3 +2448,4 @@ document what your commitments are. [wildcard patterns]: ../../reference/patterns.html#wildcard-pattern [unused_unsafe]: ../../rustc/lints/listing/warn-by-default.html#unused-unsafe [msrv-is-minor]: https://github.com/rust-lang/api-guidelines/discussions/231 +[searches for methods]: ../../reference/expressions/method-call-expr.html#method-call-expressions \ No newline at end of file