Pre-RFC: Overload Short Curcuits

[H2CO3]

@scottmcm

Can you elaborate on exactly what you mean by that, and whether that’s what this proposal actually is?

For sure it is. Whether the "treat me as a Boolean" magic happens in operator argument position or in if condition position isn't of relevance, I think. Sure, this RFC doesn't propose the latter, but it does propose the former.

@scottmcm

This seems like a strawman to me,

The same goes for this case.

@scottmcm

But Option and Result already have methods using “or” and “and”

I was sure someone will bring up this argument. The naming of these methods is not great; there are several cases where the high-level semantics doesn't match this notion, e.g. the Rust MongoDB driver regularly exposes APIs where an Option<Error> is returned along with other, unconditionally-returned information (hence, a Result wouldn't make much sense or would be less convenient to handle), where None means success and Some means failure. So, using the and and or methods of Option leads to the exact opposite behavior of what one would expect. This is why these methods shouldn't have tried to second-guess the usage of the type and stick to the low-level functions they perform; something like .if_some_then() and .if_none_then() would have been much better.

[bugaevc]

@timvermeulen

I think that trait should ideally get another Intermediate / Falsy / Empty / Failure associated type though, to be used in the Long variant of the return value of short_circuit_or . For bool and Option it could be () , but other types may want to pass some value other than self to logical_or .

TBH I don't think this is useful, because any transformation short_circuit_or() can do to get the intermediate value, logical_or() can do as well. The reason we're actually returning a value with ShortCircuit::Long is to get ownership over self back, not to pass some useful info. Perhaps

/// ... otherwise return the value
/// that will get passed to `logical_or()` (normally this
/// means returning self back, but you can change the value).

should be just

/// otherwise, pass the ownership over `self` back.

Or do you have a use case in mind?

Oh, and I haven't bikeshed the names enough, so suggestions welcome

[bugaevc]

Another reason I would like logical_or() to take self and not some intermediate type is that this way, it’s possible to call LogicalOr::logical_or() explicitly if you want a non-short-circuiting or, without going through short_circuit_or().

[timvermeulen]

@bugaevc

TBH I don’t think this is useful, because any transformation short_circuit_or() can do to get the intermediate value, logical_or() can do as well.

True, but doing that transformation twice might be undesirable and is certainly less type-safe than passing the transformed value instead.

@bugaevc

Or do you have a use case in mind?

In theory Result<T, E> could implement LogicalOr<Result<T, F>> with Output = Result<T, (E, F)>. It probably won't, given that that's not how Result::or works, but a type similar to Result may want to do something like this. It doesn't seem very far-fetched to me.

@bugaevc

The reason we’re actually returning a value with ShortCircuit::Long is to get ownership over self back, not to pass some useful info.

@bugaevc

this way, it’s possible to call LogicalOr::logical_or() explicitly if you want a non-short-circuiting or

This makes sense to me if you ever want to call this method explicitly (if not, it could just only take the rhs instead of both). Would you ever want to do this? I see the usefulness of having a method that directly corresponds to the operator, but here the method and the operator would behave differently.

Your example implementations have asserts in the body of logical_or to make sure that the lhs is false / None. If you want to encourage people to call this method directly, it should also handle the case where the lhs is true / Some, which would then lead to duplicate logic.

If we want this operator to have a method equivalent then I think it should probably be a third method on that trait with an implementation that calls the other two. And in that situation it's not required for logical_or to take self.

[bugaevc]

@timvermeulen

In theory Result<T, E> could implement LogicalOr<Result<T, F>> with Output = Result<T, (E, F)> . It probably won’t, given that that’s not how Result::or works, but a type similar to Result may want to do something like this. It doesn’t seem very far-fetched to me.

@timvermeulen

This makes sense to me if you ever want to call this method explicitly (if not, it could just only take the rhs instead of both).

That is exactly why logical_or() needs to take self (or indeed some intermediate value) even if you're not going to call it explicitly. I have a similar impl for Options in the playbin; here's how it would look like for Results:

impl<T, E, F> LogicalOr<Result<T, F>> for Result<T, E> {
    type Output = Result<T, (E, F)>;

    fn short_circuit_or(self) -> ShortCircuit<Self::Output, Self> {
        match self {
            Ok(t) => ShortCircuit::Short(Ok(t)),
            Err(e) => ShortCircuit::Long(Err(e)),
        }
    }

    fn logical_or(self, other: Result<T, F>) -> Self::Output {
        let e = match self {
            Ok(t) => return Ok(t),
            Err(e) => e,
        };
        let f = match other {
            Ok(t) => return Ok(t),
            Err(e) => f,
        };
        Err((e, f))
    }
}

@timvermeulen

Your example implementations have assert s in the body of logical_or to make sure that the lhs is false / None . If you want to encourage people to call this method directly, it should also handle the case where the lhs is true / Some , which would then lead to duplicate logic.

Yeah, when I wrote that, I didn't consider that you might want to call it independently of short_circuit_or().

True, there may be some duplication, but if the logic is complicated enough the duplication becomes a problem, it's likely a bad fit for what an operator should do anyway. I mean, I understand what you're saying, and indeed with a third type the implementations could be a bit more clear ('cause they wouldn't have unwrap self in logical_or() again), but I'm not sure that justifies adding extra "weight" (another type, and likely another method) to the trait, especially given the implementations should be pretty simple in any case.

[timvermeulen]

@bugaevc

True, there may be some duplication, but if the logic is complicated enough the duplication becomes a problem, it’s likely a bad fit for what an operator should do anyway.

It's not just duplicate Rust code, it's also duplicate machine code. The compiler will probably optimize it away for simple types like bool / Option / Result, but maybe it's better to not have to rely on that.

@bugaevc

I understand what you’re saying, and indeed with a third type the implementations could be a bit more clear ('cause they wouldn’t have unwrap self in logical_or() again), but I’m not sure that justifies adding extra “weight” (another type, and likely another method) to the trait, especially given the implementations should be pretty simple in any case.

You wouldn't need to implement the extra method yourself, so the only extra weight is the additional associated type

[Nokel81]

@H2CO3

where None means success and Some means failure. So, using the and and or methods of Option leads to the exact opposite behavior of what one would expect

However that is not meant by Option in general. None means specifically nothing here or not contained. So the "truthy" or maybe the "short-circuity" part makes a lot of sense to be over the None case.

[scottmcm]

@H2CO3

For sure it is. Whether the “treat me as a Boolean” magic happens in operator argument position or in if condition position isn’t of relevance, I think.

Except at no point does it actually become a bool -- even in operator argument position -- so the problems I'm most familiar with for operator bool wouldn't happen. I know why "0" && "2" shouldn't compile, but the arguments I know against it don't apply to None || 2_i32 producing an i32.

(I would also be against various other boolification things such as if None || None were proposed to evaluate to false, because I agree they shouldn't become bools. But that's not what's proposed.)

You clearly have some experience with other languages that you consider "sloppy". Don't just say "learn from past mistakes", tell me about those problems, and show me how this proposal has those same problems. I'm always happy to learn about a new ['1', '7', '11'].map(parseInt) ==> [1, NaN, 3], and even happier when rust doesn't grow such things

@H2CO3

there are several cases where the high-level semantics doesn’t match this notion, e.g. the Rust MongoDB driver regularly exposes APIs where an Option<Error> is returned along with other, unconditionally-returned information (hence, a Result wouldn’t make much sense or would be less convenient to handle), where None means success and Some means failure.

This feels to me like the argument that we shouldn't allow overloading << because someone might use it for output instead of shifting. That's an incorrect use of Option.

@H2CO3

This is why these methods shouldn’t have tried to second-guess the usage of the type

It's more than just those methods, though. Using ? with Option is also using the None-is-false-like meaning. So long as types aren't structural, their fields and variants have meaning.

[Nokel81]

@scottmcm

I would also be against various other boolification things such as if None || None were proposed to evaluate to false , because I agree they shouldn’t become bool s. But that’s not what’s proposed

So would I, I think that None || None should produce None

[Nokel81]

So instead of a general solution, why not a local solution just for Result<T, E> and Option<T>?

Namely, the compiler will compile <l_expr> || <r_expr> given the following and being the following type.

# | type <l_expr> | type <r_expr> | type whole
--+---------------+---------------+------------
1 | Option<T>     | Option<T>     | Option<T>
--+---------------+---------------+------------
2 | Option<T>     | <T>           | <T>
--+---------------+---------------+------------
3 | Result<T, E>  | Result<T, E>  | Result<T, E>
--+---------------+---------------+------------
4 | Result<T, E>  | <T>           | <T>

With the following semantics:

1. if l_expr.is_some() { l_expr } else { r_expr }
2. if let Some(l) = l_expr { l } else { r_expr }
3. if l_expr.is_ok() { l_expr } else { r_expr }
4. if let Ok(l) = l_expr { l } else { r_expr }

The case of && will eventually be covered by try { ... } blocks and does not need to be covered here.

[RustyYato]

I dislike special casing, and this would also make Option and Result lang items, which is undesirable.

[Nokel81]

That is fair, even though I doubt that they will ever be moved out of std. Not that that is a reason to do something like this.

I also don’t completely understand the arguments as to why this shouldn’t be allowed to be implemented (maybe even without requiring BitOr) because someone might not use the short circuit nature of it all. Someone can do that with Add anyway (pretending that it is Minus).

They why not a trait that has the following methods:

trait ShortCircutOr<T> {
    type Output;

    fn should_short_circuit(left: &Self) -> bool;
    fn or(left: Self, right: T) -> Output;
}

And the semantics for l || r is

if ShortCircutOr<R>::should_short_circuit(l) {
    l
} else { 
    ShortCircutOr<R>::or(l, r)
}

And that means that the following impls should work:

impl ShortCircutOr<Option<T>> for Option<T> {
    type Output = Option<T>;

    fn should_short_circuit(left: &Self) -> bool {
        left.is_none()
    }

    fn or(left: Self, right: T) -> Output {
        right
    }
}

[RustyYato]

This trait wouldn't work if Output != Self. So you can just remove it, but if you do that then you can't do ShortCircutOr<T> for Option<T> which seems desirable from the discussion above. Instead we could go with @bugaevc's solution, which is more robust and properly handles both cases of Option<T> || Option<T> and Option<T> || T.

For reference:

@bugaevc

enum ShortCircuit<S, L> {
    Short(S),
    Long(L),
}

trait LogicalOr<Rhs = Self>: Sized {
    type Output;

    /// Decide whether the *logical or* should short-circuit
    /// or not based on its left-hand side argument. If so,
    /// return its final result, otherwise return the value
    /// that will get passed to `logical_or()` (normally this
    /// means returning self back, but you can change the value).
    fn short_circuit_or(self) -> ShortCircuit<Self::Output, Self>;

    /// Complete the *logical or* in case it did not short-circuit.
    /// Normally this would just return `rhs`.
    fn logical_or(self, rhs: Rhs) -> Self::Output;
}

then A() || B() desugars to

match A().short_circuit_or() {
    ShortCircuit::Short(res) => res,
    ShortCircuit::Long(l) => l.logical_or(B()),
}

[Nokel81]

I would actually be fine with this signature. Would making it availabile for short circuit and make sense or not because try blocks?

[diwic]

Here’s what I would have expected A() || B() to desugar to - no traits, no new lang items, no nothin’

{
    let temp = A();
    if (&temp).into::<bool>() { temp } else { B() }
}

We might have to implement From / Into for a few &T though.

[RustyYato]

We don’t want to have a notion of truthy and falsely. That would be very bad. There is a reason why JavaScript is looked doen upon. These sorts of automatic conversions are a huge source of errors.

Also, traits are the building blocks of abstraction in Rust, so all generic behaviors will be specified via traits.

[Nokel81]

That is undesirable because we don’t really want bool conversion. And we still want the ability to define how each type actually implements the operation.

Sebastian Malton

[scottmcm]

So there have been a bunch of proposed desugarings here, but there doesn’t seem to be consensus on what the semantics of the operation should even be.

As such, I recommend that people start by posting examples of what they would expect an overridden || to actually do.

I’ll start with mine from here:

Some(4) || 2 ==> 4
Some(4) || Some(7) || 2 ==> 4
None || None || None || 2 ==> 2
∀x, None || x ==> x
Some(1) || panic!() ==> 1
Some("hello") || "" ==> "hello"
None || Some("world") ==> Some("world")

Some(2) || Some(3) // ERROR: type mismatch i32 vs Option<i32>
Some(2) || "hello" // ERROR: type mismatch i32 vs Option<&str>
4 || "hello" // ERROR: || cannot be applied to i32
"hello" || None // ERROR: || cannot be applied to &str

I’d also be interested in hearing what other types with which you’d expect to use ||. For example, what types were you thinking would be Into<bool>, @diwic, to use your desugar? Right now there are none.

[diwic]

I’m not sure why bool conversion is worse than the Result conversion we already have in ? and that everyone seems to like, but if you prefer Result conversion:

    if let Ok(temp) = A().into() { temp } else { B() }

[RustyYato]

Some(2) || Some(3) // ERROR: type mismatch i32 vs Option

I disagree with this one, this one should return Some(2), otherwise None || None || None || 2 ==> 2 wouldn't make sense, because we would have to evaluate None || None at some point, and that only works if we can also evaluate Some(x) || Some(y)

For Option<T> it should be

let t: T;
let opt_t: Option<T>;

Some::<T>(x)    || t     == x
Some::<U>(x)    || t     == "Error: Type mismatch, expected: `U`, found `T`"
None::<T>       || t     == t
None::<U>       || t     == "Error: Type mismatch, expected: `U`, found `T`"

Some::<T>(x)    || opt_t == Some::<T>(x)
Some::<U>(x)    || opt_t == "Error: Type mismatch, expected: `Option<U>`, found `Option<T>`"
None::<T>       || opt_t == opt_t
None::<U>       || opt_t == "Error: Type mismatch, expected: `Option<U>`, found `Option<T>`"