From 69b89d21b2dcbe61c9abe1744cc0ec77d89422de Mon Sep 17 00:00:00 2001 From: Damien George Date: Fri, 11 Apr 2014 13:38:30 +0000 Subject: [PATCH] py: Change compile order for default positional and keyword args. This simplifies the compiler a little, since now it can do 1 pass over a function declaration, to determine default arguments. I would have done this originally, but CPython 3.3 somehow had the default keyword args compiled before the default position args (even though they appear in the other order in the text of the script), and I thought it was important to have the same order of execution when evaluating default arguments. CPython 3.4 has changed the order to the more obvious one, so we can also change. --- py/compile.c | 69 +++++++++++++++++++-------------------------------- py/emitbc.c | 6 ++--- py/emitglue.c | 3 +++ py/objfun.c | 1 + py/vm.c | 8 +++--- 5 files changed, 37 insertions(+), 50 deletions(-) diff --git a/py/compile.c b/py/compile.c index 9f081ebbcf..47cd0c4510 100644 --- a/py/compile.c +++ b/py/compile.c @@ -53,9 +53,8 @@ typedef struct _compiler_t { uint16_t n_arg_keyword; uint8_t star_flags; uint8_t have_bare_star; - uint8_t param_pass; - uint16_t param_pass_num_dict_params; - uint16_t param_pass_num_default_params; + uint16_t num_dict_params; + uint16_t num_default_params; scope_t *scope_head; scope_t *scope_cur; @@ -897,7 +896,7 @@ void compile_funcdef_param(compiler_t *comp, mp_parse_node_t pn) { // this parameter does not have a default value // check for non-default parameters given after default parameters (allowed by parser, but not syntactically valid) - if (!comp->have_bare_star && comp->param_pass_num_default_params != 0) { + if (!comp->have_bare_star && comp->num_default_params != 0) { compile_syntax_error(comp, pn, "non-default argument follows default argument"); return; } @@ -907,27 +906,23 @@ void compile_funcdef_param(compiler_t *comp, mp_parse_node_t pn) { // in CPython, None (and True, False?) as default parameters are loaded with LOAD_NAME; don't understandy why if (comp->have_bare_star) { - comp->param_pass_num_dict_params += 1; - if (comp->param_pass == 1) { + comp->num_dict_params += 1; #if !MICROPY_EMIT_CPYTHON - // in Micro Python we put the default dict parameters into a dictionary using the bytecode - if (comp->param_pass_num_dict_params == 1) { - // first default dict param, so make the map - EMIT_ARG(build_map, 0); - } -#endif - EMIT_ARG(load_const_id, MP_PARSE_NODE_LEAF_ARG(pn_id)); - compile_node(comp, pn_equal); -#if !MICROPY_EMIT_CPYTHON - // in Micro Python we put the default dict parameters into a dictionary using the bytecode - EMIT(store_map); -#endif + // in Micro Python we put the default dict parameters into a dictionary using the bytecode + if (comp->num_dict_params == 1) { + // first default dict param, so make the map + EMIT_ARG(build_map, 0); } +#endif + EMIT_ARG(load_const_id, MP_PARSE_NODE_LEAF_ARG(pn_id)); + compile_node(comp, pn_equal); +#if !MICROPY_EMIT_CPYTHON + // in Micro Python we put the default dict parameters into a dictionary using the bytecode + EMIT(store_map); +#endif } else { - comp->param_pass_num_default_params += 1; - if (comp->param_pass == 2) { - compile_node(comp, pn_equal); - } + comp->num_default_params += 1; + compile_node(comp, pn_equal); } } @@ -948,34 +943,23 @@ qstr compile_funcdef_helper(compiler_t *comp, mp_parse_node_struct_t *pns, uint // save variables (probably don't need to do this, since we can't have nested definitions..?) uint old_have_bare_star = comp->have_bare_star; - uint old_param_pass = comp->param_pass; - uint old_param_pass_num_dict_params = comp->param_pass_num_dict_params; - uint old_param_pass_num_default_params = comp->param_pass_num_default_params; + uint old_num_dict_params = comp->num_dict_params; + uint old_num_default_params = comp->num_default_params; // compile default parameters - - // pass 1 does any default parameters after bare star comp->have_bare_star = false; - comp->param_pass = 1; - comp->param_pass_num_dict_params = 0; - comp->param_pass_num_default_params = 0; + comp->num_dict_params = 0; + comp->num_default_params = 0; apply_to_single_or_list(comp, pns->nodes[1], PN_typedargslist, compile_funcdef_param); if (comp->had_error) { return MP_QSTR_NULL; } - // pass 2 does any default parameters before bare star - comp->have_bare_star = false; - comp->param_pass = 2; - comp->param_pass_num_dict_params = 0; - comp->param_pass_num_default_params = 0; - apply_to_single_or_list(comp, pns->nodes[1], PN_typedargslist, compile_funcdef_param); - #if !MICROPY_EMIT_CPYTHON // in Micro Python we put the default positional parameters into a tuple using the bytecode - if (comp->param_pass_num_default_params > 0) { - EMIT_ARG(build_tuple, comp->param_pass_num_default_params); + if (comp->num_default_params > 0) { + EMIT_ARG(build_tuple, comp->num_default_params); } #endif @@ -983,13 +967,12 @@ qstr compile_funcdef_helper(compiler_t *comp, mp_parse_node_struct_t *pns, uint scope_t *fscope = (scope_t*)pns->nodes[4]; // make the function - close_over_variables_etc(comp, fscope, comp->param_pass_num_default_params, comp->param_pass_num_dict_params); + close_over_variables_etc(comp, fscope, comp->num_default_params, comp->num_dict_params); // restore variables comp->have_bare_star = old_have_bare_star; - comp->param_pass = old_param_pass; - comp->param_pass_num_dict_params = old_param_pass_num_dict_params; - comp->param_pass_num_default_params = old_param_pass_num_default_params; + comp->num_dict_params = old_num_dict_params; + comp->num_default_params = old_num_default_params; // return its name (the 'f' in "def f(...):") return fscope->simple_name; diff --git a/py/emitbc.c b/py/emitbc.c index fc3e5ed622..a1179a6954 100644 --- a/py/emitbc.c +++ b/py/emitbc.c @@ -744,10 +744,10 @@ STATIC void emit_bc_make_function(emit_t *emit, scope_t *scope, uint n_pos_defau if (n_pos_defaults == 0) { // load dummy entry for non-existent positional default tuple emit_bc_load_null(emit); + emit_bc_rot_two(emit); } else if (n_kw_defaults == 0) { // load dummy entry for non-existent keyword default dict emit_bc_load_null(emit); - emit_bc_rot_two(emit); } emit_bc_pre(emit, -1); emit_write_byte_code_byte_uint(emit, MP_BC_MAKE_FUNCTION_DEFARGS, scope->unique_code_id); @@ -762,11 +762,11 @@ STATIC void emit_bc_make_closure(emit_t *emit, scope_t *scope, uint n_pos_defaul if (n_pos_defaults == 0) { // load dummy entry for non-existent positional default tuple emit_bc_load_null(emit); - emit_bc_rot_two(emit); + emit_bc_rot_three(emit); } else if (n_kw_defaults == 0) { // load dummy entry for non-existent keyword default dict emit_bc_load_null(emit); - emit_bc_rot_three(emit); + emit_bc_rot_two(emit); } emit_bc_pre(emit, -2); emit_write_byte_code_byte_uint(emit, MP_BC_MAKE_CLOSURE_DEFARGS, scope->unique_code_id); diff --git a/py/emitglue.c b/py/emitglue.c index 48800bbf00..90c9b77cf8 100644 --- a/py/emitglue.c +++ b/py/emitglue.c @@ -197,6 +197,9 @@ mp_obj_t mp_make_function_from_id(uint unique_code_id, mp_obj_t def_args, mp_obj return mp_const_none; } + // def_args must be MP_OBJ_NULL or a tuple + assert(def_args == MP_OBJ_NULL || MP_OBJ_IS_TYPE(def_args, &mp_type_tuple)); + // TODO implement default kw args assert(def_kw_args == MP_OBJ_NULL); diff --git a/py/objfun.c b/py/objfun.c index adf05ce404..dd4b7347ca 100644 --- a/py/objfun.c +++ b/py/objfun.c @@ -361,6 +361,7 @@ mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_args, mp_obj_t d uint n_extra_args = 0; mp_obj_tuple_t *def_args = def_args_in; if (def_args != MP_OBJ_NULL) { + assert(MP_OBJ_IS_TYPE(def_args, &mp_type_tuple)); n_def_args = def_args->len; n_extra_args = def_args->len; } diff --git a/py/vm.c b/py/vm.c index 47d6bf8f59..1ea0c5eaa1 100644 --- a/py/vm.c +++ b/py/vm.c @@ -718,9 +718,9 @@ unwind_jump: case MP_BC_MAKE_FUNCTION_DEFARGS: DECODE_UINT; - // Stack layout: def_dict def_tuple <- TOS + // Stack layout: def_tuple def_dict <- TOS obj1 = POP(); - SET_TOP(mp_make_function_from_id(unum, obj1, TOP())); + SET_TOP(mp_make_function_from_id(unum, TOP(), obj1)); break; case MP_BC_MAKE_CLOSURE: @@ -731,10 +731,10 @@ unwind_jump: case MP_BC_MAKE_CLOSURE_DEFARGS: DECODE_UINT; - // Stack layout: def_dict def_tuple closure_tuple <- TOS + // Stack layout: def_tuple def_dict closure_tuple <- TOS obj1 = POP(); obj2 = POP(); - SET_TOP(mp_make_closure_from_id(unum, obj1, obj2, TOP())); + SET_TOP(mp_make_closure_from_id(unum, obj1, TOP(), obj2)); break; case MP_BC_CALL_FUNCTION: