Skip to content

Traits for short-circuiting || and && #144

@est31

Description

@est31

Problem statement

I propose addition of two traits And and Or for making the && and || operators overrideable.

Motivation, use-cases

It would be worth to do it for consistency reasons: the other operators can be overridden, too. IIRC && and || are the only operators for which there isn't at least an unstable trait yet.

For usage of && in c++ one can check grep.app, same for ||.

Solution sketches

Generally, I would suggest following the conventions laid down by the other traits in std::ops. The function names inside the traits all seem to contain the trait name, e.g. BitAnd has a bitand function, Div has a div function. This is useful because if you have implemented multiple op traits on some type you can just do div and you know that there is no conflict with another trait. Thus, I would suggest also putting the trait name into all And and Or functions.

The names And and Or come from Rust's internal representation, where they get called the same (AST does not distinguish & and &&, here && is just two And's, but HIR has BitAnd and And, as does MIR). This is in distinction to BitAnd which is &. Note that the way proc_macro exposes && and || is through Punct so there is no stable API that already exposes a name for &&.

The current behaviour of && and || on bool exposes short-circuiting behaviour, so if the left hand side is false, && does not evaluate the right hand side, and if it is true || does not evaluate the right hand side. In order to make it possible for bools to use the traits, and allow users to customize it, a simple trait with one function and or or is not enough, we need a second function to decide whether the right hand side should be evaluated at all.

I would propose the following design for And:

trait And<Rhs = Self> {
    type Output;
    type Intermediate;
    // gets called if and_should_eval_rhs returned Continue.
    fn and(this: Self::Intermediate, other: Rhs) -> Self::Output;
    // gets called before the and function to determine if there is a
    // short circuit or not.
    fn and_should_eval_rhs(self) -> std::ops::ControlFlow<Self::Output, Self::Intermediate>;
}

Or would be analogous. In the standard library, the two traits should be implemented on bool only (outside of tests).

Alternatively to rhs, we could also name it right, next or such. That there is already prior art in other binop traits via the Rhs generic parameter name, but of course that name is not subject to any stability guarantees.

let ret = foo().bar() && foo2().bar2()would be lowered to something like:

let ret = {
    let val = foo().bar();
    match And::and_should_eval_rhs(val) {
        ControlFlow::Continue(val) => And::and(val, foo2().bar2()),
        ControlFlow::Break(other) => other,
    }
};

The signature of and_should_eval_rhs takes self by-value to return it again so that self can be used in the Break case to construct Self::Output. This move-on-happy-path behavior might potentially be wasteful for larger types, but it's already an existing problem of Rust's operators, and adding yet another and_early_exit(self) -> Self::Output function would not be worth it. For large types, one can impl it on &mut BigType or &BigType instead, and use those instead.

On nightly, && chains belonging to if might contain let statements that might fail. Those will be represented as bools, so if someone wants let statements to work in their && chain, they need to implement And<Rhs=bool>.

Links and related work

Originally @scottmcm suggested making these trait-overrideable in zulip.

C++ has prior art via an operator &&, that is similarly implemented on bool. I'm not sure how short-circuiting is handled for non-bool return types.

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

Metadata

Metadata

Assignees

No one assigned

    Labels

    T-libs-apiapi-change-proposalA proposal to add or alter unstable APIs in the standard libraries

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions