From 1831034be13fef5344583c557ff089df31788251 Mon Sep 17 00:00:00 2001 From: Damien George Date: Tue, 14 Mar 2017 11:16:31 +1100 Subject: [PATCH] py: Allow lexer to raise exceptions during construction. This patch refactors the error handling in the lexer, to simplify it (ie reduce code size). A long time ago, when the lexer/parser/compiler were first written, the lexer and parser were designed so they didn't use exceptions (ie nlr) to report errors but rather returned an error code. Over time that has gradually changed, the parser in particular has more and more ways of raising exceptions. Also, the lexer never really handled all errors without raising, eg there were some memory errors which could raise an exception (and in these rare cases one would get a fatal nlr-not-handled fault). This patch accepts the fact that the lexer can raise exceptions in some cases and allows it to raise exceptions to handle all its errors, which are for the most part just out-of-memory errors during construction of the lexer. This makes the lexer a bit simpler, and also the persistent code stuff is simplified. What this means for users of the lexer is that calls to it must be wrapped in a nlr handler. But all uses of the lexer already have such an nlr handler for the parser (and compiler) so that doesn't put any extra burden on the callers. --- extmod/vfs_reader.c | 29 +++++++++-------------------- py/builtinevex.c | 3 --- py/builtinimport.c | 17 +++-------------- py/lexer.c | 31 +++++-------------------------- py/persistentcode.c | 9 ++------- py/reader.c | 28 +++++++++------------------- py/reader.h | 6 +++--- 7 files changed, 31 insertions(+), 92 deletions(-) diff --git a/extmod/vfs_reader.c b/extmod/vfs_reader.c index 9509582d22..891098aa1e 100644 --- a/extmod/vfs_reader.c +++ b/extmod/vfs_reader.c @@ -27,7 +27,7 @@ #include #include -#include "py/nlr.h" +#include "py/runtime.h" #include "py/stream.h" #include "py/reader.h" #include "extmod/vfs.h" @@ -69,30 +69,19 @@ STATIC void mp_reader_vfs_close(void *data) { m_del_obj(mp_reader_vfs_t, reader); } -int mp_reader_new_file(mp_reader_t *reader, const char *filename) { - mp_reader_vfs_t *rf = m_new_obj_maybe(mp_reader_vfs_t); - if (rf == NULL) { - return MP_ENOMEM; - } - // TODO we really should just let this function raise a uPy exception - nlr_buf_t nlr; - if (nlr_push(&nlr) == 0) { - mp_obj_t arg = mp_obj_new_str(filename, strlen(filename), false); - rf->file = mp_vfs_open(1, &arg, (mp_map_t*)&mp_const_empty_map); - int errcode; - rf->len = mp_stream_rw(rf->file, rf->buf, sizeof(rf->buf), &errcode, MP_STREAM_RW_READ | MP_STREAM_RW_ONCE); - nlr_pop(); - if (errcode != 0) { - return errcode; - } - } else { - return MP_ENOENT; // assume error was "file not found" +void mp_reader_new_file(mp_reader_t *reader, const char *filename) { + mp_reader_vfs_t *rf = m_new_obj(mp_reader_vfs_t); + mp_obj_t arg = mp_obj_new_str(filename, strlen(filename), false); + rf->file = mp_vfs_open(1, &arg, (mp_map_t*)&mp_const_empty_map); + int errcode; + rf->len = mp_stream_rw(rf->file, rf->buf, sizeof(rf->buf), &errcode, MP_STREAM_RW_READ | MP_STREAM_RW_ONCE); + if (errcode != 0) { + mp_raise_OSError(errcode); } rf->pos = 0; reader->data = rf; reader->readbyte = mp_reader_vfs_readbyte; reader->close = mp_reader_vfs_close; - return 0; // success } #endif // MICROPY_READER_VFS diff --git a/py/builtinevex.c b/py/builtinevex.c index 636f869300..b0514ee994 100644 --- a/py/builtinevex.c +++ b/py/builtinevex.c @@ -136,9 +136,6 @@ STATIC mp_obj_t eval_exec_helper(size_t n_args, const mp_obj_t *args, mp_parse_i mp_lexer_t *lex; if (MICROPY_PY_BUILTINS_EXECFILE && parse_input_kind == MP_PARSE_SINGLE_INPUT) { lex = mp_lexer_new_from_file(str); - if (lex == NULL) { - nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_OSError, "could not open file '%s'", str)); - } parse_input_kind = MP_PARSE_FILE_INPUT; } else { lex = mp_lexer_new_from_str_len(MP_QSTR__lt_string_gt_, str, str_len, 0); diff --git a/py/builtinimport.c b/py/builtinimport.c index 0a917c6f83..96846b9636 100644 --- a/py/builtinimport.c +++ b/py/builtinimport.c @@ -131,18 +131,7 @@ STATIC mp_import_stat_t find_file(const char *file_str, uint file_len, vstr_t *d } #if MICROPY_ENABLE_COMPILER -STATIC void do_load_from_lexer(mp_obj_t module_obj, mp_lexer_t *lex, const char *fname) { - - if (lex == NULL) { - // we verified the file exists using stat, but lexer could still fail - if (MICROPY_ERROR_REPORTING == MICROPY_ERROR_REPORTING_TERSE) { - mp_raise_msg(&mp_type_ImportError, "module not found"); - } else { - nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_ImportError, - "no module named '%s'", fname)); - } - } - +STATIC void do_load_from_lexer(mp_obj_t module_obj, mp_lexer_t *lex) { #if MICROPY_PY___FILE__ qstr source_name = lex->source_name; mp_store_attr(module_obj, MP_QSTR___file__, MP_OBJ_NEW_QSTR(source_name)); @@ -207,7 +196,7 @@ STATIC void do_load(mp_obj_t module_obj, vstr_t *file) { // found the filename in the list of frozen files, then load and execute it. #if MICROPY_MODULE_FROZEN_STR if (frozen_type == MP_FROZEN_STR) { - do_load_from_lexer(module_obj, modref, file_str); + do_load_from_lexer(module_obj, modref); return; } #endif @@ -235,7 +224,7 @@ STATIC void do_load(mp_obj_t module_obj, vstr_t *file) { #if MICROPY_ENABLE_COMPILER { mp_lexer_t *lex = mp_lexer_new_from_file(file_str); - do_load_from_lexer(module_obj, lex, file_str); + do_load_from_lexer(module_obj, lex); return; } #endif diff --git a/py/lexer.c b/py/lexer.c index 9dcdd19eb5..fadaee6f35 100644 --- a/py/lexer.c +++ b/py/lexer.c @@ -699,13 +699,7 @@ void mp_lexer_to_next(mp_lexer_t *lex) { } mp_lexer_t *mp_lexer_new(qstr src_name, mp_reader_t reader) { - mp_lexer_t *lex = m_new_obj_maybe(mp_lexer_t); - - // check for memory allocation error - if (lex == NULL) { - reader.close(reader.data); - return NULL; - } + mp_lexer_t *lex = m_new_obj(mp_lexer_t); lex->source_name = src_name; lex->reader = reader; @@ -715,16 +709,9 @@ mp_lexer_t *mp_lexer_new(qstr src_name, mp_reader_t reader) { lex->nested_bracket_level = 0; lex->alloc_indent_level = MICROPY_ALLOC_LEXER_INDENT_INIT; lex->num_indent_level = 1; - lex->indent_level = m_new_maybe(uint16_t, lex->alloc_indent_level); + lex->indent_level = m_new(uint16_t, lex->alloc_indent_level); vstr_init(&lex->vstr, 32); - // check for memory allocation error - // note: vstr_init above may fail on malloc, but so may mp_lexer_to_next below - if (lex->indent_level == NULL) { - mp_lexer_free(lex); - return NULL; - } - // store sentinel for first indentation level lex->indent_level[0] = 0; @@ -764,9 +751,7 @@ mp_lexer_t *mp_lexer_new(qstr src_name, mp_reader_t reader) { mp_lexer_t *mp_lexer_new_from_str_len(qstr src_name, const char *str, size_t len, size_t free_len) { mp_reader_t reader; - if (!mp_reader_new_mem(&reader, (const byte*)str, len, free_len)) { - return NULL; - } + mp_reader_new_mem(&reader, (const byte*)str, len, free_len); return mp_lexer_new(src_name, reader); } @@ -774,10 +759,7 @@ mp_lexer_t *mp_lexer_new_from_str_len(qstr src_name, const char *str, size_t len mp_lexer_t *mp_lexer_new_from_file(const char *filename) { mp_reader_t reader; - int ret = mp_reader_new_file(&reader, filename); - if (ret != 0) { - return NULL; - } + mp_reader_new_file(&reader, filename); return mp_lexer_new(qstr_from_str(filename), reader); } @@ -785,10 +767,7 @@ mp_lexer_t *mp_lexer_new_from_file(const char *filename) { mp_lexer_t *mp_lexer_new_from_fd(qstr filename, int fd, bool close_fd) { mp_reader_t reader; - int ret = mp_reader_new_file_from_fd(&reader, fd, close_fd); - if (ret != 0) { - return NULL; - } + mp_reader_new_file_from_fd(&reader, fd, close_fd); return mp_lexer_new(filename, reader); } diff --git a/py/persistentcode.c b/py/persistentcode.c index 5cb5117093..2a9a5b7cc0 100644 --- a/py/persistentcode.c +++ b/py/persistentcode.c @@ -225,18 +225,13 @@ mp_raw_code_t *mp_raw_code_load(mp_reader_t *reader) { mp_raw_code_t *mp_raw_code_load_mem(const byte *buf, size_t len) { mp_reader_t reader; - if (!mp_reader_new_mem(&reader, buf, len, 0)) { - m_malloc_fail(BYTES_PER_WORD); // we need to raise a MemoryError - } + mp_reader_new_mem(&reader, buf, len, 0); return mp_raw_code_load(&reader); } mp_raw_code_t *mp_raw_code_load_file(const char *filename) { mp_reader_t reader; - int ret = mp_reader_new_file(&reader, filename); - if (ret != 0) { - mp_raise_OSError(ret); - } + mp_reader_new_file(&reader, filename); return mp_raw_code_load(&reader); } diff --git a/py/reader.c b/py/reader.c index d7de7aa6c4..5df45c4957 100644 --- a/py/reader.c +++ b/py/reader.c @@ -27,6 +27,7 @@ #include #include +#include "py/runtime.h" #include "py/mperrno.h" #include "py/reader.h" @@ -54,11 +55,8 @@ STATIC void mp_reader_mem_close(void *data) { m_del_obj(mp_reader_mem_t, reader); } -bool mp_reader_new_mem(mp_reader_t *reader, const byte *buf, size_t len, size_t free_len) { - mp_reader_mem_t *rm = m_new_obj_maybe(mp_reader_mem_t); - if (rm == NULL) { - return false; - } +void mp_reader_new_mem(mp_reader_t *reader, const byte *buf, size_t len, size_t free_len) { + mp_reader_mem_t *rm = m_new_obj(mp_reader_mem_t); rm->free_len = free_len; rm->beg = buf; rm->cur = buf; @@ -66,7 +64,6 @@ bool mp_reader_new_mem(mp_reader_t *reader, const byte *buf, size_t len, size_t reader->data = rm; reader->readbyte = mp_reader_mem_readbyte; reader->close = mp_reader_mem_close; - return true; } #if MICROPY_READER_POSIX @@ -110,14 +107,8 @@ STATIC void mp_reader_posix_close(void *data) { m_del_obj(mp_reader_posix_t, reader); } -int mp_reader_new_file_from_fd(mp_reader_t *reader, int fd, bool close_fd) { - mp_reader_posix_t *rp = m_new_obj_maybe(mp_reader_posix_t); - if (rp == NULL) { - if (close_fd) { - close(fd); - } - return MP_ENOMEM; - } +void mp_reader_new_file_from_fd(mp_reader_t *reader, int fd, bool close_fd) { + mp_reader_posix_t *rp = m_new_obj(mp_reader_posix_t); rp->close_fd = close_fd; rp->fd = fd; int n = read(rp->fd, rp->buf, sizeof(rp->buf)); @@ -125,22 +116,21 @@ int mp_reader_new_file_from_fd(mp_reader_t *reader, int fd, bool close_fd) { if (close_fd) { close(fd); } - return errno; + mp_raise_OSError(errno); } rp->len = n; rp->pos = 0; reader->data = rp; reader->readbyte = mp_reader_posix_readbyte; reader->close = mp_reader_posix_close; - return 0; // success } -int mp_reader_new_file(mp_reader_t *reader, const char *filename) { +void mp_reader_new_file(mp_reader_t *reader, const char *filename) { int fd = open(filename, O_RDONLY, 0644); if (fd < 0) { - return errno; + mp_raise_OSError(errno); } - return mp_reader_new_file_from_fd(reader, fd, true); + mp_reader_new_file_from_fd(reader, fd, true); } #endif diff --git a/py/reader.h b/py/reader.h index b02d96149b..8511c72ce5 100644 --- a/py/reader.h +++ b/py/reader.h @@ -39,8 +39,8 @@ typedef struct _mp_reader_t { void (*close)(void *data); } mp_reader_t; -bool mp_reader_new_mem(mp_reader_t *reader, const byte *buf, size_t len, size_t free_len); -int mp_reader_new_file(mp_reader_t *reader, const char *filename); -int mp_reader_new_file_from_fd(mp_reader_t *reader, int fd, bool close_fd); +void mp_reader_new_mem(mp_reader_t *reader, const byte *buf, size_t len, size_t free_len); +void mp_reader_new_file(mp_reader_t *reader, const char *filename); +void mp_reader_new_file_from_fd(mp_reader_t *reader, int fd, bool close_fd); #endif // MICROPY_INCLUDED_PY_READER_H