Skip to content

Commit

Permalink
py/compile: Remove over-eager optimisation of tuples as if condition.
Browse files Browse the repository at this point in the history
When a tuple is the condition of an if statement, it's only possible to
optimise that tuple away when it is a constant tuple (ie all its elements
are constants), because if it's not constant then the elements must be
evaluated in case they have side effects (even though the resulting tuple
will always be "true").

The code before this change handled the empty tuple OK (because it doesn't
need to be evaluated), but it discarded non-empty tuples without evaluating
them, which is incorrect behaviour (as show by the updated test).

This optimisation is anyway rarely applied because it's not common Python
coding practice to write things like `if (): ...` and `if (1, 2): ...`, so
removing this optimisation completely won't affect much code, if any.

Furthermore, when MICROPY_COMP_CONST_TUPLE is enabled, constant tuples are
already optimised by the parser, so expression with constant tuples like
`if (): ...` and `if (1, 2): ...` will continue to be optimised properly
(and so when this option is enabled the code that's deleted in this commit
is actually unreachable when the if condition is a constant tuple).

Signed-off-by: Damien George <damien@micropython.org>
  • Loading branch information
dpgeorge committed May 3, 2023
1 parent 957bd51 commit 1b980c9
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 15 deletions.
15 changes: 0 additions & 15 deletions py/compile.c
Expand Up @@ -423,21 +423,6 @@ STATIC void c_if_cond(compiler_t *comp, mp_parse_node_t pn, bool jump_if, int la
} else if (MP_PARSE_NODE_STRUCT_KIND(pns) == PN_not_test_2) {
c_if_cond(comp, pns->nodes[0], !jump_if, label);
return;
} else if (MP_PARSE_NODE_STRUCT_KIND(pns) == PN_atom_paren) {
// cond is something in parenthesis
if (MP_PARSE_NODE_IS_NULL(pns->nodes[0])) {
// empty tuple, acts as false for the condition
if (jump_if == false) {
EMIT_ARG(jump, label);
}
} else {
assert(MP_PARSE_NODE_IS_STRUCT_KIND(pns->nodes[0], PN_testlist_comp));
// non-empty tuple, acts as true for the condition
if (jump_if == true) {
EMIT_ARG(jump, label);
}
}
return;
}
}

Expand Down
14 changes: 14 additions & 0 deletions tests/basics/ifcond.py
Expand Up @@ -57,6 +57,20 @@
else:
print('b')

# test evaluation of the if-condition with tuples as arguments
# non-constant tuples should be evaluated even though they will evaluate to true

def f(x):
print("f", x)

if (f(1),):
print(18)

if (f(2), f(3)):
print(19)

# test if-conditions within a function

f2 = 0

def f(t1, t2, f1):
Expand Down

0 comments on commit 1b980c9

Please sign in to comment.