From 2fe4cf7761ffc40d81b4780d59c2b6ef0a5c16ff Mon Sep 17 00:00:00 2001 From: stijn Date: Sat, 4 Oct 2014 08:51:33 +0200 Subject: [PATCH] Implement kwargs for builtin open() and _io.FileIO This makes open() and _io.FileIO() more CPython compliant. The mode kwarg is fully iplemented. The encoding kwarg is allowed but not implemented; mainly to allow the tests to specify encoding for CPython, see #874 --- bare-arm/main.c | 4 +- py/builtin.h | 2 +- py/qstrdefs.h | 3 ++ qemu-arm/main.c | 4 +- qemu-arm/test_main.c | 4 +- stmhal/file.c | 89 ++++++++++++++++++++++++-------------------- unix/file.c | 43 +++++++++++++-------- unix/modsocket.c | 4 +- 8 files changed, 90 insertions(+), 63 deletions(-) diff --git a/bare-arm/main.c b/bare-arm/main.c index 084374d3f1..0724f735ea 100644 --- a/bare-arm/main.c +++ b/bare-arm/main.c @@ -70,10 +70,10 @@ mp_import_stat_t mp_import_stat(const char *path) { return MP_IMPORT_STAT_NO_EXIST; } -mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args) { +mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args, mp_map_t *kwargs) { return mp_const_none; } -MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, mp_builtin_open); +MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open); void nlr_jump_fail(void *val) { } diff --git a/py/builtin.h b/py/builtin.h index 01ec22d2b0..a69712bec7 100644 --- a/py/builtin.h +++ b/py/builtin.h @@ -25,7 +25,7 @@ */ mp_obj_t mp_builtin___import__(mp_uint_t n_args, mp_obj_t *args); -mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args); +mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args, mp_map_t *kwargs); MP_DECLARE_CONST_FUN_OBJ(mp_builtin___build_class___obj); MP_DECLARE_CONST_FUN_OBJ(mp_builtin___import___obj); diff --git a/py/qstrdefs.h b/py/qstrdefs.h index 2c2c45b5be..c10b2fabac 100644 --- a/py/qstrdefs.h +++ b/py/qstrdefs.h @@ -446,6 +446,9 @@ Q(StringIO) Q(BytesIO) Q(getvalue) Q(file) +Q(mode) +Q(r) +Q(encoding) #endif #if MICROPY_PY_GC diff --git a/qemu-arm/main.c b/qemu-arm/main.c index 7bb8472541..1189ae7b8b 100644 --- a/qemu-arm/main.c +++ b/qemu-arm/main.c @@ -70,10 +70,10 @@ mp_import_stat_t mp_import_stat(const char *path) { return MP_IMPORT_STAT_NO_EXIST; } -mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args) { +mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args, mp_map_t *kwargs) { return mp_const_none; } -MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, mp_builtin_open); +MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open); void nlr_jump_fail(void *val) { } diff --git a/qemu-arm/test_main.c b/qemu-arm/test_main.c index 099cd94ba1..fb95130772 100644 --- a/qemu-arm/test_main.c +++ b/qemu-arm/test_main.c @@ -78,10 +78,10 @@ mp_import_stat_t mp_import_stat(const char *path) { return MP_IMPORT_STAT_NO_EXIST; } -mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args) { +mp_obj_t mp_builtin_open(uint n_args, const mp_obj_t *args, mp_map_t *kwargs) { return mp_const_none; } -MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, mp_builtin_open); +MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open); void nlr_jump_fail(void *val) { } diff --git a/stmhal/file.c b/stmhal/file.c index d49a8a259b..16cca79370 100644 --- a/stmhal/file.c +++ b/stmhal/file.c @@ -160,49 +160,51 @@ mp_obj_t file_obj_tell(mp_obj_t self_in) { } STATIC MP_DEFINE_CONST_FUN_OBJ_1(file_obj_tell_obj, file_obj_tell); -STATIC mp_obj_t file_obj_make_new(mp_obj_t type, mp_uint_t n_args, mp_uint_t n_kw, const mp_obj_t *args) { - mp_arg_check_num(n_args, n_kw, 1, 2, false); - - const char *fname = mp_obj_str_get_str(args[0]); +// Note: encoding is ignored for now; it's also not a valid kwarg for CPython's FileIO, +// but by adding it here we can use one single mp_arg_t array for open() and FileIO's constructor +STATIC const mp_arg_t file_open_args[] = { + { MP_QSTR_file, MP_ARG_OBJ | MP_ARG_REQUIRED, {.u_obj = mp_const_none} }, + { MP_QSTR_mode, MP_ARG_OBJ, {.u_obj = MP_OBJ_NEW_QSTR(MP_QSTR_r)} }, + { MP_QSTR_encoding, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none} }, +}; +#define FILE_OPEN_NUM_ARGS MP_ARRAY_SIZE(file_open_args) +STATIC mp_obj_t file_open(mp_obj_t type, mp_arg_val_t *args) { int mode = 0; - if (n_args == 1) { - mode = FA_READ; - } else { - const char *mode_s = mp_obj_str_get_str(args[1]); - // TODO make sure only one of r, w, x, a, and b, t are specified - while (*mode_s) { - switch (*mode_s++) { - case 'r': - mode |= FA_READ; - break; - case 'w': - mode |= FA_WRITE | FA_CREATE_ALWAYS; - break; - case 'x': - mode |= FA_WRITE | FA_CREATE_NEW; - break; - case 'a': - mode |= FA_WRITE | FA_OPEN_ALWAYS; - break; - case '+': - mode |= FA_READ | FA_WRITE; - break; - #if MICROPY_PY_IO_FILEIO - case 'b': - type = (mp_obj_t)&mp_type_fileio; - break; - #endif - case 't': - type = (mp_obj_t)&mp_type_textio; - break; - } + const char *mode_s = mp_obj_str_get_str(args[1].u_obj); + // TODO make sure only one of r, w, x, a, and b, t are specified + while (*mode_s) { + switch (*mode_s++) { + case 'r': + mode |= FA_READ; + break; + case 'w': + mode |= FA_WRITE | FA_CREATE_ALWAYS; + break; + case 'x': + mode |= FA_WRITE | FA_CREATE_NEW; + break; + case 'a': + mode |= FA_WRITE | FA_OPEN_ALWAYS; + break; + case '+': + mode |= FA_READ | FA_WRITE; + break; + #if MICROPY_PY_IO_FILEIO + case 'b': + type = (mp_obj_t)&mp_type_fileio; + break; + #endif + case 't': + type = (mp_obj_t)&mp_type_textio; + break; } } pyb_file_obj_t *o = m_new_obj_with_finaliser(pyb_file_obj_t); o->base.type = type; + const char *fname = mp_obj_str_get_str(args[0].u_obj); FRESULT res = f_open(&o->fp, fname, mode); if (res != FR_OK) { m_del_obj(pyb_file_obj_t, o); @@ -217,6 +219,12 @@ STATIC mp_obj_t file_obj_make_new(mp_obj_t type, mp_uint_t n_args, mp_uint_t n_k return o; } +STATIC mp_obj_t file_obj_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_kw, const mp_obj_t *args) { + mp_arg_val_t arg_vals[FILE_OPEN_NUM_ARGS]; + mp_arg_parse_all_kw_array(n_args, n_kw, args, FILE_OPEN_NUM_ARGS, file_open_args, arg_vals); + return file_open(type_in, arg_vals); +} + // TODO gc hook to close the file if not already closed STATIC const mp_map_elem_t rawfile_locals_dict_table[] = { @@ -273,9 +281,10 @@ const mp_obj_type_t mp_type_textio = { }; // Factory function for I/O stream classes -STATIC mp_obj_t pyb_io_open(mp_uint_t n_args, const mp_obj_t *args) { - // TODO: analyze mode and buffering args and instantiate appropriate type - return file_obj_make_new((mp_obj_t)&mp_type_textio, n_args, 0, args); +mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args, mp_map_t *kwargs) { + // TODO: analyze buffering args and instantiate appropriate type + mp_arg_val_t arg_vals[FILE_OPEN_NUM_ARGS]; + mp_arg_parse_all(n_args, args, kwargs, FILE_OPEN_NUM_ARGS, file_open_args, arg_vals); + return file_open((mp_obj_t)&mp_type_textio, arg_vals); } - -MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, pyb_io_open); +MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open); diff --git a/unix/file.c b/unix/file.c index bb7ed247f0..21b2726848 100644 --- a/unix/file.c +++ b/unix/file.c @@ -118,16 +118,19 @@ STATIC mp_obj_t fdfile_fileno(mp_obj_t self_in) { } STATIC MP_DEFINE_CONST_FUN_OBJ_1(fdfile_fileno_obj, fdfile_fileno); -STATIC mp_obj_t fdfile_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_kw, const mp_obj_t *args) { +// Note: encoding is ignored for now; it's also not a valid kwarg for CPython's FileIO, +// but by adding it here we can use one single mp_arg_t array for open() and FileIO's constructor +STATIC const mp_arg_t file_open_args[] = { + { MP_QSTR_file, MP_ARG_OBJ | MP_ARG_REQUIRED, {.u_obj = mp_const_none} }, + { MP_QSTR_mode, MP_ARG_OBJ, {.u_obj = MP_OBJ_NEW_QSTR(MP_QSTR_r)} }, + { MP_QSTR_encoding, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = mp_const_none} }, +}; +#define FILE_OPEN_NUM_ARGS MP_ARRAY_SIZE(file_open_args) + +STATIC mp_obj_t fdfile_open(mp_obj_t type_in, mp_arg_val_t *args) { mp_obj_fdfile_t *o = m_new_obj(mp_obj_fdfile_t); mp_const_obj_t type = type_in; - - const char *mode_s; - if (n_args > 1) { - mode_s = mp_obj_str_get_str(args[1]); - } else { - mode_s = "r"; - } + const char *mode_s = mp_obj_str_get_str(args[1].u_obj); int mode = 0; while (*mode_s) { @@ -159,12 +162,14 @@ STATIC mp_obj_t fdfile_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_ o->base.type = type; - if (MP_OBJ_IS_SMALL_INT(args[0])) { - o->fd = MP_OBJ_SMALL_INT_VALUE(args[0]); + mp_obj_t fid = args[0].u_obj; + + if (MP_OBJ_IS_SMALL_INT(fid)) { + o->fd = MP_OBJ_SMALL_INT_VALUE(fid); return o; } - const char *fname = mp_obj_str_get_str(args[0]); + const char *fname = mp_obj_str_get_str(fid); int fd = open(fname, mode, 0644); if (fd == -1) { nlr_raise(mp_obj_new_exception_arg1(&mp_type_OSError, MP_OBJ_NEW_SMALL_INT(errno))); @@ -173,6 +178,12 @@ STATIC mp_obj_t fdfile_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_ return o; } +STATIC mp_obj_t fdfile_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_kw, const mp_obj_t *args) { + mp_arg_val_t arg_vals[FILE_OPEN_NUM_ARGS]; + mp_arg_parse_all_kw_array(n_args, n_kw, args, FILE_OPEN_NUM_ARGS, file_open_args, arg_vals); + return fdfile_open(type_in, arg_vals); +} + STATIC const mp_map_elem_t rawfile_locals_dict_table[] = { { MP_OBJ_NEW_QSTR(MP_QSTR_fileno), (mp_obj_t)&fdfile_fileno_obj }, { MP_OBJ_NEW_QSTR(MP_QSTR_read), (mp_obj_t)&mp_stream_read_obj }, @@ -225,11 +236,13 @@ const mp_obj_type_t mp_type_textio = { }; // Factory function for I/O stream classes -mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args) { - // TODO: analyze mode and buffering args and instantiate appropriate type - return fdfile_make_new((mp_obj_t)&mp_type_textio, n_args, 0, args); +mp_obj_t mp_builtin_open(mp_uint_t n_args, const mp_obj_t *args, mp_map_t *kwargs) { + // TODO: analyze buffering args and instantiate appropriate type + mp_arg_val_t arg_vals[FILE_OPEN_NUM_ARGS]; + mp_arg_parse_all(n_args, args, kwargs, FILE_OPEN_NUM_ARGS, file_open_args, arg_vals); + return fdfile_open((mp_obj_t)&mp_type_textio, arg_vals); } -MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(mp_builtin_open_obj, 1, 2, mp_builtin_open); +MP_DEFINE_CONST_FUN_OBJ_KW(mp_builtin_open_obj, 1, mp_builtin_open); const mp_obj_fdfile_t mp_sys_stdin_obj = { .base = {&mp_type_textio}, .fd = STDIN_FILENO }; const mp_obj_fdfile_t mp_sys_stdout_obj = { .base = {&mp_type_textio}, .fd = STDOUT_FILENO }; diff --git a/unix/modsocket.c b/unix/modsocket.c index 7f9184db64..82d833653f 100644 --- a/unix/modsocket.c +++ b/unix/modsocket.c @@ -257,7 +257,9 @@ STATIC mp_obj_t socket_makefile(mp_uint_t n_args, const mp_obj_t *args) { mp_obj_t *new_args = alloca(n_args * sizeof(mp_obj_t)); memcpy(new_args + 1, args + 1, (n_args - 1) * sizeof(mp_obj_t)); new_args[0] = MP_OBJ_NEW_SMALL_INT(self->fd); - return mp_builtin_open(n_args, new_args); + mp_map_t kwargs; + mp_map_init(&kwargs, 0); + return mp_builtin_open(n_args, new_args, &kwargs); } STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(socket_makefile_obj, 1, 3, socket_makefile);