Added either::map_both, either:try_map_both

[JohnScience]

Added an additional line to the doc of either::for_both!:

/// Unlike [`map_both!`], this macro converges both variants to the type returned by the expression.

Implementation of map_both!

/// Evaluate the provided expression for both [`Either::Left`] and [`Either::Right`],
/// returning an [`Either`] with the results.
///
/// This macro is useful in cases where both sides of [`Either`] can be interacted with
/// in the same way even though the don't share the same type.
///
/// Syntax: `either::map_both!(` *expression* `,` *pattern* `=>` *expression* `)`
///
/// Unlike [`for_both!`], this macro returns an [`Either`] with the results of the expressions.
///
/// # Example
///
/// ```
/// use either::Either;
///
/// struct Wrapper<T>(T);
///
/// fn wrap(owned_or_borrowed: Either<String, &'static str>) -> Either<Wrapper<String>, Wrapper<&'static str>> {
///    either::map_both!(owned_or_borrowed, s => Wrapper(s))
/// }
/// ```
#[macro_export]
macro_rules! map_both {
    ($value:expr, $pattern:pat => $result:expr) => {
        match $value {
            $crate::Either::Left($pattern) => $crate::Either::Left($result),
            $crate::Either::Right($pattern) => $crate::Either::Right($result),
        }
    };
}

Implementation of try_map_both!:

/// Evaluate the provided expression for both [`Either::Left`] and [`Either::Right`],
/// returning a [Result] where the [`Ok`] variant is an [`Either`] with the results.
///
/// This macro is useful in cases where both sides of [`Either`] can be interacted with
/// in the same way even though the don't share the same type.
///
/// `either::map_both!(` *expression* `,` *pattern* `=>` *expression* `)`
///
/// Unlike [`map_both!`], this macro returns a [Result] where the [`Ok`] variant is an [`Either`] with the results.
///
/// # Example
///
/// ```
/// use either::Either;
///
/// fn wrap(owned_or_borrowed: Either<String, &'static str>) -> Result<Either<String, &'static str>, ()> {
///    either::try_map_both!(owned_or_borrowed, s => Ok(s))
/// }
/// ```
#[macro_export]
macro_rules! try_map_both {
    ($value:expr, $pattern:pat => $result:expr) => {
        match $value {
            $crate::Either::Left($pattern) => match $result {
                Ok(ok) => Ok($crate::Either::Left(ok)),
                Err(err) => Err(err),
            },
            $crate::Either::Right($pattern) => match $result {
                Ok(ok) => Ok($crate::Either::Right(ok)),
                Err(err) => Err(err),
            },
        }
    };
}

P.S.

If you have an idea for a simple and meaningful example for try_map_both!, it will be very welcome!

[JohnScience]

@cuviper Ready for the review!

[cuviper]

This functionality already exists privately as map_either!:

either/src/lib.rs

Lines 133 to 140 in 53ae3de

	macro_rules! map_either {
	($value:expr, $pattern:pat => $result:expr) => {
	match $value {
	Left($pattern) => Left($result),
	Right($pattern) => Right($result),
	}
	};
	}

So the internal use is evidence enough that this is a useful macro. The existing macro is named like the map_either method, although that takes two separate mapping functions. Your map_both! is similar to the existing for_both!, where "both" is justified because a single $pattern => $result is applied to both variants. That seems fine to me, so we can change names.

Please do update the existing uses to map_both! and remove map_either!.

[cuviper]

I'm less sure about your new addition of try_map_both! because it's limited to Result instead of generic Try, whereas you can probably just use map_both! with an ? operator inside the $result in many cases.

[JohnScience]

I'm less sure about your new addition of try_map_both! because it's limited to Result instead of generic Try, whereas you can probably just use map_both! with an ? operator inside the $result in many cases.

I'm not sure how to implement it properly given that https://doc.rust-lang.org/beta/std/ops/trait.Try.html is unstable.

whereas you can probably just use map_both! with an ? operator inside the $result in many cases.

And it is an amazing idea.

[JohnScience]

I'll have to start working just in 3 minutes, so I'll probably be able to come back to it only in the evening!

Still, thank you a lot for the feedback!

[cuviper]

I'm not sure how to implement it properly given that https://doc.rust-lang.org/beta/std/ops/trait.Try.html is unstable.

Right, you can't, which gives the advantage to the ? operator.

[JohnScience]

I'm not sure how to implement it properly given that https://doc.rust-lang.org/beta/std/ops/trait.Try.html is unstable.

Right, you can't, which gives the advantage to the ? operator.

I reimplemented the code using a hidden and sealed Tryish trait, which mimics a part of the functionality of the Try trait, which is necessary to allow for try_map_both macro work as intended while Try trait is unstable.

Once Try trait is stabilized, we can switch to using Try trait instead of Tryish trait directly.

For the time being, it works with a Result and Option.

[JohnScience]

@cuviper Ready for the second review!

[JohnScience]

There are a small annoyances that make me want to have a dedicated try_map_both! macro.

• If we want to handle a certain subset of errors of the returned Result::Err, we'd have to have a closure/function that would capture the short-circuited with ? error.
• If we write an application-level function with the return type of anyhow::Result<()> and call a function with a custom error, which we would like to handle, we would need to have a closure/function that would capture the short-circuited with ? error.
• Under the hood, the point of converge of types to the result would be spread across the branches, which could create more problems for the optimization passes of LLVM.

[cuviper]

Hidden closures in the macro also have costs, since that prevents control flow and some borrowck patterns that would otherwise be allowed in local code.

Have you considered map_both!(...).factor_err() for your try_map_both! use case?

[JohnScience]

Hidden closures in the macro also have costs, since that prevents control flow and some borrowck patterns that would otherwise be allowed in local code.

Where can I learn more about that problem? It feels more like an issue with the compiler than with the idea.

Have you considered map_both!(...).factor_err() for your try_map_both! use case?

And I guess it'll work but try_map_both! has the potential to support any Try type even before Try is stable.

[cuviper]

Hidden closures in the macro also have costs, since that prevents control flow and some borrowck patterns that would otherwise be allowed in local code.

Where can I learn more about that problem? It feels more like an issue with the compiler than with the idea.

The control flow limitation is straightforward -- stuff like break, continue, and return can't reach outside the closure.

For borrowck, the biggest difference I can think of is with initialization. You can move locals into a closure capture without trouble, but you can't move them back to re-initialize the local, nor can you initialize it for the first time. Without a closure, borrowck is able to track discontinuous regions where a local is initialized.

[cuviper]

FWIW, I do have a private Try in rayon too, but I think that has a much stronger justification for enabling parallel try_fold and such, which would be very difficult for users to accomplish otherwise. It also doesn't have the drawbacks about closures because it's not trying to do anything local like a macro.

Here, we're talking about a convenience macro for a relatively simple match, and I don't think the contortions and drawbacks are justified to emulate Try.

[cuviper]

@JohnScience there's a new request for map_both! in #135. Would you like to scale this PR back to just that?

I think the "try" part would be better left separate, possibly once Try and try{} blocks are stable, though I suspect we may not even need a dedicated macro with those features available.

[JohnScience]

@cuviper I pivoted from Software Engineering to Sales.

I kinda hated the job market.

If there's a free day (I don't really have days off), I might come back and do something about it.

However, it might take me up to 6 months to be really free.

Good luck tho!

[ronnodas]

@cuviper would you like me to file another PR with just map_both!?

[cuviper]

@ronnodas go ahead if you want -- I think you could even cherry-pick from this PR without the "try" commits, as they look reasonably separate.