Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Improve AsRef / AsMut docs on blanket impls #99460

Merged
merged 4 commits into from
Oct 3, 2022
Merged

docs: Improve AsRef / AsMut docs on blanket impls #99460

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
551d921
docs: Improve AsRef / AsMut docs on blanket impls
JanBeh Jul 19, 2022
9f68e3e
fixup! docs: Improve AsRef / AsMut docs on blanket impls
JanBeh Jul 19, 2022
e4a259b
fixup! docs: Improve AsRef / AsMut docs on blanket impls
JanBeh Jul 19, 2022
e6b761b
fixup! docs: Improve AsRef / AsMut docs on blanket impls
JanBeh Jul 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
205 changes: 187 additions & 18 deletions library/core/src/convert/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
//! # Generic Implementations
//!
//! - [`AsRef`] and [`AsMut`] auto-dereference if the inner type is a reference
//! (but not generally for all [dereferenceable types][core::ops::Deref])
//! - [`From`]`<U> for T` implies [`Into`]`<T> for U`
//! - [`TryFrom`]`<U> for T` implies [`TryInto`]`<T> for U`
//! - [`From`] and [`Into`] are reflexive, which means that all types can
Expand Down Expand Up @@ -108,10 +109,12 @@ pub const fn identity<T>(x: T) -> T {
/// If you need to do a costly conversion it is better to implement [`From`] with type
/// `&T` or write a custom function.
///
/// # Relation to `Borrow`
///
/// `AsRef` has the same signature as [`Borrow`], but [`Borrow`] is different in a few aspects:
///
/// - Unlike `AsRef`, [`Borrow`] has a blanket impl for any `T`, and can be used to accept either
/// a reference or a value.
/// a reference or a value. (See also note on `AsRef`'s reflexibility below.)
/// - [`Borrow`] also requires that [`Hash`], [`Eq`] and [`Ord`] for a borrowed value are
/// equivalent to those of the owned value. For this reason, if you want to
/// borrow only a single field of a struct you can implement `AsRef`, but not [`Borrow`].
Expand All @@ -121,9 +124,66 @@ pub const fn identity<T>(x: T) -> T {
///
/// # Generic Implementations
///
/// - `AsRef` auto-dereferences if the inner type is a reference or a mutable
/// reference (e.g.: `foo.as_ref()` will work the same if `foo` has type
/// `&mut Foo` or `&&mut Foo`)
/// `AsRef` auto-dereferences if the inner type is a reference or a mutable reference
/// (e.g.: `foo.as_ref()` will work the same if `foo` has type `&mut Foo` or `&&mut Foo`).
///
/// Note that due to historic reasons, the above currently does not hold generally for all
/// [dereferenceable types], e.g. `foo.as_ref()` will *not* work the same as
/// `Box::new(foo).as_ref()`. Instead, many smart pointers provide an `as_ref` implementation which
/// simply returns a reference to the [pointed-to value] (but do not perform a cheap
/// reference-to-reference conversion for that value). However, [`AsRef::as_ref`] should not be
/// used for the sole purpose of dereferencing; instead ['`Deref` coercion'] can be used:
///
/// [dereferenceable types]: core::ops::Deref
/// [pointed-to value]: core::ops::Deref::Target
/// ['`Deref` coercion']: core::ops::Deref#more-on-deref-coercion
///
/// ```
/// let x = Box::new(5i32);
/// // Avoid this:
/// // let y: &i32 = x.as_ref();
/// // Better just write:
/// let y: &i32 = &x;
/// ```
///
/// Types which implement [`Deref`] should consider implementing `AsRef<T>` as follows:
///
/// [`Deref`]: core::ops::Deref
///
/// ```
/// # use core::ops::Deref;
/// # struct SomeType;
/// # impl Deref for SomeType {
/// # type Target = [u8];
/// # fn deref(&self) -> &[u8] {
/// # &[]
/// # }
/// # }
/// impl<T> AsRef<T> for SomeType
/// where
/// T: ?Sized,
/// <SomeType as Deref>::Target: AsRef<T>,
/// {
/// fn as_ref(&self) -> &T {
/// self.deref().as_ref()
/// }
/// }
/// ```
///
/// # Reflexivity
///
/// Ideally, `AsRef` would be reflexive, that is there is an `impl<T: ?Sized> AsRef<T> for T`, with
JanBeh marked this conversation as resolved.
Show resolved Hide resolved
/// [`as_ref`] simply returning its argument unchanged.
/// Such a blanket implementation is currently *not* provided due to technical restrictions of
/// Rust's type system (it would be overlapping with another existing blanket implementation for
/// `&T where T: AsRef<U>` which allows `AsRef` to auto-dereference, see "Generic Implementations"
/// above).
///
/// [`as_ref`]: AsRef::as_ref
///
/// A trivial implementation of `AsRef<T> for T` must be added explicitly for a particular type `T`
/// where needed or desired. Note, however, that not all types from `std` contain such an
/// implementation, and those cannot be added by external code due to orphan rules.
///
/// # Examples
///
Expand Down Expand Up @@ -170,29 +230,138 @@ pub trait AsRef<T: ?Sized> {
///
/// # Generic Implementations
///
/// - `AsMut` auto-dereferences if the inner type is a mutable reference
/// (e.g.: `foo.as_mut()` will work the same if `foo` has type `&mut Foo`
/// or `&mut &mut Foo`)
/// `AsMut` auto-dereferences if the inner type is a mutable reference
/// (e.g.: `foo.as_mut()` will work the same if `foo` has type `&mut Foo` or `&mut &mut Foo`).
///
/// Note that due to historic reasons, the above currently does not hold generally for all
/// [mutably dereferenceable types], e.g. `foo.as_mut()` will *not* work the same as
/// `Box::new(foo).as_mut()`. Instead, many smart pointers provide an `as_mut` implementation which
/// simply returns a reference to the [pointed-to value] (but do not perform a cheap
/// reference-to-reference conversion for that value). However, [`AsMut::as_mut`] should not be
/// used for the sole purpose of mutable dereferencing; instead ['`Deref` coercion'] can be used:
///
/// [mutably dereferenceable types]: core::ops::DerefMut
/// [pointed-to value]: core::ops::Deref::Target
/// ['`Deref` coercion']: core::ops::DerefMut#more-on-deref-coercion
///
/// ```
/// let mut x = Box::new(5i32);
/// // Avoid this:
/// // let y: &mut i32 = x.as_mut();
/// // Better just write:
/// let y: &mut i32 = &mut x;
/// ```
///
/// Types which implement [`DerefMut`] should consider to add an implementation of `AsMut<T>` as
/// follows:
///
/// [`DerefMut`]: core::ops::DerefMut
///
/// ```
/// # use core::ops::{Deref, DerefMut};
/// # struct SomeType;
/// # impl Deref for SomeType {
/// # type Target = [u8];
/// # fn deref(&self) -> &[u8] {
/// # &[]
/// # }
/// # }
/// # impl DerefMut for SomeType {
/// # fn deref_mut(&mut self) -> &mut [u8] {
/// # &mut []
/// # }
/// # }
/// impl<T> AsMut<T> for SomeType
/// where
/// <SomeType as Deref>::Target: AsMut<T>,
/// {
/// fn as_mut(&mut self) -> &mut T {
/// self.deref_mut().as_mut()
/// }
/// }
/// ```
///
/// # Reflexivity
///
/// Ideally, `AsMut` would be reflexive, that is there is an `impl<T: ?Sized> AsMut<T> for T`, with
JanBeh marked this conversation as resolved.
Show resolved Hide resolved
/// [`as_mut`] simply returning its argument unchanged.
/// Such a blanket implementation is currently *not* provided due to technical restrictions of
/// Rust's type system (it would be overlapping with another existing blanket implementation for
/// `&mut T where T: AsMut<U>` which allows `AsMut` to auto-dereference, see "Generic
/// Implementations" above).
///
/// [`as_mut`]: AsMut::as_mut
///
/// A trivial implementation of `AsMut<T> for T` must be added explicitly for a particular type `T`
/// where needed or desired. Note, however, that not all types from `std` contain such an
/// implementation, and those cannot be added by external code due to orphan rules.
///
/// # Examples
///
/// Using `AsMut` as trait bound for a generic function we can accept all mutable references
/// that can be converted to type `&mut T`. Because [`Box<T>`] implements `AsMut<T>` we can
/// write a function `add_one` that takes all arguments that can be converted to `&mut u64`.
/// Because [`Box<T>`] implements `AsMut<T>`, `add_one` accepts arguments of type
/// `&mut Box<u64>` as well:
/// Using `AsMut` as trait bound for a generic function, we can accept all mutable references that
/// can be converted to type `&mut T`. Unlike [dereference], which has a single [target type],
/// there can be multiple implementations of `AsMut` for a type. In particular, `Vec<T>` implements
/// both `AsMut<Vec<T>>` and `AsMut<[T]>`.
///
/// In the following, the example functions `caesar` and `null_terminate` provide a generic
/// interface which work with any type that can be converted by cheap mutable-to-mutable conversion
/// into a byte slice (`[u8]`) or byte vector (`Vec<u8>`), respectively.
///
/// [dereference]: core::ops::DerefMut
/// [target type]: core::ops::Deref::Target
///
/// ```
/// fn add_one<T: AsMut<u64>>(num: &mut T) {
/// *num.as_mut() += 1;
/// struct Document {
/// info: String,
/// content: Vec<u8>,
/// }
///
/// let mut boxed_num = Box::new(0);
/// add_one(&mut boxed_num);
/// assert_eq!(*boxed_num, 1);
/// impl<T: ?Sized> AsMut<T> for Document
/// where
/// Vec<u8>: AsMut<T>,
/// {
/// fn as_mut(&mut self) -> &mut T {
/// self.content.as_mut()
/// }
/// }
///
/// fn caesar<T: AsMut<[u8]>>(data: &mut T, key: u8) {
/// for byte in data.as_mut() {
/// *byte = byte.wrapping_add(key);
/// }
/// }
///
/// fn null_terminate<T: AsMut<Vec<u8>>>(data: &mut T) {
/// // Using a non-generic inner function, which contains most of the
/// // functionality, helps to minimize monomorphization overhead.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is unclear to me whether this monomorphize optimization is helpful to portray the meaning of the example.
(e.g. remove the doit function, and mention the possible optimization in words below the example instead, with a relevant link. That is, if that "tip" merits staying on this AsMut doc page.)

I haven't read over very much of std doc, so it is possible things like this "tip not strictly related, but useful" are more common than I realize.

Just wanted to draw attention for consideration / discussion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Providing a generic API is the use case of AsRef. Unfortunately, the compiler isn't capable of doing these optimizations by itself. I think people who write generic APIs should see how it's done correctly (with current Rust).

I agree, however, that it makes the example even more complex. Maybe the example code could be non-optimized and the "better practice" added at the end:

Note, however, that APIs don’t need to be generic. In many cases taking a &mut [u8] or &mut Vec<u8>, for example, is the better choice (callers need to pass the correct type then).

When providing a generic API, it's good practice to use a non-generic inner function to minimize monomorphization overhead. Thus the function null_terminate from above would be better written as:

fn null_terminate<T: AsMut<Vec<u8>>>(data: &mut T) {
    fn inner(data: &mut Vec<u8>) {
        let len = data.len();
        if len == 0 || data[len-1] != 0 {
            data.push(0);
        }
    }
    inner(data.as_mut());
}

What do you think about it, and do you know how (and with what target?) I could (should?) link "monomorphization" in the docs properly?

Downside would be it makes the example section even longer, but maybe it's necessary to not confuse readers.

Copy link

@danjl1100 danjl1100 Jul 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I now see what you mean, generic usage is pretty central to these types.

The only reference I could find comes from a rust perf book, which is a separate project, not part of the official rust documentation. I don't think this should be linked in std docs.

I like having this optimization separated out. When the compiler is improved in the future to optimize this for users, then this addendum could be easily removed.

I'd like to hear what more seasoned rust members think, for the question of whether to include the optimization example now.
As that link mentions,

The effects of these sorts of changes on
compile times will usually be small,
though occasionally they can be large. Example

Copy link
Contributor Author

@JanBeh JanBeh Jul 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally would feel okay with keeping it as it is, but I also would like to hear some other opinions, as I acknowledge this makes the whole example a bit long. I don't see a good reference to link to (and none that is part of std or rust itself) – at least I didn't find any.

/// fn doit(data: &mut Vec<u8>) {
/// let len = data.len();
/// if len == 0 || data[len-1] != 0 {
/// data.push(0);
/// }
/// }
/// doit(data.as_mut());
/// }
///
/// fn main() {
/// let mut v: Vec<u8> = vec![1, 2, 3];
/// caesar(&mut v, 5);
/// assert_eq!(v, [6, 7, 8]);
/// null_terminate(&mut v);
/// assert_eq!(v, [6, 7, 8, 0]);
/// let mut doc = Document {
/// info: String::from("Example"),
/// content: vec![17, 19, 8],
/// };
/// caesar(&mut doc, 1);
/// assert_eq!(doc.content, [18, 20, 9]);
/// null_terminate(&mut doc);
/// assert_eq!(doc.content, [18, 20, 9, 0]);
/// }
/// ```
///
/// [`Box<T>`]: ../../std/boxed/struct.Box.html
/// Note, however, that APIs don't need to be generic. In many cases taking a `&mut [u8]` or
/// `&mut Vec<u8>`, for example, is the better choice (callers need to pass the correct type then).
#[stable(feature = "rust1", since = "1.0.0")]
#[cfg_attr(not(test), rustc_diagnostic_item = "AsMut")]
pub trait AsMut<T: ?Sized> {
Expand Down