From f615d82d5b4a8877462ce7c4f511e24c94a7b740 Mon Sep 17 00:00:00 2001 From: Damien George Date: Fri, 24 Feb 2017 14:56:37 +1100 Subject: [PATCH] py/parse: Simplify handling of errors by raising them directly. The parser was originally written to work without raising any exceptions and instead return an error value to the caller. But it's now required that a call to the parser be wrapped in an nlr handler, so we may as well make use of that fact and simplify the parser so that it doesn't need to keep track of any memory errors that it had. The parser anyway explicitly raises an exception at the end if there was an error. This patch simplifies the parser by letting the underlying memory allocation functions raise an exception if they fail to allocate any memory. And if there is an error parsing the " = const()" pattern then that also raises an exception right away instead of trying to recover gracefully and then raise. --- py/parse.c | 119 +++++++++++------------------------------------------ 1 file changed, 25 insertions(+), 94 deletions(-) diff --git a/py/parse.c b/py/parse.c index 5a5adc6093..2f16748a6c 100644 --- a/py/parse.c +++ b/py/parse.c @@ -147,15 +147,7 @@ typedef struct _mp_parse_chunk_t { byte data[]; } mp_parse_chunk_t; -typedef enum { - PARSE_ERROR_NONE = 0, - PARSE_ERROR_MEMORY, - PARSE_ERROR_CONST, -} parse_error_t; - typedef struct _parser_t { - parse_error_t parse_error; - size_t rule_stack_alloc; size_t rule_stack_top; rule_stack_t *rule_stack; @@ -216,15 +208,8 @@ STATIC void *parser_alloc(parser_t *parser, size_t num_bytes) { } STATIC void push_rule(parser_t *parser, size_t src_line, const rule_t *rule, size_t arg_i) { - if (parser->parse_error) { - return; - } if (parser->rule_stack_top >= parser->rule_stack_alloc) { - rule_stack_t *rs = m_renew_maybe(rule_stack_t, parser->rule_stack, parser->rule_stack_alloc, parser->rule_stack_alloc + MICROPY_ALLOC_PARSE_RULE_INC, true); - if (rs == NULL) { - parser->parse_error = PARSE_ERROR_MEMORY; - return; - } + rule_stack_t *rs = m_renew(rule_stack_t, parser->rule_stack, parser->rule_stack_alloc, parser->rule_stack_alloc + MICROPY_ALLOC_PARSE_RULE_INC); parser->rule_stack = rs; parser->rule_stack_alloc += MICROPY_ALLOC_PARSE_RULE_INC; } @@ -241,7 +226,6 @@ STATIC void push_rule_from_arg(parser_t *parser, size_t arg) { } STATIC void pop_rule(parser_t *parser, const rule_t **rule, size_t *arg_i, size_t *src_line) { - assert(!parser->parse_error); parser->rule_stack_top -= 1; *rule = rules[parser->rule_stack[parser->rule_stack_top].rule_id]; *arg_i = parser->rule_stack[parser->rule_stack_top].arg_i; @@ -354,31 +338,18 @@ STATIC void result_stack_show(parser_t *parser) { */ STATIC mp_parse_node_t pop_result(parser_t *parser) { - if (parser->parse_error) { - return MP_PARSE_NODE_NULL; - } assert(parser->result_stack_top > 0); return parser->result_stack[--parser->result_stack_top]; } STATIC mp_parse_node_t peek_result(parser_t *parser, size_t pos) { - if (parser->parse_error) { - return MP_PARSE_NODE_NULL; - } assert(parser->result_stack_top > pos); return parser->result_stack[parser->result_stack_top - 1 - pos]; } STATIC void push_result_node(parser_t *parser, mp_parse_node_t pn) { - if (parser->parse_error) { - return; - } if (parser->result_stack_top >= parser->result_stack_alloc) { - mp_parse_node_t *stack = m_renew_maybe(mp_parse_node_t, parser->result_stack, parser->result_stack_alloc, parser->result_stack_alloc + MICROPY_ALLOC_PARSE_RESULT_INC, true); - if (stack == NULL) { - parser->parse_error = PARSE_ERROR_MEMORY; - return; - } + mp_parse_node_t *stack = m_renew(mp_parse_node_t, parser->result_stack, parser->result_stack_alloc, parser->result_stack_alloc + MICROPY_ALLOC_PARSE_RESULT_INC); parser->result_stack = stack; parser->result_stack_alloc += MICROPY_ALLOC_PARSE_RESULT_INC; } @@ -387,10 +358,6 @@ STATIC void push_result_node(parser_t *parser, mp_parse_node_t pn) { STATIC mp_parse_node_t make_node_const_object(parser_t *parser, size_t src_line, mp_obj_t obj) { mp_parse_node_struct_t *pn = parser_alloc(parser, sizeof(mp_parse_node_struct_t) + sizeof(mp_obj_t)); - if (pn == NULL) { - parser->parse_error = PARSE_ERROR_MEMORY; - return MP_PARSE_NODE_NULL; - } pn->source_line = src_line; #if MICROPY_OBJ_REPR == MICROPY_OBJ_REPR_D // nodes are 32-bit pointers, but need to store 64-bit object @@ -653,8 +620,11 @@ STATIC bool fold_constants(parser_t *parser, const rule_t *rule, size_t num_args mp_parse_node_t pn_value = ((mp_parse_node_struct_t*)((mp_parse_node_struct_t*)pn1)->nodes[1])->nodes[0]; mp_obj_t value; if (!mp_parse_node_get_int_maybe(pn_value, &value)) { - parser->parse_error = PARSE_ERROR_CONST; - return false; + mp_obj_t exc = mp_obj_new_exception_msg(&mp_type_SyntaxError, + "constant must be an integer"); + mp_obj_exception_add_traceback(exc, parser->lexer->source_name, + ((mp_parse_node_struct_t*)pn1)->source_line, MP_QSTR_NULL); + nlr_raise(exc); } // store the value in the table of dynamic constants @@ -755,10 +725,6 @@ STATIC void push_result_rule(parser_t *parser, size_t src_line, const rule_t *ru #endif mp_parse_node_struct_t *pn = parser_alloc(parser, sizeof(mp_parse_node_struct_t) + sizeof(mp_parse_node_t) * num_args); - if (pn == NULL) { - parser->parse_error = PARSE_ERROR_MEMORY; - return; - } pn->source_line = src_line; pn->kind_num_nodes = (rule->rule_id & 0xff) | (num_args << 8); for (size_t i = num_args; i > 0; i--) { @@ -773,15 +739,13 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) { parser_t parser; - parser.parse_error = PARSE_ERROR_NONE; - parser.rule_stack_alloc = MICROPY_ALLOC_PARSE_RULE_INIT; parser.rule_stack_top = 0; - parser.rule_stack = m_new_maybe(rule_stack_t, parser.rule_stack_alloc); + parser.rule_stack = m_new(rule_stack_t, parser.rule_stack_alloc); parser.result_stack_alloc = MICROPY_ALLOC_PARSE_RESULT_INIT; parser.result_stack_top = 0; - parser.result_stack = m_new_maybe(mp_parse_node_t, parser.result_stack_alloc); + parser.result_stack = m_new(mp_parse_node_t, parser.result_stack_alloc); parser.lexer = lex; @@ -792,11 +756,6 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) { mp_map_init(&parser.consts, 0); #endif - // check if we could allocate the stacks - if (parser.rule_stack == NULL || parser.result_stack == NULL) { - goto memory_error; - } - // work out the top-level rule to use, and push it on the stack size_t top_level_rule; switch (input_kind) { @@ -815,7 +774,7 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) { for (;;) { next_rule: - if (parser.rule_stack_top == 0 || parser.parse_error) { + if (parser.rule_stack_top == 0) { break; } @@ -1077,27 +1036,12 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) { parser.tree.chunk = parser.cur_chunk; } - mp_obj_t exc; - - if (parser.parse_error) { - #if MICROPY_COMP_CONST - if (parser.parse_error == PARSE_ERROR_CONST) { - exc = mp_obj_new_exception_msg(&mp_type_SyntaxError, - "constant must be an integer"); - } else - #endif - { - assert(parser.parse_error == PARSE_ERROR_MEMORY); - memory_error: - exc = mp_obj_new_exception_msg(&mp_type_MemoryError, - "parser could not allocate enough memory"); - } - parser.tree.root = MP_PARSE_NODE_NULL; - } else if ( + if ( lex->tok_kind != MP_TOKEN_END // check we are at the end of the token stream || parser.result_stack_top == 0 // check that we got a node (can fail on empty input) ) { - syntax_error: + syntax_error:; + mp_obj_t exc; if (lex->tok_kind == MP_TOKEN_INDENT) { exc = mp_obj_new_exception_msg(&mp_type_IndentationError, "unexpected indent"); @@ -1108,37 +1052,24 @@ mp_parse_tree_t mp_parse(mp_lexer_t *lex, mp_parse_input_kind_t input_kind) { exc = mp_obj_new_exception_msg(&mp_type_SyntaxError, "invalid syntax"); } - parser.tree.root = MP_PARSE_NODE_NULL; - } else { - // no errors - - //result_stack_show(parser); - //printf("rule stack alloc: %d\n", parser.rule_stack_alloc); - //printf("result stack alloc: %d\n", parser.result_stack_alloc); - //printf("number of parse nodes allocated: %d\n", num_parse_nodes_allocated); - - // get the root parse node that we created - assert(parser.result_stack_top == 1); - exc = MP_OBJ_NULL; - parser.tree.root = parser.result_stack[0]; + // add traceback to give info about file name and location + // we don't have a 'block' name, so just pass the NULL qstr to indicate this + mp_obj_exception_add_traceback(exc, lex->source_name, lex->tok_line, MP_QSTR_NULL); + nlr_raise(exc); } + // get the root parse node that we created + assert(parser.result_stack_top == 1); + parser.tree.root = parser.result_stack[0]; + // free the memory that we don't need anymore m_del(rule_stack_t, parser.rule_stack, parser.rule_stack_alloc); m_del(mp_parse_node_t, parser.result_stack, parser.result_stack_alloc); - // we also free the lexer on behalf of the caller (see below) - if (exc != MP_OBJ_NULL) { - // had an error so raise the exception - // add traceback to give info about file name and location - // we don't have a 'block' name, so just pass the NULL qstr to indicate this - mp_obj_exception_add_traceback(exc, lex->source_name, lex->tok_line, MP_QSTR_NULL); - mp_lexer_free(lex); - nlr_raise(exc); - } else { - mp_lexer_free(lex); - return parser.tree; - } + // we also free the lexer on behalf of the caller + mp_lexer_free(lex); + + return parser.tree; } void mp_parse_tree_clear(mp_parse_tree_t *tree) {