Add `Option::merge`

[kurtlawrence]

Proposal

Problem statement

Merging two options must currently be done via pattern matching, resulting in verbose code with the potential for errors.

Motivating examples or use cases

A simple example would be adding two numbers if both exist, or taking one or the other if either exist:

fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    match (a, b) {
        (Some(a), Some(b)) => Some(a + b),
        (None, b) => b,
        (a, None) => a,
    }
}

I have come across a concrete example where many Option<Aabb>s needed to be unioned together.

Solution sketch

I figure a simple implementation would suffice. Potentially, a merge_mut might be implemented which takes Option<&mut T> and uses FnOnce(&mut T, T)

impl Option<T>
  fn merge<F>(self, b: Option<T>, f: F) -> Option<T>
  where
      F: FnOnce(T, T) -> T,
  {
      match (a, b) {
          (Some(a), Some(b)) => Some(f(a, b)),
          (None, b) => b,
          (a, None) => a,
      }
  }
}

Thus, the example above becomes

fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    a.merge(b, |a, b| a + b)
}

and is chainable.

Alternatives

and/or do not handle keeping both sides, whilst zip only works if you have both sides. If T: Copy a combination of zip and or can work: a.zip(b).map(|(a,b)| a + b).or(a).or(b). This is still a mouthful.

Not fussed on the name. merge only comes to mind since I've used something similar in Elm

Links and related work

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

• We think this problem seems worth solving, and the standard library might be the right place to solve it.
• We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

• We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
• We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.

[programmerjake]

maybe generalize it somewhat:

impl<T> Option<T> {
    fn merge<U, R, F>(self, other: Option<U>, f: F) -> Option<R>
    where
        T: Into<R>,
        U: Into<R>,
        F: FnOnce(T, U) -> R,
    {
        match (self, other) {
            (Some(a), Some(b)) => Some(f(a, b)),
            (Some(a), _) => Some(a.into()),
            (_, Some(b)) => Some(b.into()),
            _ => None,
        }
    }
}

[shepmaster]

where many Option<Aabb>s needed to be unioned

Another way to do this today is to use iterators:

fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    [a, b].into_iter().flatten().reduce(|a, b| a + b)
}

[pitaj]

Is this not the same as Option::zip_with?

Oh I guess not, because that returns None if either is None, whereas this would only return None when both are None.

[m-ou-se]

We briefly discussed this in the libs-api meeting.

We generally have a pretty high bar for adding more methods to types like Option, and we're note sure if this method meets it. Would merge() be clearer than a if let or match statement?

Does Iterator::reduce also work for your use cases? If not, it'd be useful to mention it in your APC and address the similarity with Iterator::reduce and for which use cases Option::merge would be a significantly better solution.

Thanks!

[kennytm]

fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    [a, b].into_iter().flatten().reduce(|a, b| a + b)
}

this could be further simplified as

fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    a.into_iter().chain(b).reduce(|a, b| a + b)
}

it also generates much simpler ASM similar to the hand-written match.

[kurtlawrence]

Would merge() be clearer than a if let or match statement?

Personally, I find the pattern matching harder to follow in this case.

fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    a.into_iter().chain(b).reduce(|a, b| a + b)
}

This works well enough in lieu of a dedicated Option::merge, although it (obviously) isn't a clear approach. Would additional documentation in the option module giving the above example be amenable?

[pitaj]

I wouldn't consider the iterator code readable either. Seems like since zip_with exists (the AND case) this should also exist (the OR case).

Another option without iterators

fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    a.and_then(|a| Some(a + b?)).or(b)
}

[the8472]

There already is is the Iterating over Option section.

Perhaps it could be more direct and say something like "options are iterable, so all the iterator combinators apply".

Seems like since zip_with exists (the AND case) this should also exist (the OR case).

Those are generally poor arguments because if we apply it inductively it means all possible logic operations should have their own methods. And the issue with that is that those would be specific to options. Adding all the same methods to result would need different names because there we need to distinguish Ok/Err/applies-to-both cases. Which makes Option its own little microcosm of operations you need to remember that don't transfer to other parts of the language.

[pitaj]

Another option using zip_with

#![feature(option_zip)]
fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    a.zip_with(b, |a, b| a + b).or(a).or(b)
}

[kurtlawrence]

Another option using zip_with

#![feature(option_zip)]
fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    a.zip_with(b, |a, b| a + b).or(a).or(b)
}

These options do not work when T does not implement Copy.

[m-ou-se]

What's the use case for an add function like this?

fn add(a: Option<i32>, b: Option<i32>) -> Option<i32> {
    a.merge(b, |a, b| a + b)
}

In all cases where I personally added two Option<i32>, I used None in the same way as f32::NAN: if either side if the + are NAN, the result should be NAN.

When would you use an add for two Option<i32> that only results in None when both inputs are None, but treats them as zero otherwise?

[Amanieu]

It seems like the main use case here ("many Option<Aabb>s needed to be unioned together") involves a large number of Options that need to be merged into one. This is much better handled using iterator adapters than pairwise merging:

fn merge_aabbs(slice: &[Option<Aabb>]) -> Aabb {
    // flatten treats the Option as an iterator, effectively skipping empty options.
    // You could also do this with filter_map, which may be clearer.
    slice.iter().flatten().reduce(|a, b| a.union(b))
}

[kurtlawrence]

The add was just a minimal example.

My gripe came from having multiple AABB queries/calculations within a function and trying to tie them all together. The function has additional constraints on mutably accessing a cache and accessing visibility hierarchies, so chaining together iterators doesn't work, each query set would need to resolve itself first.

I ended up using something similar to

a.into_iter()
    .chain(b)
    .chain(c)
    // ... more
    .reduce(|a, b| a.union(b))

I'll close out, I can appreciate avoiding bloating the API.