Repository: nushell/nushell
File: crates/nu-engine/src/compile/operator.rs (Rust)

Code before:
    } else {
        // Not an assignment: just do the binary op
        let lhs_reg = out_reg;

        compile_expression(
            working_set,
            builder,
            lhs,
            RedirectModes::value(lhs.span),
            None,
            lhs_reg,
        )?;

        match op.item {
            // `and` / `or` are short-circuiting, use `match` to avoid running the RHS if LHS is
            // the correct value. Be careful to support and/or on non-boolean values
            Operator::Boolean(bool_op @ Boolean::And)
            | Operator::Boolean(bool_op @ Boolean::Or) => {
                // `and` short-circuits on false, and `or` short-circuits on true.
                let short_circuit_value = match bool_op {
                    Boolean::And => false,
                    Boolean::Or => true,
                    Boolean::Xor => unreachable!(),
                };

                // Before match against lhs_reg, it's important to collect it first
                builder.push(Instruction::Collect { src_dst: lhs_reg }.into_spanned(lhs.span))?;
                // Short-circuit to return `lhs_reg`. `match` op does not consume `lhs_reg`.
                let short_circuit_label = builder.label(None);
                builder.r#match(
                    Pattern::Value(Value::bool(short_circuit_value, op.span)),
                    lhs_reg,
                    short_circuit_label,
                    op.span,
                )?;

                // If the match failed then this was not the short-circuit value, so we have to run
                // the RHS expression
                let rhs_reg = builder.next_register()?;
                compile_expression(
                    working_set,
                    builder,
                    rhs,
                    RedirectModes::value(rhs.span),
                    None,
                    rhs_reg,
                )?;

                // It may seem intuitive that we can just return RHS here, but we do have to
                // actually execute the binary-op in case this is not a boolean
                builder.push(

Review comment (bug):
For future reference if you could add a small blurb on why it is important could be helpful, otherwise the assumption could become we would always have to collect and not for a particular reason.

(There may be good reasons to optimize away a collect and failr earlier.)

Code after:
    } else {
        // Not an assignment: just do the binary op
        let lhs_reg = out_reg;

        compile_expression(
            working_set,
            builder,
            lhs,
            RedirectModes::value(lhs.span),
            None,
            lhs_reg,
        )?;

        match op.item {
            // `and` / `or` are short-circuiting, use `match` to avoid running the RHS if LHS is
            // the correct value. Be careful to support and/or on non-boolean values
            Operator::Boolean(bool_op @ Boolean::And)
            | Operator::Boolean(bool_op @ Boolean::Or) => {
                // `and` short-circuits on false, and `or` short-circuits on true.
                let short_circuit_value = match bool_op {
                    Boolean::And => false,
                    Boolean::Or => true,
                    Boolean::Xor => unreachable!(),
                };

                // Before match against lhs_reg, it's important to collect it first to get a concrete value if there is a subexpression.
                builder.push(Instruction::Collect { src_dst: lhs_reg }.into_spanned(lhs.span))?;
                // Short-circuit to return `lhs_reg`. `match` op does not consume `lhs_reg`.
                let short_circuit_label = builder.label(None);
                builder.r#match(
                    Pattern::Value(Value::bool(short_circuit_value, op.span)),
                    lhs_reg,
                    short_circuit_label,
                    op.span,
                )?;

                // If the match failed then this was not the short-circuit value, so we have to run
                // the RHS expression
                let rhs_reg = builder.next_register()?;
                compile_expression(
                    working_set,
                    builder,
                    rhs,
                    RedirectModes::value(rhs.span),
                    None,
                    rhs_reg,
                )?;

                // It may seem intuitive that we can just return RHS here, but we do have to
                // actually execute the binary-op in case this is not a boolean
                builder.push(