From f0778a7ccbbdd1d7bf116a6939c7eb05173e1987 Mon Sep 17 00:00:00 2001 From: Damien George Date: Sat, 7 Jun 2014 22:01:00 +0100 Subject: [PATCH] py: Implement default keyword only args. Should finish addressing issue #524. --- py/compile.c | 12 +++++++----- py/emitglue.c | 6 +++--- py/obj.h | 2 +- py/objfun.c | 40 +++++++++++++++++++++++++++++--------- py/objfun.h | 7 ++++++- tests/basics/fun-kwonly.py | 16 +++++++-------- 6 files changed, 56 insertions(+), 27 deletions(-) diff --git a/py/compile.c b/py/compile.c index 1f0d90570e..946c8924b2 100644 --- a/py/compile.c +++ b/py/compile.c @@ -1035,7 +1035,10 @@ void compile_funcdef_param(compiler_t *comp, mp_parse_node_t pn) { if (comp->have_star) { comp->num_dict_params += 1; -#if !MICROPY_EMIT_CPYTHON +#if MICROPY_EMIT_CPYTHON + EMIT_ARG(load_const_str, MP_PARSE_NODE_LEAF_ARG(pn_id), false); + compile_node(comp, pn_equal); +#else // in Micro Python we put the default dict parameters into a dictionary using the bytecode if (comp->num_dict_params == 1) { // in Micro Python we put the default positional parameters into a tuple using the bytecode @@ -1048,11 +1051,10 @@ void compile_funcdef_param(compiler_t *comp, mp_parse_node_t pn) { // first default dict param, so make the map EMIT_ARG(build_map, 0); } -#endif - EMIT_ARG(load_const_str, MP_PARSE_NODE_LEAF_ARG(pn_id), false); + + // compile value then key, then store it to the dict 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_ARG(load_const_str, MP_PARSE_NODE_LEAF_ARG(pn_id), false); EMIT(store_map); #endif } else { diff --git a/py/emitglue.c b/py/emitglue.c index a89ef6766a..f9b9460837 100644 --- a/py/emitglue.c +++ b/py/emitglue.c @@ -119,14 +119,14 @@ mp_obj_t mp_make_function_from_raw_code(mp_raw_code_t *rc, mp_obj_t def_args, mp // 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); + // def_kw_args must be MP_OBJ_NULL or a dict + assert(def_kw_args == MP_OBJ_NULL || MP_OBJ_IS_TYPE(def_kw_args, &mp_type_dict)); // make the function, depending on the raw code kind mp_obj_t fun; switch (rc->kind) { case MP_CODE_BYTECODE: - fun = mp_obj_new_fun_bc(rc->scope_flags, rc->arg_names, rc->n_pos_args, rc->n_kwonly_args, def_args, rc->u_byte.code); + fun = mp_obj_new_fun_bc(rc->scope_flags, rc->arg_names, rc->n_pos_args, rc->n_kwonly_args, def_args, def_kw_args, rc->u_byte.code); break; case MP_CODE_NATIVE_PY: fun = mp_make_function_n(rc->n_pos_args, rc->u_native.fun); diff --git a/py/obj.h b/py/obj.h index aa78b2a22d..c7f0a9879e 100644 --- a/py/obj.h +++ b/py/obj.h @@ -380,7 +380,7 @@ mp_obj_t mp_obj_new_exception_arg1(const mp_obj_type_t *exc_type, mp_obj_t arg); mp_obj_t mp_obj_new_exception_args(const mp_obj_type_t *exc_type, uint n_args, const mp_obj_t *args); mp_obj_t mp_obj_new_exception_msg(const mp_obj_type_t *exc_type, const char *msg); mp_obj_t mp_obj_new_exception_msg_varg(const mp_obj_type_t *exc_type, const char *fmt, ...); // counts args by number of % symbols in fmt, excluding %%; can only handle void* sizes (ie no float/double!) -mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_pos_args, uint n_kwonly_args, mp_obj_t def_args, const byte *code); +mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_pos_args, uint n_kwonly_args, mp_obj_t def_args, mp_obj_t def_kw_args, const byte *code); mp_obj_t mp_obj_new_fun_asm(uint n_args, void *fun); mp_obj_t mp_obj_new_gen_wrap(mp_obj_t fun); mp_obj_t mp_obj_new_closure(mp_obj_t fun, uint n_closed, const mp_obj_t *closed); diff --git a/py/objfun.c b/py/objfun.c index 57398e6d61..de770c0040 100644 --- a/py/objfun.c +++ b/py/objfun.c @@ -249,6 +249,10 @@ STATIC mp_obj_t fun_bc_call(mp_obj_t self_in, uint n_args, uint n_kw, const mp_o // This function is pretty complicated. It's main aim is to be efficient in speed and RAM // usage for the common case of positional only args. // + // TODO Now that we allocate the state for the bytecode execution, we probably don't + // need extra_args anymore, and definitely don't need flat_args. + // + // TODO The following comment is obsolete and probably wrong: // extra_args layout: def_args, var_arg tuple, kwonly args, var_kw dict DEBUG_printf("Input n_args: %d, n_kw: %d\n", n_args, n_kw); @@ -260,7 +264,7 @@ STATIC mp_obj_t fun_bc_call(mp_obj_t self_in, uint n_args, uint n_kw, const mp_o DEBUG_printf("Func n_def_args: %d\n", self->n_def_args); const mp_obj_t *kwargs = args + n_args; - mp_obj_t *extra_args = self->extra_args + self->n_def_args; + mp_obj_t *extra_args = self->extra_args + self->n_def_args + self->has_def_kw_args; uint n_extra_args = 0; // check positional arguments @@ -295,7 +299,7 @@ STATIC mp_obj_t fun_bc_call(mp_obj_t self_in, uint n_args, uint n_kw, const mp_o // check keyword arguments - if (n_kw != 0) { + if (n_kw != 0 || self->has_def_kw_args) { // We cannot use dynamically-sized array here, because GCC indeed // deallocates it on leaving defining scope (unlike most static stack allocs). // So, we have 2 choices: allocate it unconditionally at the top of function @@ -355,10 +359,19 @@ continue2:; } // Check that all mandatory keyword args are specified + // Fill in default kw args if we have them for (int i = 0; i < self->n_kwonly_args; i++) { if (flat_args[self->n_pos_args + i] == MP_OBJ_NULL) { - nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_TypeError, - "function missing required keyword argument '%s'", qstr_str(self->args[self->n_pos_args + i]))); + mp_map_elem_t *elem = NULL; + if (self->has_def_kw_args) { + elem = mp_map_lookup(&((mp_obj_dict_t*)self->extra_args[self->n_def_args])->map, MP_OBJ_NEW_QSTR(self->args[self->n_pos_args + i]), MP_MAP_LOOKUP); + } + if (elem != NULL) { + flat_args[self->n_pos_args + i] = elem->value; + } else { + nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_TypeError, + "function missing required keyword argument '%s'", qstr_str(self->args[self->n_pos_args + i]))); + } } } @@ -513,7 +526,7 @@ const mp_obj_type_t mp_type_fun_bc = { .binary_op = fun_binary_op, }; -mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_pos_args, uint n_kwonly_args, mp_obj_t def_args_in, const byte *code) { +mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_pos_args, uint n_kwonly_args, mp_obj_t def_args_in, mp_obj_t def_kw_args, const byte *code) { uint n_def_args = 0; uint n_extra_args = 0; mp_obj_tuple_t *def_args = def_args_in; @@ -522,6 +535,9 @@ mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_pos_args, uint n n_def_args = def_args->len; n_extra_args = def_args->len; } + if (def_kw_args != MP_OBJ_NULL) { + n_extra_args += 1; + } if ((scope_flags & MP_SCOPE_FLAG_VARARGS) != 0) { n_extra_args += 1; } @@ -535,18 +551,24 @@ mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_pos_args, uint n o->n_pos_args = n_pos_args; o->n_kwonly_args = n_kwonly_args; o->n_def_args = n_def_args; + o->has_def_kw_args = def_kw_args != MP_OBJ_NULL; o->takes_var_args = (scope_flags & MP_SCOPE_FLAG_VARARGS) != 0; o->takes_kw_args = (scope_flags & MP_SCOPE_FLAG_VARKEYWORDS) != 0; o->bytecode = code; - memset(o->extra_args, 0, n_extra_args * sizeof(mp_obj_t)); + mp_obj_t *extra_args = o->extra_args; + memset(extra_args, 0, n_extra_args * sizeof(mp_obj_t)); if (def_args != MP_OBJ_NULL) { - memcpy(o->extra_args, def_args->items, n_def_args * sizeof(mp_obj_t)); + memcpy(extra_args, def_args->items, n_def_args * sizeof(mp_obj_t)); + extra_args += n_def_args; + } + if (def_kw_args != MP_OBJ_NULL) { + *extra_args++ = def_kw_args; } if ((scope_flags & MP_SCOPE_FLAG_VARARGS) != 0) { - o->extra_args[n_def_args] = MP_OBJ_NULL; + *extra_args++ = MP_OBJ_NULL; } if ((scope_flags & MP_SCOPE_FLAG_VARARGS) != 0) { - o->extra_args[n_extra_args - 1] = MP_OBJ_NULL; + *extra_args = MP_OBJ_NULL; } return o; } diff --git a/py/objfun.h b/py/objfun.h index b049e5c7e8..f607a5eb6f 100644 --- a/py/objfun.h +++ b/py/objfun.h @@ -30,10 +30,15 @@ typedef struct _mp_obj_fun_bc_t { machine_uint_t n_pos_args : 16; // number of arguments this function takes machine_uint_t n_kwonly_args : 16; // number of arguments this function takes machine_uint_t n_def_args : 16; // number of default arguments + machine_uint_t has_def_kw_args : 1; // set if this function has default keyword args machine_uint_t takes_var_args : 1; // set if this function takes variable args machine_uint_t takes_kw_args : 1; // set if this function takes keyword args const byte *bytecode; // bytecode for the function qstr *args; // argument names (needed to resolve positional args passed as keywords) - // values of default args (if any), plus a slot at the end for var args and/or kw args (if it takes them) + // the following extra_args array is allocated space to take (in order): + // - values of positional default args (if any) + // - a single slot for default kw args dict (if it has them) + // - a single slot for var args tuple (if it takes them) + // - a single slot for kw args dict (if it takes them) mp_obj_t extra_args[]; } mp_obj_fun_bc_t; diff --git a/tests/basics/fun-kwonly.py b/tests/basics/fun-kwonly.py index 8bda68d95a..dd654eb441 100644 --- a/tests/basics/fun-kwonly.py +++ b/tests/basics/fun-kwonly.py @@ -43,14 +43,14 @@ def f(a, *, b, **kw): f(1, b=2) f(1, b=2, c=3) -## with a default value; not currently working -#def g(a, *, b=2, c): -# print(a, b, c) -# -#g(1, c=3) -#g(1, b=3, c=4) -#g(1, **{'c':3}) -#g(1, **{'b':'3', 'c':4}) +# with a default value +def g(a, *, b=2, c): + print(a, b, c) + +g(1, c=3) +g(1, b=3, c=4) +g(1, **{'c':3}) +g(1, **{'b':'3', 'c':4}) # with named star def f(*a, b, c):