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.

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

Tracking Issue for Option::zip and Option::zip_with (feature option_zip) #70086

Open
1 of 4 tasks
LukasKalbertodt opened this issue Mar 17, 2020 · 15 comments
Open
1 of 4 tasks
Labels
A-result-option Area: Result and Option combinators B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@LukasKalbertodt
Copy link
Member

LukasKalbertodt commented Mar 17, 2020

This is a tracking issue for #![feature(option_zip)] which was introduced in #69997.

About tracking issues

Tracking issues are used to record the overall progress of implementation.
They are also uses as hubs connecting to other relevant issues, e.g., bugs or open design questions.
A tracking issue is however not meant for large scale discussion, questions, or bug reports about a feature.
Instead, open a dedicated issue for the specific matter and add the relevant feature gate label.

Steps

Unresolved Questions

  • Should we also include zip_with in the public API? Seems like a niche API and is easily replaced with a.zip(b).map(f).
@LukasKalbertodt LukasKalbertodt added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Mar 17, 2020
@cwfitzgerald
Copy link

cwfitzgerald commented May 25, 2020

For what it's worth: my personal two cents are that both should be included, as even if zip_with's use may be a bit more rare, I think the pattern of (Option, Option) -> Option<Something> is common enough, and the terseness win is great enough, that it should be kept.

Edit: One note is that zip_with and zip().map() take different functions as argument, the former takes FnOnce(T, U) -> R the other takes FnOnce((T, U)) -> R, the former of which is generally more directly useful to calling off to other methods.

Edit 2: Is there a better place for this discussion, or is here okay?

@WaffleLapkin
Copy link
Member

Edit: One note is that zip_with and zip().map() take different functions as argument, the former takes FnOnce(T, U) -> R the other takes FnOnce((T, U)) -> R, the former of which is generally more directly useful to calling off to other methods.

Yeah, I've just found the same thing. E.g.:

let a: Option<i32> = ...;
let b: Option<i32> = ...;
let res = a.zip_with(b, Add::add);
// or
let res = a.zip(b).map(|(a, b)| a + b); // destruction needed 

I think in this example zip_with is nicer/clearer than zip+map

@tesuji
Copy link
Contributor

tesuji commented Jun 3, 2020

I opened a PR to stabilize Option::zip only: #72938

@qm3ster
Copy link
Contributor

qm3ster commented Jun 10, 2020

I was originally looking for this, but I feel like the upcoming try_blocks greatly diminished the uses.

let a = Some(7);
let b = Some(true);
let out = a.iter().zip(b).map(|(a, b)| s * o as u8).next(); //stable
let out = a.zip(b).map(|(a, b)| a * b as u8); // #![feature(option_zip)]
let out = a.zip_with(b, |a, b| a * b as u8); // #![feature(option_zip)]
let out: Option<_> = try { a? * b? as u8 }; // #![feature(try_blocks)]

wow, such expressivity!
@WaffleLapkin's case:

let res = a.zip_with(b, Add::add);
let res = try { a? + b? };

Option::zip is still king for the trivial zipping action in isolation:

wants_tuple(a.iter().zip(b).next()); //stable
wants_tuple(a.zip(b)); // #![feature(option_zip)]
wants_tuple(try { (a? * b?) }); // #![feature(try_blocks)]

@WaffleLapkin
Copy link
Member

One interesting note is that try { (a?, b?) } is lazy, where is a.zip(b) is not. However, I personally think that the later is easier to read (I think that early returns are weird in cases like that or Ok(res?)).

Maybe we should also implement smt like this:

fn zip_lazy(self, f: impl FnOnce() -> Option<U>) -> Option<T, U> { ... }

? 🤔

RalfJung added a commit to RalfJung/rust that referenced this issue Jun 15, 2020
Stabilize Option::zip

This PR stabilizes the following API:

```rust
impl<T> Option<T> {
    pub fn zip<U>(self, other: Option<U>) -> Option<(T, U)>;
}
```

This API has real world usage as seen in <https://grep.app/search?q=-%3E%20Option%3C%5C%28T%2C%5Cs%3FU%5C%29%3E&regexp=true&filter[lang][0]=Rust>.

The `zip_with` method is left unstably as this API is kinda niche
and it hasn't received much usage in Rust repositories on GitHub.

cc rust-lang#70086
RalfJung added a commit to RalfJung/rust that referenced this issue Jun 15, 2020
Stabilize Option::zip

This PR stabilizes the following API:

```rust
impl<T> Option<T> {
    pub fn zip<U>(self, other: Option<U>) -> Option<(T, U)>;
}
```

This API has real world usage as seen in <https://grep.app/search?q=-%3E%20Option%3C%5C%28T%2C%5Cs%3FU%5C%29%3E&regexp=true&filter[lang][0]=Rust>.

The `zip_with` method is left unstably as this API is kinda niche
and it hasn't received much usage in Rust repositories on GitHub.

cc rust-lang#70086
flip1995 pushed a commit to flip1995/rust that referenced this issue Jun 23, 2020
Stabilize Option::zip

This PR stabilizes the following API:

```rust
impl<T> Option<T> {
    pub fn zip<U>(self, other: Option<U>) -> Option<(T, U)>;
}
```

This API has real world usage as seen in <https://grep.app/search?q=-%3E%20Option%3C%5C%28T%2C%5Cs%3FU%5C%29%3E&regexp=true&filter[lang][0]=Rust>.

The `zip_with` method is left unstably as this API is kinda niche
and it hasn't received much usage in Rust repositories on GitHub.

cc rust-lang#70086
@KodrAus KodrAus added A-result-option Area: Result and Option combinators Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. labels Jul 29, 2020
@lebensterben
Copy link
Contributor

lebensterben commented Sep 1, 2020

There's one use case, suppose foo and bar are Option<Vec<T>>.
With this new feature, we can have
foo.zip_with(bar, Vec::extend) for example.
If we use zip() and map(), that'd be
foo.zip(bar).map(|first, second| first.extend(second))
That's just unnecessarily long.

@qm3ster
Copy link
Contributor

qm3ster commented Sep 5, 2020

@lebensterben do you mean foo: Option<&mut Vec<T>>? Or foo.as_mut() (in either)?
Because otherwise it would also be immediately dropped.

foo.as_mut().map(|first| Some(first.append(&mut bar?)));
// or
let _: Option<_> = try{ foo.as_mut()?.append(&mut bar?) };

@nixypanda
Copy link

I really like the interface haskell's lift functions have

liftA :: Applicative f => (a -> b) -> f a -> f b
liftA3 :: Applicative f => (a -> b -> c -> d) -> f a -> f b -> f c -> f d
liftA2 :: (a -> b -> c) -> f a -> f b -> f c

Maybe we can have something similar in Rust

Option::lift(f, optional1)
Option::lift2(f, optional1, optiona2)
// etc

Probably the same interface for all applicatives (Iterators, Result, etc).

@sollyucko
Copy link
Contributor

Would lift be equivalent to map and lift2 equivalent to zip_with?

@nixypanda
Copy link

nixypanda commented Dec 9, 2020

@sollyucko yeah! If we consider doing it the number up to which we should have these functions is debatable.

dtolnay added a commit to dtolnay/clang-ast that referenced this issue Apr 28, 2021
Required by the use of Option::zip.

    error[E0658]: use of unstable library feature 'option_zip'
       --> src/loc.rs:551:14
        |
    551 |             .zip(self.expansion_loc.as_ref())
        |              ^^^
        |
        = note: see issue #70086 <rust-lang/rust#70086> for more information

    error[E0658]: use of unstable library feature 'option_zip'
       --> src/loc.rs:529:36
        |
    529 |             spelling_included_from.zip(expansion_included_from).map_or(
        |                                    ^^^
        |
        = note: see issue #70086 <rust-lang/rust#70086> for more information
@sdroege
Copy link
Contributor

sdroege commented Jul 27, 2021

Should we also include zip_with in the public API? Seems like a niche API and is easily replaced with a.zip(b).map(f).

We have quite many calls to zip() followed by map() now in the GStreamer Rust bindings for working with Option<u64>, e.g. to add them together. x.zip(y).map(|(x, y)| x + y) is more annoying to type than x.zip_with(y, |x, y| x + y) but I can see the point of not including it because if you include it, why not also have a shorter form (zip_and_then?) of the equally often used x.zip(y).and_then(|(x, y)| x.checked_add(y)).

@qm3ster
Copy link
Contributor

qm3ster commented Jul 27, 2021

@sdroege

let a = try { x? + y? };
let b = try { x?.checked_add(y?)? };

feels_good

@LoganDark
Copy link

Hell yeah another useful utility function locked behind unstable, unchanged for almost 3 years

@WaffleLapkin
Copy link
Member

@LoganDark fyi Option::zip is stabilized. zip_with is not, but it can be emulated with a.zip(b).map(|(x, y)| f(x, y)).

I got under the impression that T-libs did not want to stabilize zip_with at all, maybe it should be removed (or maybe the opinions have changed since last time I checked).

@LoganDark
Copy link

LoganDark commented Aug 24, 2023

fyi Option::zip is stabilized.

oh, it is? I accidentally autocompleted zip_with and then corrected it to zip but thought that was still included in the option_zip feature, oops. can confirm zip itself is stabilized, thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-result-option Area: Result and Option combinators B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests