Skip to content

Commit 69b89d2

Browse files
dpgeorgedpgeorge
committed
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.
1 parent 0e3329a commit 69b89d2

5 files changed

Lines changed: 35 additions & 48 deletions

File tree

‎py/compile.c‎

Lines changed: 24 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,8 @@ typedef struct _compiler_t {
5353
uint16_t n_arg_keyword;
5454
uint8_t star_flags;
5555
uint8_t have_bare_star;
56-
uint8_t param_pass;
57-
uint16_t param_pass_num_dict_params;
58-
uint16_t param_pass_num_default_params;
56+
uint16_t num_dict_params;
57+
uint16_t num_default_params;
5958

6059
scope_t *scope_head;
6160
scope_t *scope_cur;
@@ -897,7 +896,7 @@ void compile_funcdef_param(compiler_t *comp, mp_parse_node_t pn) {
897896
// this parameter does not have a default value
898897

899898
// check for non-default parameters given after default parameters (allowed by parser, but not syntactically valid)
900-
if (!comp->have_bare_star && comp->param_pass_num_default_params != 0) {
899+
if (!comp->have_bare_star && comp->num_default_params != 0) {
901900
compile_syntax_error(comp, pn, "non-default argument follows default argument");
902901
return;
903902
}
@@ -907,27 +906,23 @@ void compile_funcdef_param(compiler_t *comp, mp_parse_node_t pn) {
907906
// in CPython, None (and True, False?) as default parameters are loaded with LOAD_NAME; don't understandy why
908907

909908
if (comp->have_bare_star) {
910-
comp->param_pass_num_dict_params += 1;
911-
if (comp->param_pass == 1) {
909+
comp->num_dict_params += 1;
912910
#if !MICROPY_EMIT_CPYTHON
913-
// in Micro Python we put the default dict parameters into a dictionary using the bytecode
914-
if (comp->param_pass_num_dict_params == 1) {
915-
// first default dict param, so make the map
916-
EMIT_ARG(build_map, 0);
917-
}
911+
// in Micro Python we put the default dict parameters into a dictionary using the bytecode
912+
if (comp->num_dict_params == 1) {
913+
// first default dict param, so make the map
914+
EMIT_ARG(build_map, 0);
915+
}
918916
#endif
919-
EMIT_ARG(load_const_id, MP_PARSE_NODE_LEAF_ARG(pn_id));
920-
compile_node(comp, pn_equal);
917+
EMIT_ARG(load_const_id, MP_PARSE_NODE_LEAF_ARG(pn_id));
918+
compile_node(comp, pn_equal);
921919
#if !MICROPY_EMIT_CPYTHON
922-
// in Micro Python we put the default dict parameters into a dictionary using the bytecode
923-
EMIT(store_map);
920+
// in Micro Python we put the default dict parameters into a dictionary using the bytecode
921+
EMIT(store_map);
924922
#endif
925-
}
926923
} else {
927-
comp->param_pass_num_default_params += 1;
928-
if (comp->param_pass == 2) {
929-
compile_node(comp, pn_equal);
930-
}
924+
comp->num_default_params += 1;
925+
compile_node(comp, pn_equal);
931926
}
932927
}
933928

@@ -948,48 +943,36 @@ qstr compile_funcdef_helper(compiler_t *comp, mp_parse_node_struct_t *pns, uint
948943

949944
// save variables (probably don't need to do this, since we can't have nested definitions..?)
950945
uint old_have_bare_star = comp->have_bare_star;
951-
uint old_param_pass = comp->param_pass;
952-
uint old_param_pass_num_dict_params = comp->param_pass_num_dict_params;
953-
uint old_param_pass_num_default_params = comp->param_pass_num_default_params;
946+
uint old_num_dict_params = comp->num_dict_params;
947+
uint old_num_default_params = comp->num_default_params;
954948

955949
// compile default parameters
956-
957-
// pass 1 does any default parameters after bare star
958950
comp->have_bare_star = false;
959-
comp->param_pass = 1;
960-
comp->param_pass_num_dict_params = 0;
961-
comp->param_pass_num_default_params = 0;
951+
comp->num_dict_params = 0;
952+
comp->num_default_params = 0;
962953
apply_to_single_or_list(comp, pns->nodes[1], PN_typedargslist, compile_funcdef_param);
963954

964955
if (comp->had_error) {
965956
return MP_QSTR_NULL;
966957
}
967958

968-
// pass 2 does any default parameters before bare star
969-
comp->have_bare_star = false;
970-
comp->param_pass = 2;
971-
comp->param_pass_num_dict_params = 0;
972-
comp->param_pass_num_default_params = 0;
973-
apply_to_single_or_list(comp, pns->nodes[1], PN_typedargslist, compile_funcdef_param);
974-
975959
#if !MICROPY_EMIT_CPYTHON
976960
// in Micro Python we put the default positional parameters into a tuple using the bytecode
977-
if (comp->param_pass_num_default_params > 0) {
978-
EMIT_ARG(build_tuple, comp->param_pass_num_default_params);
961+
if (comp->num_default_params > 0) {
962+
EMIT_ARG(build_tuple, comp->num_default_params);
979963
}
980964
#endif
981965

982966
// get the scope for this function
983967
scope_t *fscope = (scope_t*)pns->nodes[4];
984968

985969
// make the function
986-
close_over_variables_etc(comp, fscope, comp->param_pass_num_default_params, comp->param_pass_num_dict_params);
970+
close_over_variables_etc(comp, fscope, comp->num_default_params, comp->num_dict_params);
987971

988972
// restore variables
989973
comp->have_bare_star = old_have_bare_star;
990-
comp->param_pass = old_param_pass;
991-
comp->param_pass_num_dict_params = old_param_pass_num_dict_params;
992-
comp->param_pass_num_default_params = old_param_pass_num_default_params;
974+
comp->num_dict_params = old_num_dict_params;
975+
comp->num_default_params = old_num_default_params;
993976

994977
// return its name (the 'f' in "def f(...):")
995978
return fscope->simple_name;

‎py/emitbc.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -744,10 +744,10 @@ STATIC void emit_bc_make_function(emit_t *emit, scope_t *scope, uint n_pos_defau
744744
if (n_pos_defaults == 0) {
745745
// load dummy entry for non-existent positional default tuple
746746
emit_bc_load_null(emit);
747+
emit_bc_rot_two(emit);
747748
} else if (n_kw_defaults == 0) {
748749
// load dummy entry for non-existent keyword default dict
749750
emit_bc_load_null(emit);
750-
emit_bc_rot_two(emit);
751751
}
752752
emit_bc_pre(emit, -1);
753753
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
762762
if (n_pos_defaults == 0) {
763763
// load dummy entry for non-existent positional default tuple
764764
emit_bc_load_null(emit);
765-
emit_bc_rot_two(emit);
765+
emit_bc_rot_three(emit);
766766
} else if (n_kw_defaults == 0) {
767767
// load dummy entry for non-existent keyword default dict
768768
emit_bc_load_null(emit);
769-
emit_bc_rot_three(emit);
769+
emit_bc_rot_two(emit);
770770
}
771771
emit_bc_pre(emit, -2);
772772
emit_write_byte_code_byte_uint(emit, MP_BC_MAKE_CLOSURE_DEFARGS, scope->unique_code_id);

‎py/emitglue.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,9 @@ mp_obj_t mp_make_function_from_id(uint unique_code_id, mp_obj_t def_args, mp_obj
197197
return mp_const_none;
198198
}
199199

200+
// def_args must be MP_OBJ_NULL or a tuple
201+
assert(def_args == MP_OBJ_NULL || MP_OBJ_IS_TYPE(def_args, &mp_type_tuple));
202+
200203
// TODO implement default kw args
201204
assert(def_kw_args == MP_OBJ_NULL);
202205

‎py/objfun.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,7 @@ mp_obj_t mp_obj_new_fun_bc(uint scope_flags, qstr *args, uint n_args, mp_obj_t d
361361
uint n_extra_args = 0;
362362
mp_obj_tuple_t *def_args = def_args_in;
363363
if (def_args != MP_OBJ_NULL) {
364+
assert(MP_OBJ_IS_TYPE(def_args, &mp_type_tuple));
364365
n_def_args = def_args->len;
365366
n_extra_args = def_args->len;
366367
}

‎py/vm.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -718,9 +718,9 @@ mp_vm_return_kind_t mp_execute_byte_code_2(const byte *code_info, const byte **i
718718

719719
case MP_BC_MAKE_FUNCTION_DEFARGS:
720720
DECODE_UINT;
721-
// Stack layout: def_dict def_tuple <- TOS
721+
// Stack layout: def_tuple def_dict <- TOS
722722
obj1 = POP();
723-
SET_TOP(mp_make_function_from_id(unum, obj1, TOP()));
723+
SET_TOP(mp_make_function_from_id(unum, TOP(), obj1));
724724
break;
725725

726726
case MP_BC_MAKE_CLOSURE:
@@ -731,10 +731,10 @@ mp_vm_return_kind_t mp_execute_byte_code_2(const byte *code_info, const byte **i
731731

732732
case MP_BC_MAKE_CLOSURE_DEFARGS:
733733
DECODE_UINT;
734-
// Stack layout: def_dict def_tuple closure_tuple <- TOS
734+
// Stack layout: def_tuple def_dict closure_tuple <- TOS
735735
obj1 = POP();
736736
obj2 = POP();
737-
SET_TOP(mp_make_closure_from_id(unum, obj1, obj2, TOP()));
737+
SET_TOP(mp_make_closure_from_id(unum, obj1, TOP(), obj2));
738738
break;
739739

740740
case MP_BC_CALL_FUNCTION:

0 commit comments

Comments
 (0)