Skip to content

Commit 93b3726

Browse files
dpgeorgedpgeorge
committed
py/parse: Optimise away parse node that's just parenthesis around expr.
Before this patch, (x+y)*z would be parsed to a tree that contained a redundant identity parse node corresponding to the parenthesis. With this patch such nodes are optimised away, which reduces memory requirements for expressions with parenthesis, and simplifies the compiler because it doesn't need to handle this identity case. A parenthesis parse node is still needed for tuples.
1 parent 67f40fb commit 93b3726

2 files changed

Lines changed: 25 additions & 21 deletions

File tree

‎py/compile.c‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -299,14 +299,12 @@ STATIC void c_if_cond(compiler_t *comp, mp_parse_node_t pn, bool jump_if, int la
299299
if (jump_if == false) {
300300
EMIT_ARG(jump, label);
301301
}
302-
} else if (MP_PARSE_NODE_IS_STRUCT_KIND(pns->nodes[0], PN_testlist_comp)) {
302+
} else {
303+
assert(MP_PARSE_NODE_IS_STRUCT_KIND(pns->nodes[0], PN_testlist_comp));
303304
// non-empty tuple, acts as true for the condition
304305
if (jump_if == true) {
305306
EMIT_ARG(jump, label);
306307
}
307-
} else {
308-
// parenthesis around 1 item, is just that item
309-
c_if_cond(comp, pns->nodes[0], jump_if, label);
310308
}
311309
return;
312310
}
@@ -420,7 +418,6 @@ STATIC void c_assign_tuple(compiler_t *comp, mp_parse_node_t node_head, uint num
420418

421419
// assigns top of stack to pn
422420
STATIC void c_assign(compiler_t *comp, mp_parse_node_t pn, assign_kind_t assign_kind) {
423-
tail_recursion:
424421
assert(!MP_PARSE_NODE_IS_NULL(pn));
425422
if (MP_PARSE_NODE_IS_LEAF(pn)) {
426423
if (MP_PARSE_NODE_IS_ID(pn)) {
@@ -462,16 +459,13 @@ STATIC void c_assign(compiler_t *comp, mp_parse_node_t pn, assign_kind_t assign_
462459
if (MP_PARSE_NODE_IS_NULL(pns->nodes[0])) {
463460
// empty tuple
464461
goto cannot_assign;
465-
} else if (MP_PARSE_NODE_IS_STRUCT_KIND(pns->nodes[0], PN_testlist_comp)) {
462+
} else {
463+
assert(MP_PARSE_NODE_IS_STRUCT_KIND(pns->nodes[0], PN_testlist_comp));
466464
if (assign_kind != ASSIGN_STORE) {
467465
goto bad_aug;
468466
}
469467
pns = (mp_parse_node_struct_t*)pns->nodes[0];
470468
goto testlist_comp;
471-
} else {
472-
// parenthesis around 1 item, is just that item
473-
pn = pns->nodes[0];
474-
goto tail_recursion;
475469
}
476470
break;
477471

@@ -885,7 +879,10 @@ STATIC void c_del_stmt(compiler_t *comp, mp_parse_node_t pn) {
885879
}
886880
} else if (MP_PARSE_NODE_IS_STRUCT_KIND(pn, PN_atom_paren)) {
887881
pn = ((mp_parse_node_struct_t*)pn)->nodes[0];
888-
if (MP_PARSE_NODE_IS_STRUCT_KIND(pn, PN_testlist_comp)) {
882+
if (MP_PARSE_NODE_IS_NULL(pn)) {
883+
goto cannot_delete;
884+
} else {
885+
assert(MP_PARSE_NODE_IS_STRUCT_KIND(pn, PN_testlist_comp));
889886
mp_parse_node_struct_t *pns = (mp_parse_node_struct_t*)pn;
890887
// TODO perhaps factorise testlist_comp code with other uses of PN_testlist_comp
891888

@@ -915,12 +912,9 @@ STATIC void c_del_stmt(compiler_t *comp, mp_parse_node_t pn) {
915912
c_del_stmt(comp, pns->nodes[0]);
916913
c_del_stmt(comp, pns->nodes[1]);
917914
}
918-
} else {
919-
// tuple with 1 element
920-
c_del_stmt(comp, pn);
921915
}
922916
} else {
923-
// TODO is there anything else to implement?
917+
// some arbitrary statment that we can't delete (eg del 1)
924918
goto cannot_delete;
925919
}
926920

@@ -2185,7 +2179,8 @@ STATIC void compile_atom_paren(compiler_t *comp, mp_parse_node_struct_t *pns) {
21852179
if (MP_PARSE_NODE_IS_NULL(pns->nodes[0])) {
21862180
// an empty tuple
21872181
c_tuple(comp, MP_PARSE_NODE_NULL, NULL);
2188-
} else if (MP_PARSE_NODE_IS_STRUCT_KIND(pns->nodes[0], PN_testlist_comp)) {
2182+
} else {
2183+
assert(MP_PARSE_NODE_IS_STRUCT_KIND(pns->nodes[0], PN_testlist_comp));
21892184
pns = (mp_parse_node_struct_t*)pns->nodes[0];
21902185
assert(!MP_PARSE_NODE_IS_NULL(pns->nodes[1]));
21912186
if (MP_PARSE_NODE_IS_STRUCT(pns->nodes[1])) {
@@ -2209,9 +2204,6 @@ STATIC void compile_atom_paren(compiler_t *comp, mp_parse_node_struct_t *pns) {
22092204
tuple_with_2_items:
22102205
c_tuple(comp, MP_PARSE_NODE_NULL, pns);
22112206
}
2212-
} else {
2213-
// parenthesis around a single item, is just that item
2214-
compile_node(comp, pns->nodes[0]);
22152207
}
22162208
}
22172209

‎py/parse.c‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,20 @@ STATIC bool fold_constants(parser_t *parser, const rule_t *rule, size_t num_args
647647
#endif
648648

649649
STATIC void push_result_rule(parser_t *parser, size_t src_line, const rule_t *rule, size_t num_args) {
650+
// optimise away parenthesis around an expression if possible
651+
if (rule->rule_id == RULE_atom_paren) {
652+
// there should be just 1 arg for this rule
653+
mp_parse_node_t pn = peek_result(parser, 0);
654+
if (MP_PARSE_NODE_IS_NULL(pn)) {
655+
// need to keep parenthesis for ()
656+
} else if (MP_PARSE_NODE_IS_STRUCT_KIND(pn, RULE_testlist_comp)) {
657+
// need to keep parenthesis for (a, b, ...)
658+
} else {
659+
// parenthesis around a single expression, so it's just the expression
660+
return;
661+
}
662+
}
663+
650664
#if MICROPY_COMP_CONST_FOLDING
651665
if (fold_constants(parser, rule, num_args)) {
652666
// we folded this rule so return straight away
@@ -864,8 +878,6 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) {
864878

865879
// if a rule has the RULE_ACT_ALLOW_IDENT bit set then this
866880
// rule should not be emitted if it has only 1 argument
867-
// NOTE: can't set this flag for atom_paren because we need it
868-
// to distinguish, for example, [a,b] from [(a,b)]
869881
if (rule->act & RULE_ACT_ALLOW_IDENT) {
870882
emit_rule = false;
871883
}

0 commit comments

Comments
 (0)