Added AsRef implementations for Arc and Rc #26008
Conversation
|
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
| @@ -332,6 +332,15 @@ impl<T: ?Sized> Deref for Arc<T> { | |||
| } | |||
| } | |||
|
|
|||
| #[stable(feature = "rc_arc_as_ref", since = "1.2.0")] | |||
There was a problem hiding this comment.
We actually have a lint to ensure that the same feature name (e.g. rust) is not stabilized in multiple different versions. This allows us to know that feature names are entirely stabilized all at once in only one version (useful for future analysis)
There was a problem hiding this comment.
Aha! Yeah I thought I grepped to check but I must have misgrepped, I see that in the codebase now.
The implementations are straightforward, I only hope I got the stability attributes correct(?).
These common traits were left off originally by accident from these smart pointers, and a past attempt (rust-lang#26008) to add them was later reverted (rust-lang#26160) due to unexpected breakge (rust-lang#26096) occurring. The specific breakage in worry is the meaning of this return value changed: let a: Box<Option<T>> = ...; a.as_ref() Currently this returns `Option<&T>` but after this change it will return `&Option<T>` because the `AsRef::as_ref` method shares the same name as `Option::as_ref`. A [crater report][crater] of this change, however, has shown that the fallout of this change is quite minimal. These trait implementations are "the right impls to add" to these smart pointers and would enable various generalizations such as those in rust-lang#27197. [crater]: https://gist.github.com/anonymous/0ba4c3512b07641c0f99 This commit is a breaking change for the above reasons mentioned, and the mitigation strategies look like any of: Option::as_ref(&a) a.as_ref().as_ref() (*a).as_ref()
These common traits were left off originally by accident from these smart pointers, and a past attempt (#26008) to add them was later reverted (#26160) due to unexpected breakge (#26096) occurring. The specific breakage in worry is the meaning of this return value changed: let a: Box<Option<T>> = ...; a.as_ref() Currently this returns `Option<&T>` but after this change it will return `&Option<T>` because the `AsRef::as_ref` method shares the same name as `Option::as_ref`. A [crater report][crater] of this change, however, has shown that the fallout of this change is quite minimal. These trait implementations are "the right impls to add" to these smart pointers and would enable various generalizations such as those in #27197. [crater]: https://gist.github.com/anonymous/0ba4c3512b07641c0f99 This commit is a breaking change for the above reasons mentioned, and the mitigation strategies look like any of: Option::as_ref(&a) a.as_ref().as_ref() (*a).as_ref()
The implementations are straightforward, I only hope I got the stability attributes correct(?).