Skip to content

Commit 2c0842b

Browse files
dpgeorgedpgeorge
committed
py: Change the way function arguments are compiled.
New way uses slightly less ROM and RAM, should be slightly faster, and, most importantly, allows to catch the error "non-keyword arg following keyword arg". Addresses issue adafruit#466.
1 parent 2827d62 commit 2c0842b

2 files changed

Lines changed: 65 additions & 78 deletions

File tree

‎py/compile.c‎

Lines changed: 61 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,6 @@ typedef struct _compiler_t {
5151
int break_continue_except_level;
5252
uint16_t cur_except_level; // increased for SETUP_EXCEPT, SETUP_FINALLY; decreased for POP_BLOCK, POP_EXCEPT
5353

54-
uint16_t n_arg_keyword;
55-
uint8_t star_flags;
5654
uint8_t have_star;
5755
uint16_t num_dict_params;
5856
uint16_t num_default_params;
@@ -264,6 +262,7 @@ STATIC mp_parse_node_t fold_constants(mp_parse_node_t pn) {
264262
}
265263

266264
STATIC void compile_trailer_paren_helper(compiler_t *comp, mp_parse_node_t pn_arglist, bool is_method_call, int n_positional_extra);
265+
void compile_comprehension(compiler_t *comp, mp_parse_node_struct_t *pns, scope_kind_t kind);
267266
STATIC void compile_node(compiler_t *comp, mp_parse_node_t pn);
268267

269268
STATIC uint comp_next_label(compiler_t *comp) {
@@ -298,21 +297,6 @@ STATIC scope_t *scope_new_and_link(compiler_t *comp, scope_kind_t kind, mp_parse
298297
return scope;
299298
}
300299

301-
STATIC int list_len(mp_parse_node_t pn, int pn_kind) {
302-
if (MP_PARSE_NODE_IS_NULL(pn)) {
303-
return 0;
304-
} else if (MP_PARSE_NODE_IS_LEAF(pn)) {
305-
return 1;
306-
} else {
307-
mp_parse_node_struct_t *pns = (mp_parse_node_struct_t*)pn;
308-
if (MP_PARSE_NODE_STRUCT_KIND(pns) != pn_kind) {
309-
return 1;
310-
} else {
311-
return MP_PARSE_NODE_STRUCT_NUM_NODES(pns);
312-
}
313-
}
314-
}
315-
316300
STATIC void apply_to_single_or_list(compiler_t *comp, mp_parse_node_t pn, int pn_list_kind, void (*f)(compiler_t*, mp_parse_node_t)) {
317301
if (MP_PARSE_NODE_IS_STRUCT(pn) && MP_PARSE_NODE_STRUCT_KIND((mp_parse_node_struct_t*)pn) == pn_list_kind) {
318302
mp_parse_node_struct_t *pns = (mp_parse_node_struct_t*)pn;
@@ -485,7 +469,7 @@ STATIC void cpython_c_tuple(compiler_t *comp, mp_parse_node_t pn, mp_parse_node_
485469
#endif
486470

487471
// funnelling all tuple creations through this function is purely so we can optionally agree with CPython
488-
void c_tuple(compiler_t *comp, mp_parse_node_t pn, mp_parse_node_struct_t *pns_list) {
472+
STATIC void c_tuple(compiler_t *comp, mp_parse_node_t pn, mp_parse_node_struct_t *pns_list) {
489473
#if MICROPY_EMIT_CPYTHON
490474
cpython_c_tuple(comp, pn, pns_list);
491475
#else
@@ -2331,30 +2315,70 @@ STATIC void compile_trailer_paren_helper(compiler_t *comp, mp_parse_node_t pn_ar
23312315
}
23322316
#endif
23332317

2334-
uint old_n_arg_keyword = comp->n_arg_keyword;
2335-
uint old_star_flags = comp->star_flags;
2336-
comp->n_arg_keyword = 0;
2337-
comp->star_flags = 0;
2338-
2339-
compile_node(comp, pn_arglist); // arguments to function call; can be null
2340-
2341-
// compute number of positional arguments
2342-
int n_positional = n_positional_extra + list_len(pn_arglist, PN_arglist) - comp->n_arg_keyword;
2343-
if (comp->star_flags & MP_EMIT_STAR_FLAG_SINGLE) {
2344-
n_positional -= 1;
2345-
}
2346-
if (comp->star_flags & MP_EMIT_STAR_FLAG_DOUBLE) {
2347-
n_positional -= 1;
2318+
// get the list of arguments
2319+
mp_parse_node_t *args;
2320+
int n_args = list_get(&pn_arglist, PN_arglist, &args);
2321+
2322+
// compile the arguments
2323+
// Rather than calling compile_node on the list, we go through the list of args
2324+
// explicitly here so that we can count the number of arguments and give sensible
2325+
// error messages.
2326+
int n_positional = n_positional_extra;
2327+
uint n_keyword = 0;
2328+
uint star_flags = 0;
2329+
for (int i = 0; i < n_args; i++) {
2330+
if (MP_PARSE_NODE_IS_STRUCT(args[i])) {
2331+
mp_parse_node_struct_t *pns_arg = (mp_parse_node_struct_t*)args[i];
2332+
if (MP_PARSE_NODE_STRUCT_KIND(pns_arg) == PN_arglist_star) {
2333+
if (star_flags & MP_EMIT_STAR_FLAG_SINGLE) {
2334+
compile_syntax_error(comp, (mp_parse_node_t)pns_arg, "can't have multiple *x");
2335+
return;
2336+
}
2337+
star_flags |= MP_EMIT_STAR_FLAG_SINGLE;
2338+
compile_node(comp, pns_arg->nodes[0]);
2339+
} else if (MP_PARSE_NODE_STRUCT_KIND(pns_arg) == PN_arglist_dbl_star) {
2340+
if (star_flags & MP_EMIT_STAR_FLAG_DOUBLE) {
2341+
compile_syntax_error(comp, (mp_parse_node_t)pns_arg, "can't have multiple **x");
2342+
return;
2343+
}
2344+
star_flags |= MP_EMIT_STAR_FLAG_DOUBLE;
2345+
compile_node(comp, pns_arg->nodes[0]);
2346+
} else if (MP_PARSE_NODE_STRUCT_KIND(pns_arg) == PN_argument) {
2347+
assert(MP_PARSE_NODE_IS_STRUCT(pns_arg->nodes[1])); // should always be
2348+
mp_parse_node_struct_t *pns2 = (mp_parse_node_struct_t*)pns_arg->nodes[1];
2349+
if (MP_PARSE_NODE_STRUCT_KIND(pns2) == PN_argument_3) {
2350+
if (!MP_PARSE_NODE_IS_ID(pns_arg->nodes[0])) {
2351+
compile_syntax_error(comp, (mp_parse_node_t)pns_arg, "LHS of keyword arg must be an id");
2352+
return;
2353+
}
2354+
EMIT_ARG(load_const_id, MP_PARSE_NODE_LEAF_ARG(pns_arg->nodes[0]));
2355+
compile_node(comp, pns2->nodes[0]);
2356+
n_keyword += 1;
2357+
} else {
2358+
assert(MP_PARSE_NODE_STRUCT_KIND(pns2) == PN_comp_for); // should always be
2359+
compile_comprehension(comp, pns_arg, SCOPE_GEN_EXPR);
2360+
n_positional++;
2361+
}
2362+
} else {
2363+
goto normal_argument;
2364+
}
2365+
} else {
2366+
normal_argument:
2367+
if (n_keyword > 0) {
2368+
compile_syntax_error(comp, args[i], "non-keyword arg after keyword arg");
2369+
return;
2370+
}
2371+
compile_node(comp, args[i]);
2372+
n_positional++;
2373+
}
23482374
}
23492375

2376+
// emit the function/method call
23502377
if (is_method_call) {
2351-
EMIT_ARG(call_method, n_positional, comp->n_arg_keyword, comp->star_flags);
2378+
EMIT_ARG(call_method, n_positional, n_keyword, star_flags);
23522379
} else {
2353-
EMIT_ARG(call_function, n_positional, comp->n_arg_keyword, comp->star_flags);
2380+
EMIT_ARG(call_function, n_positional, n_keyword, star_flags);
23542381
}
2355-
2356-
comp->n_arg_keyword = old_n_arg_keyword;
2357-
comp->star_flags = old_star_flags;
23582382
}
23592383

23602384
void compile_power_trailers(compiler_t *comp, mp_parse_node_struct_t *pns) {
@@ -2677,43 +2701,6 @@ void compile_classdef(compiler_t *comp, mp_parse_node_struct_t *pns) {
26772701
EMIT_ARG(store_id, cname);
26782702
}
26792703

2680-
void compile_arglist_star(compiler_t *comp, mp_parse_node_struct_t *pns) {
2681-
if (comp->star_flags & MP_EMIT_STAR_FLAG_SINGLE) {
2682-
compile_syntax_error(comp, (mp_parse_node_t)pns, "can't have multiple *x");
2683-
return;
2684-
}
2685-
comp->star_flags |= MP_EMIT_STAR_FLAG_SINGLE;
2686-
compile_node(comp, pns->nodes[0]);
2687-
}
2688-
2689-
void compile_arglist_dbl_star(compiler_t *comp, mp_parse_node_struct_t *pns) {
2690-
if (comp->star_flags & MP_EMIT_STAR_FLAG_DOUBLE) {
2691-
compile_syntax_error(comp, (mp_parse_node_t)pns, "can't have multiple **x");
2692-
return;
2693-
}
2694-
comp->star_flags |= MP_EMIT_STAR_FLAG_DOUBLE;
2695-
compile_node(comp, pns->nodes[0]);
2696-
}
2697-
2698-
void compile_argument(compiler_t *comp, mp_parse_node_struct_t *pns) {
2699-
assert(MP_PARSE_NODE_IS_STRUCT(pns->nodes[1])); // should always be
2700-
mp_parse_node_struct_t *pns2 = (mp_parse_node_struct_t*)pns->nodes[1];
2701-
if (MP_PARSE_NODE_STRUCT_KIND(pns2) == PN_argument_3) {
2702-
if (!MP_PARSE_NODE_IS_ID(pns->nodes[0])) {
2703-
compile_syntax_error(comp, (mp_parse_node_t)pns, "left-hand-side of keyword argument must be an id");
2704-
return;
2705-
}
2706-
EMIT_ARG(load_const_id, MP_PARSE_NODE_LEAF_ARG(pns->nodes[0]));
2707-
compile_node(comp, pns2->nodes[0]);
2708-
comp->n_arg_keyword += 1;
2709-
} else if (MP_PARSE_NODE_STRUCT_KIND(pns2) == PN_comp_for) {
2710-
compile_comprehension(comp, pns, SCOPE_GEN_EXPR);
2711-
} else {
2712-
// shouldn't happen
2713-
assert(0);
2714-
}
2715-
}
2716-
27172704
void compile_yield_expr(compiler_t *comp, mp_parse_node_struct_t *pns) {
27182705
if (comp->scope_cur->kind != SCOPE_FUNCTION && comp->scope_cur->kind != SCOPE_LAMBDA) {
27192706
compile_syntax_error(comp, (mp_parse_node_t)pns, "'yield' outside function");

‎py/grammar.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,10 +275,10 @@ DEF_RULE(classdef_2, nc, and(3), tok(DEL_PAREN_OPEN), opt_rule(arglist), tok(DEL
275275
// arglist: (argument ',')* (argument [','] | '*' test (',' argument)* [',' '**' test] | '**' test)
276276

277277
// TODO arglist lets through more than is allowed, compiler needs to do further verification
278-
DEF_RULE(arglist, c(generic_all_nodes), list_with_end, rule(arglist_2), tok(DEL_COMMA))
278+
DEF_RULE(arglist, nc, list_with_end, rule(arglist_2), tok(DEL_COMMA))
279279
DEF_RULE(arglist_2, nc, or(3), rule(arglist_star), rule(arglist_dbl_star), rule(argument))
280-
DEF_RULE(arglist_star, c(arglist_star), and(2), tok(OP_STAR), rule(test))
281-
DEF_RULE(arglist_dbl_star, c(arglist_dbl_star), and(2), tok(OP_DBL_STAR), rule(test))
280+
DEF_RULE(arglist_star, nc, and(2), tok(OP_STAR), rule(test))
281+
DEF_RULE(arglist_dbl_star, nc, and(2), tok(OP_DBL_STAR), rule(test))
282282

283283
// # The reason that keywords are test nodes instead of NAME is that using NAME
284284
// # results in an ambiguity. ast.c makes sure it's a NAME.
@@ -287,7 +287,7 @@ DEF_RULE(arglist_dbl_star, c(arglist_dbl_star), and(2), tok(OP_DBL_STAR), rule(t
287287
// comp_for: 'for' exprlist 'in' or_test [comp_iter]
288288
// comp_if: 'if' test_nocond [comp_iter]
289289

290-
DEF_RULE(argument, c(argument), and(2), rule(test), opt_rule(argument_2))
290+
DEF_RULE(argument, nc, and(2), rule(test), opt_rule(argument_2))
291291
DEF_RULE(argument_2, nc, or(2), rule(comp_for), rule(argument_3))
292292
DEF_RULE(argument_3, nc, and(2), tok(DEL_EQUAL), rule(test))
293293
DEF_RULE(comp_iter, nc, or(2), rule(comp_for), rule(comp_if))

0 commit comments

Comments
 (0)