Skip to content

Commit 5e34a11

Browse files
dpgeorgedpgeorge
committed
py/runtime: Add MP_BINARY_OP_CONTAINS as reverse of MP_BINARY_OP_IN.
Before this patch MP_BINARY_OP_IN had two meanings: coming from bytecode it meant that the args needed to be swapped, but coming from within the runtime meant that the args were already in the correct order. This lead to some confusion in the code and comments stating how args were reversed. It also lead to 2 bugs: 1) containment for a subclass of a native type didn't work; 2) the expression "{True} in True" would illegally succeed and return True. In both of these cases it was because the args to MP_BINARY_OP_IN ended up being reversed twice. To fix these things this patch introduces MP_BINARY_OP_CONTAINS which corresponds exactly to the __contains__ special method, and this is the operator that built-in types should implement. MP_BINARY_OP_IN is now only emitted by the compiler and is converted to MP_BINARY_OP_CONTAINS by swapping the arguments.
1 parent 5b2f62a commit 5e34a11

File tree

11 files changed

+34
-36
lines changed

11 files changed

+34
-36
lines changed

‎extmod/modbtree.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,7 @@ STATIC mp_obj_t btree_subscr(mp_obj_t self_in, mp_obj_t index, mp_obj_t value) {
282282
STATIC mp_obj_t btree_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs_in) {
283283
mp_obj_btree_t *self = MP_OBJ_TO_PTR(lhs_in);
284284
switch (op) {
285-
case MP_BINARY_OP_IN: {
285+
case MP_BINARY_OP_CONTAINS: {
286286
DBT key, val;
287287
key.data = (void*)mp_obj_str_get_data(rhs_in, &key.size);
288288
int res = __bt_get(self->db, &key, &val, 0);

‎py/objarray.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,7 @@ STATIC mp_obj_t array_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs
269269
return lhs_in;
270270
}
271271

272-
case MP_BINARY_OP_IN: {
273-
/* NOTE `a in b` is `b.__contains__(a)` */
272+
case MP_BINARY_OP_CONTAINS: {
274273
mp_buffer_info_t lhs_bufinfo;
275274
mp_buffer_info_t rhs_bufinfo;
276275

‎py/objdict.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ STATIC mp_obj_t dict_unary_op(mp_unary_op_t op, mp_obj_t self_in) {
115115
STATIC mp_obj_t dict_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs_in) {
116116
mp_obj_dict_t *o = MP_OBJ_TO_PTR(lhs_in);
117117
switch (op) {
118-
case MP_BINARY_OP_IN: {
118+
case MP_BINARY_OP_CONTAINS: {
119119
mp_map_elem_t *elem = mp_map_lookup(&o->map, rhs_in, MP_MAP_LOOKUP);
120120
return mp_obj_new_bool(elem != NULL);
121121
}
@@ -485,7 +485,7 @@ STATIC mp_obj_t dict_view_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t
485485
if (o->kind != MP_DICT_VIEW_KEYS) {
486486
return MP_OBJ_NULL; // op not supported
487487
}
488-
if (op != MP_BINARY_OP_IN) {
488+
if (op != MP_BINARY_OP_CONTAINS) {
489489
return MP_OBJ_NULL; // op not supported
490490
}
491491
return dict_binary_op(op, o->dict, rhs_in);

‎py/objint_mpz.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ mp_obj_t mp_obj_int_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs_i
207207
return mp_obj_new_float(flhs / frhs);
208208
#endif
209209

210-
} else if (op >= MP_BINARY_OP_INPLACE_OR) {
210+
} else if (op >= MP_BINARY_OP_INPLACE_OR && op < MP_BINARY_OP_CONTAINS) {
211211
mp_obj_int_t *res = mp_obj_int_new_mpz();
212212

213213
switch (op) {

‎py/objset.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ STATIC mp_obj_t set_binary_op(mp_binary_op_t op, mp_obj_t lhs, mp_obj_t rhs) {
461461
#else
462462
bool update = true;
463463
#endif
464-
if (op != MP_BINARY_OP_IN && !is_set_or_frozenset(rhs)) {
464+
if (op != MP_BINARY_OP_CONTAINS && !is_set_or_frozenset(rhs)) {
465465
// For all ops except containment the RHS must be a set/frozenset
466466
return MP_OBJ_NULL;
467467
}
@@ -507,7 +507,7 @@ STATIC mp_obj_t set_binary_op(mp_binary_op_t op, mp_obj_t lhs, mp_obj_t rhs) {
507507
return set_issubset(lhs, rhs);
508508
case MP_BINARY_OP_MORE_EQUAL:
509509
return set_issuperset(lhs, rhs);
510-
case MP_BINARY_OP_IN: {
510+
case MP_BINARY_OP_CONTAINS: {
511511
mp_obj_set_t *o = MP_OBJ_TO_PTR(lhs);
512512
mp_obj_t elem = mp_set_lookup(&o->set, rhs, MP_MAP_LOOKUP);
513513
return mp_obj_new_bool(elem != MP_OBJ_NULL);

‎py/objstr.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -384,8 +384,7 @@ mp_obj_t mp_obj_str_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_obj_t rhs_i
384384
return mp_obj_new_str_from_vstr(lhs_type, &vstr);
385385
}
386386

387-
case MP_BINARY_OP_IN:
388-
/* NOTE `a in b` is `b.__contains__(a)` */
387+
case MP_BINARY_OP_CONTAINS:
389388
return mp_obj_new_bool(find_subbytes(lhs_data, lhs_len, rhs_data, rhs_len, 1) != NULL);
390389

391390
//case MP_BINARY_OP_NOT_EQUAL: // This is never passed here

‎py/objtype.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -424,7 +424,7 @@ const byte mp_binary_op_method_name[MP_BINARY_OP_NUM_RUNTIME] = {
424424
[MP_BINARY_OP_LESS_EQUAL] = MP_QSTR___le__,
425425
[MP_BINARY_OP_MORE_EQUAL] = MP_QSTR___ge__,
426426
// MP_BINARY_OP_NOT_EQUAL, // a != b calls a == b and inverts result
427-
[MP_BINARY_OP_IN] = MP_QSTR___contains__,
427+
[MP_BINARY_OP_CONTAINS] = MP_QSTR___contains__,
428428

429429
// All inplace methods are optional, and normal methods will be used
430430
// as a fallback.

‎py/opmethods.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,6 @@ MP_DEFINE_CONST_FUN_OBJ_2(mp_op_delitem_obj, op_delitem);
4747

4848
STATIC mp_obj_t op_contains(mp_obj_t lhs_in, mp_obj_t rhs_in) {
4949
mp_obj_type_t *type = mp_obj_get_type(lhs_in);
50-
return type->binary_op(MP_BINARY_OP_IN, lhs_in, rhs_in);
50+
return type->binary_op(MP_BINARY_OP_CONTAINS, lhs_in, rhs_in);
5151
}
5252
MP_DEFINE_CONST_FUN_OBJ_2(mp_op_contains_obj, op_contains);

‎py/runtime.c‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -523,30 +523,12 @@ mp_obj_t mp_binary_op(mp_binary_op_t op, mp_obj_t lhs, mp_obj_t rhs) {
523523
}
524524
}
525525

526-
/* deal with `in`
527-
*
528-
* NOTE `a in b` is `b.__contains__(a)`, hence why the generic dispatch
529-
* needs to go below with swapped arguments
530-
*/
526+
// Convert MP_BINARY_OP_IN to MP_BINARY_OP_CONTAINS with swapped args.
531527
if (op == MP_BINARY_OP_IN) {
532-
mp_obj_type_t *type = mp_obj_get_type(rhs);
533-
if (type->binary_op != NULL) {
534-
mp_obj_t res = type->binary_op(op, rhs, lhs);
535-
if (res != MP_OBJ_NULL) {
536-
return res;
537-
}
538-
}
539-
540-
// final attempt, walk the iterator (will raise if rhs is not iterable)
541-
mp_obj_iter_buf_t iter_buf;
542-
mp_obj_t iter = mp_getiter(rhs, &iter_buf);
543-
mp_obj_t next;
544-
while ((next = mp_iternext(iter)) != MP_OBJ_STOP_ITERATION) {
545-
if (mp_obj_equal(next, lhs)) {
546-
return mp_const_true;
547-
}
548-
}
549-
return mp_const_false;
528+
op = MP_BINARY_OP_CONTAINS;
529+
mp_obj_t temp = lhs;
530+
lhs = rhs;
531+
rhs = temp;
550532
}
551533

552534
// generic binary_op supplied by type
@@ -575,6 +557,20 @@ mp_obj_t mp_binary_op(mp_binary_op_t op, mp_obj_t lhs, mp_obj_t rhs) {
575557
}
576558
#endif
577559

560+
if (op == MP_BINARY_OP_CONTAINS) {
561+
// If type didn't support containment then explicitly walk the iterator.
562+
// mp_getiter will raise the appropriate exception if lhs is not iterable.
563+
mp_obj_iter_buf_t iter_buf;
564+
mp_obj_t iter = mp_getiter(lhs, &iter_buf);
565+
mp_obj_t next;
566+
while ((next = mp_iternext(iter)) != MP_OBJ_STOP_ITERATION) {
567+
if (mp_obj_equal(next, rhs)) {
568+
return mp_const_true;
569+
}
570+
}
571+
return mp_const_false;
572+
}
573+
578574
unsupported_op:
579575
if (MICROPY_ERROR_REPORTING == MICROPY_ERROR_REPORTING_TERSE) {
580576
mp_raise_TypeError("unsupported type for operator");

‎py/runtime0.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,10 @@ typedef enum {
131131
#endif
132132
,
133133

134+
// The runtime will convert MP_BINARY_OP_IN to this operator with swapped args.
135+
// A type should implement this containment operator instead of MP_BINARY_OP_IN.
136+
MP_BINARY_OP_CONTAINS,
137+
134138
MP_BINARY_OP_NUM_RUNTIME,
135139

136140
// These 2 are not supported by the runtime and must be synthesised by the emitter

0 commit comments

Comments
 (0)