MP_BINARY_OP_REVERSE for relational operators

[v923z]

Hi all,

in numpy, the following is valid

Code: Select all

a = array([1, 2, 3, 4])
a < 2
2 > a

Both expressions mean the same, the difference is in, whether the array stands on the right or left hand side of the less than/more than operator. In micropython, operand ordering is handled via the

Code: Select all

MP_BINARY_OP_REVERSE_

constants. However, those are not defined for the relational operators. Is there a specific reason for that? If there isn't, would it be too much hassle to implement them? I am willing to mount an effort, but before doing that, I wanted to see, whether they were excluded on purpose.

Cheers,

Zoltán

[jimmo]

Yes this isn't currently implement in MicroPython.

However, I don't think MP_BINARY_OP_REVERSE_ is the way this is supposed to work -- that's specifically for making sure it calls (for example) __radd__ instead of __add__ on the second attempt (see the logic in runtime.c, mp_binary_op). Python doesn't have __rle__, etc.

Comparisons are handled differently to the other binary ops. This is described in PEP 207 "Rich Comparisons". I believe the correct behavior is that (specifically for the comparison ops, lt, le, gt, ge), if type->binary_op returns MP_OBJ_NULL then it should have another go (using gt, ge, lt, le respectively). This is different to the reverse ops -- you don't need a new opcode and the handler doesn't need to know it's the "reverse" version. (Doing this in the minimal possible code size left as an exercise to the reader :p ).

Here's a rough sketch of how this should work, based on how the reverse ops have a second go at the "generic_binary_op" label.

Code: Select all

    // in mp_binary_op (runtime.c):
    
    // generic binary_op supplied by type
    mp_binary_op_t saved_op = 0;                    // Add this line.
    const mp_obj_type_t *type;
    generic_binary_op:
    
    ....
    
    // Add this just before the "unsupported_op" label.

    if (saved_op == 0) {
        saved_op = op;
        switch (op) {
            case MP_BINARY_OP_LESS:
                op = MP_BINARY_OP_MORE;
                break;
            case MP_BINARY_OP_MORE:
                op = MP_BINARY_OP_LESS;
                break;
            case MP_BINARY_OP_LESS_EQUAL:
                op = MP_BINARY_OP_MORE_EQUAL;
                break;
            case MP_BINARY_OP_MORE_EQUAL:
                op = MP_BINARY_OP_LESS_EQUAL;
                break;
            default:
                goto unsupported_op;
        }
        mp_obj_t t = rhs;
        rhs = lhs;
        lhs = t;
        // Have another go at binary_op, with updated op and arguments reversed.
        goto generic_binary_op;
    } else {
        // Second go at generic_binary_op failed, restore the original op so the error message is correct.
        op = saved_op;
    }
    
    unsupported_op:

I only very roughly tested it, you'd need to write some more detailed tests (ideally for user-defined and native types).

[v923z]

Jim,


Thanks for the comments! I wasn't aware of the difference between binary and comparison operators; at the moment, I handle everything in a single switch in binary_op, but obviously that is not the way to go. I will try to incorporate your suggestions.

Cheers,
Zoltán