Skip to content

Commit

Permalink
Rollup merge of #92444 - dtolnay:coremethods, r=joshtriplett
Browse files Browse the repository at this point in the history 
Consolidate Result's and Option's methods into fewer impl blocks

`Result`'s and `Option`'s methods have historically been separated up into `impl` blocks based on their trait bounds, with the bounds specified on type parameters of the impl block. I find this unhelpful because closely related methods, like `unwrap_or` and `unwrap_or_default`, end up disproportionately far apart in source code and rustdocs:

<pre>
impl&lt;T&gt; Option&lt;T&gt; {
    pub fn unwrap_or(self, default: T) -&gt; T {
        ...
    }

    <img alt="one eternity later" src="https://user-images.githubusercontent.com/1940490/147780325-ad4e01a4-c971-436e-bdf4-e755f2d35f15.jpg" width="750">
}

impl&lt;T: Default&gt; Option&lt;T&gt; {
    pub fn unwrap_or_default(self) -&gt; T {
        ...
    }
}
</pre>

I'd prefer for method to be in as few impl blocks as possible, with the most logical grouping within each impl block. Any bounds needed can be written as `where` clauses on the method instead:

```rust
impl<T> Option<T> {
    pub fn unwrap_or(self, default: T) -> T {
        ...
    }

    pub fn unwrap_or_default(self) -> T
    where
        T: Default,
    {
        ...
    }
}
```

*Warning: the end-to-end diff of this PR is computed confusingly by git / rendered confusingly by GitHub; it's practically impossible to review that way. I've broken the PR into commits that move small groups of methods for which git behaves better &mdash; these each should be easily individually reviewable.*
  • Loading branch information
@matthiaskrgr
matthiaskrgr committed Jan 3, 2022
2 parents df92119 + 5960f7a commit 13e2840
Show file tree
Hide file tree
Showing 6 changed files with 529 additions and 512 deletions.
240 changes: 118 additions & 122 deletions library/core/src/option.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,45 @@ impl<T> Option<T> {
}
}

/// Returns the contained [`Some`] value or a default.
///
/// Consumes the `self` argument then, if [`Some`], returns the contained
/// value, otherwise if [`None`], returns the [default value] for that
/// type.
///
/// # Examples
///
/// Converts a string to an integer, turning poorly-formed strings
/// into 0 (the default value for integers). [`parse`] converts
/// a string to any other type that implements [`FromStr`], returning
/// [`None`] on error.
///
/// ```
/// let good_year_from_input = "1909";
/// let bad_year_from_input = "190blarg";
/// let good_year = good_year_from_input.parse().ok().unwrap_or_default();
/// let bad_year = bad_year_from_input.parse().ok().unwrap_or_default();
///
/// assert_eq!(1909, good_year);
/// assert_eq!(0, bad_year);
/// ```
///
/// [default value]: Default::default
/// [`parse`]: str::parse
/// [`FromStr`]: crate::str::FromStr
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[rustc_const_unstable(feature = "const_option_ext", issue = "91930")]
pub const fn unwrap_or_default(self) -> T
where
T: ~const Default,
{
match self {
Some(x) => x,
None => Default::default(),
}
}

/// Returns the contained [`Some`] value, consuming the `self` value,
/// without checking that the value is not [`None`].
///
Expand Down Expand Up @@ -1033,6 +1072,58 @@ impl<T> Option<T> {
}
}

/// Converts from `Option<T>` (or `&Option<T>`) to `Option<&T::Target>`.
///
/// Leaves the original Option in-place, creating a new one with a reference
/// to the original one, additionally coercing the contents via [`Deref`].
///
/// # Examples
///
/// ```
/// let x: Option<String> = Some("hey".to_owned());
/// assert_eq!(x.as_deref(), Some("hey"));
///
/// let x: Option<String> = None;
/// assert_eq!(x.as_deref(), None);
/// ```
#[stable(feature = "option_deref", since = "1.40.0")]
#[rustc_const_unstable(feature = "const_option_ext", issue = "91930")]
pub const fn as_deref(&self) -> Option<&T::Target>
where
T: ~const Deref,
{
match self.as_ref() {
Some(t) => Some(t.deref()),
None => None,
}
}

/// Converts from `Option<T>` (or `&mut Option<T>`) to `Option<&mut T::Target>`.
///
/// Leaves the original `Option` in-place, creating a new one containing a mutable reference to
/// the inner type's [`Deref::Target`] type.
///
/// # Examples
///
/// ```
/// let mut x: Option<String> = Some("hey".to_owned());
/// assert_eq!(x.as_deref_mut().map(|x| {
/// x.make_ascii_uppercase();
/// x
/// }), Some("HEY".to_owned().as_mut_str()));
/// ```
#[stable(feature = "option_deref", since = "1.40.0")]
#[rustc_const_unstable(feature = "const_option_ext", issue = "91930")]
pub const fn as_deref_mut(&mut self) -> Option<&mut T::Target>
where
T: ~const DerefMut,
{
match self.as_mut() {
Some(t) => Some(t.deref_mut()),
None => None,
}
}

/////////////////////////////////////////////////////////////////////////
// Iterator constructors
/////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -1581,7 +1672,7 @@ impl<T, U> Option<(T, U)> {
}
}

impl<T: Copy> Option<&T> {
impl<T> Option<&T> {
/// Maps an `Option<&T>` to an `Option<T>` by copying the contents of the
/// option.
///
Expand All @@ -1597,41 +1688,18 @@ impl<T: Copy> Option<&T> {
#[must_use = "`self` will be dropped if the result is not used"]
#[stable(feature = "copied", since = "1.35.0")]
#[rustc_const_unstable(feature = "const_option", issue = "67441")]
pub const fn copied(self) -> Option<T> {
pub const fn copied(self) -> Option<T>
where
T: Copy,
{
// FIXME: this implementation, which sidesteps using `Option::map` since it's not const
// ready yet, should be reverted when possible to avoid code repetition
match self {
Some(&v) => Some(v),
None => None,
}
}
}

impl<T: Copy> Option<&mut T> {
/// Maps an `Option<&mut T>` to an `Option<T>` by copying the contents of the
/// option.
///
/// # Examples
///
/// ```
/// let mut x = 12;
/// let opt_x = Some(&mut x);
/// assert_eq!(opt_x, Some(&mut 12));
/// let copied = opt_x.copied();
/// assert_eq!(copied, Some(12));
/// ```
#[must_use = "`self` will be dropped if the result is not used"]
#[stable(feature = "copied", since = "1.35.0")]
#[rustc_const_unstable(feature = "const_option_ext", issue = "91930")]
pub const fn copied(self) -> Option<T> {
match self {
Some(&mut t) => Some(t),
None => None,
}
}
}

impl<T: Clone> Option<&T> {
/// Maps an `Option<&T>` to an `Option<T>` by cloning the contents of the
/// option.
///
Expand All @@ -1658,8 +1726,8 @@ impl<T: Clone> Option<&T> {
}
}

impl<T: Clone> Option<&mut T> {
/// Maps an `Option<&mut T>` to an `Option<T>` by cloning the contents of the
impl<T> Option<&mut T> {
/// Maps an `Option<&mut T>` to an `Option<T>` by copying the contents of the
/// option.
///
/// # Examples
Expand All @@ -1668,115 +1736,43 @@ impl<T: Clone> Option<&mut T> {
/// let mut x = 12;
/// let opt_x = Some(&mut x);
/// assert_eq!(opt_x, Some(&mut 12));
/// let cloned = opt_x.cloned();
/// assert_eq!(cloned, Some(12));
/// let copied = opt_x.copied();
/// assert_eq!(copied, Some(12));
/// ```
#[must_use = "`self` will be dropped if the result is not used"]
#[stable(since = "1.26.0", feature = "option_ref_mut_cloned")]
#[rustc_const_unstable(feature = "const_option_cloned", issue = "91582")]
pub const fn cloned(self) -> Option<T>
where
T: ~const Clone,
{
match self {
Some(t) => Some(t.clone()),
None => None,
}
}
}

impl<T: Default> Option<T> {
/// Returns the contained [`Some`] value or a default.
///
/// Consumes the `self` argument then, if [`Some`], returns the contained
/// value, otherwise if [`None`], returns the [default value] for that
/// type.
///
/// # Examples
///
/// Converts a string to an integer, turning poorly-formed strings
/// into 0 (the default value for integers). [`parse`] converts
/// a string to any other type that implements [`FromStr`], returning
/// [`None`] on error.
///
/// ```
/// let good_year_from_input = "1909";
/// let bad_year_from_input = "190blarg";
/// let good_year = good_year_from_input.parse().ok().unwrap_or_default();
/// let bad_year = bad_year_from_input.parse().ok().unwrap_or_default();
///
/// assert_eq!(1909, good_year);
/// assert_eq!(0, bad_year);
/// ```
///
/// [default value]: Default::default
/// [`parse`]: str::parse
/// [`FromStr`]: crate::str::FromStr
#[inline]
#[stable(feature = "rust1", since = "1.0.0")]
#[stable(feature = "copied", since = "1.35.0")]
#[rustc_const_unstable(feature = "const_option_ext", issue = "91930")]
pub const fn unwrap_or_default(self) -> T
pub const fn copied(self) -> Option<T>
where
T: ~const Default,
T: Copy,
{
match self {
Some(x) => x,
None => Default::default(),
}
}
}

impl<T: Deref> Option<T> {
/// Converts from `Option<T>` (or `&Option<T>`) to `Option<&T::Target>`.
///
/// Leaves the original Option in-place, creating a new one with a reference
/// to the original one, additionally coercing the contents via [`Deref`].
///
/// # Examples
///
/// ```
/// let x: Option<String> = Some("hey".to_owned());
/// assert_eq!(x.as_deref(), Some("hey"));
///
/// let x: Option<String> = None;
/// assert_eq!(x.as_deref(), None);
/// ```
#[stable(feature = "option_deref", since = "1.40.0")]
#[rustc_const_unstable(feature = "const_option_ext", issue = "91930")]
pub const fn as_deref(&self) -> Option<&T::Target>
where
T: ~const Deref,
{
match self.as_ref() {
Some(t) => Some(t.deref()),
Some(&mut t) => Some(t),
None => None,
}
}
}

impl<T: DerefMut> Option<T> {
/// Converts from `Option<T>` (or `&mut Option<T>`) to `Option<&mut T::Target>`.
///
/// Leaves the original `Option` in-place, creating a new one containing a mutable reference to
/// the inner type's [`Deref::Target`] type.
/// Maps an `Option<&mut T>` to an `Option<T>` by cloning the contents of the
/// option.
///
/// # Examples
///
/// ```
/// let mut x: Option<String> = Some("hey".to_owned());
/// assert_eq!(x.as_deref_mut().map(|x| {
/// x.make_ascii_uppercase();
/// x
/// }), Some("HEY".to_owned().as_mut_str()));
/// let mut x = 12;
/// let opt_x = Some(&mut x);
/// assert_eq!(opt_x, Some(&mut 12));
/// let cloned = opt_x.cloned();
/// assert_eq!(cloned, Some(12));
/// ```
#[stable(feature = "option_deref", since = "1.40.0")]
#[rustc_const_unstable(feature = "const_option_ext", issue = "91930")]
pub const fn as_deref_mut(&mut self) -> Option<&mut T::Target>
#[must_use = "`self` will be dropped if the result is not used"]
#[stable(since = "1.26.0", feature = "option_ref_mut_cloned")]
#[rustc_const_unstable(feature = "const_option_cloned", issue = "91582")]
pub const fn cloned(self) -> Option<T>
where
T: ~const DerefMut,
T: ~const Clone,
{
match self.as_mut() {
Some(t) => Some(t.deref_mut()),
match self {
Some(t) => Some(t.clone()),
None => None,
}
}
Expand Down
Loading

0 comments on commit 13e2840

Please sign in to comment.