diff options
author | HASUMI Hitoshi <hasumikin@gmail.com> | 2024-04-27 16:28:52 +0900 |
---|---|---|
committer | Yuichiro Kaneko <spiketeika@gmail.com> | 2024-04-28 12:08:21 +0900 |
commit | ddd8da4b6ba3dfcca21ca710e7cef2fa3b9632d7 (patch) | |
tree | a1549d2a8b59ec0b5a7b187d804c3b594a7c0108 | |
parent | 8ad0b2cd310b4ca5af9a24610117a05acc35bcae (diff) |
[Universal parser] Improve AST structure
This patch moves `ast->node_buffer->config` to `ast->config` aiming to improve readability and maintainability of the source.
## Background
We could not add the `config` field to the `rb_ast_t *` due to the five-word restriction of the IMEMO object.
But it is now doable by merging https://github.com/ruby/ruby/pull/10618
## About assigning `&rb_global_parser_config` to `ast->config` in `ast_alloc()`
The approach of not setting `ast->config` in `ast_alloc()` means that the client, CRuby in this scenario, that directly calls `ast_alloc()` will be responsible for releasing it if a resource that is passed to AST needs to be released.
However, we have put on hold whether we can guarantee the above so far, thus, this patch looks like that.
```
// ruby_parser.c
static VALUE
ast_alloc(void)
{
rb_ast_t *ast;
VALUE vast = TypedData_Make_Struct(0, rb_ast_t, &ast_data_type, ast);
#ifdef UNIVERSAL_PARSER
ast = (rb_ast_t *)DATA_PTR(vast);
ast->config = &rb_global_parser_config;
#endif
return vast;
}
```
-rw-r--r-- | node.c | 39 | ||||
-rw-r--r-- | node.h | 3 | ||||
-rw-r--r-- | ruby_parser.c | 11 | ||||
-rw-r--r-- | rubyparser.h | 7 |
4 files changed, 23 insertions, 37 deletions
@@ -59,19 +59,16 @@ rb_node_buffer_new(void) init_node_buffer_list(&nb->buffer_list, (node_buffer_elem_t*)&nb[1], ruby_xmalloc); nb->local_tables = 0; nb->tokens = 0; -#ifdef UNIVERSAL_PARSER - nb->config = config; -#endif return nb; } #ifdef UNIVERSAL_PARSER #undef ruby_xmalloc -#define ruby_xmalloc ast->node_buffer->config->malloc +#define ruby_xmalloc ast->config->malloc #undef xfree -#define xfree ast->node_buffer->config->free -#define rb_xmalloc_mul_add ast->node_buffer->config->xmalloc_mul_add -#define ruby_xrealloc(var,size) (ast->node_buffer->config->realloc_n((void *)var, 1, size)) +#define xfree ast->config->free +#define rb_xmalloc_mul_add ast->config->xmalloc_mul_add +#define ruby_xrealloc(var,size) (ast->config->realloc_n((void *)var, 1, size)) #endif typedef void node_itr_t(rb_ast_t *ast, void *ctx, NODE *node); @@ -218,8 +215,8 @@ free_ast_value(rb_ast_t *ast, void *ctx, NODE *node) static void rb_node_buffer_free(rb_ast_t *ast, node_buffer_t *nb) { - if (ast->node_buffer && ast->node_buffer->tokens) { - parser_tokens_free(ast, ast->node_buffer->tokens); + if (nb && nb->tokens) { + parser_tokens_free(ast, nb->tokens); } iterate_node_values(ast, &nb->buffer_list, free_ast_value, NULL); node_buffer_list_free(ast, &nb->buffer_list); @@ -304,7 +301,9 @@ rb_ast_t * rb_ast_new(const rb_parser_config_t *config) { node_buffer_t *nb = rb_node_buffer_new(config); - return config->ast_new(nb); + rb_ast_t *ast = config->ast_new(nb); + ast->config = config; + return ast; } #else rb_ast_t * @@ -351,24 +350,8 @@ script_lines_free(rb_ast_t *ast, rb_parser_ary_t *script_lines) void rb_ast_free(rb_ast_t *ast) { - /* TODO - * The current impl. of rb_ast_free() and rb_ast_dispose() is complicated. - * Upcoming task of changing `ast->node_buffer->config` to `ast->config`, - * that is based on "deIMEMO" of `rb_ast_t *`, will let us simplify the code. - */ -#ifdef UNIVERSAL_PARSER - if (ast && ast->node_buffer) { - void (*free_func)(void *) = xfree; - script_lines_free(ast, ast->body.script_lines); - ast->body.script_lines = NULL; - rb_node_buffer_free(ast, ast->node_buffer); - ast->node_buffer = 0; - free_func(ast); - } -#else rb_ast_dispose(ast); xfree(ast); -#endif } static size_t @@ -431,16 +414,12 @@ rb_ast_memsize(const rb_ast_t *ast) void rb_ast_dispose(rb_ast_t *ast) { -#ifdef UNIVERSAL_PARSER - // noop. See the comment in rb_ast_free(). -#else if (ast && ast->node_buffer) { script_lines_free(ast, ast->body.script_lines); ast->body.script_lines = NULL; rb_node_buffer_free(ast, ast->node_buffer); ast->node_buffer = 0; } -#endif } VALUE @@ -39,9 +39,6 @@ struct node_buffer_struct { // - location info // Array, whose entry is array rb_parser_ary_t *tokens; -#ifdef UNIVERSAL_PARSER - const rb_parser_config_t *config; -#endif }; RUBY_SYMBOL_EXPORT_BEGIN diff --git a/ruby_parser.c b/ruby_parser.c index 63cd8df814..40b7e18aca 100644 --- a/ruby_parser.c +++ b/ruby_parser.c @@ -758,9 +758,7 @@ static void ast_free(void *ptr) { rb_ast_t *ast = (rb_ast_t *)ptr; - if (ast) { - rb_ast_free(ast); - } + rb_ast_free(ast); } static const rb_data_type_t ast_data_type = { @@ -777,7 +775,12 @@ static VALUE ast_alloc(void) { rb_ast_t *ast; - return TypedData_Make_Struct(0, rb_ast_t, &ast_data_type, ast); + VALUE vast = TypedData_Make_Struct(0, rb_ast_t, &ast_data_type, ast); +#ifdef UNIVERSAL_PARSER + ast = (rb_ast_t *)DATA_PTR(vast); + ast->config = &rb_global_parser_config; +#endif + return vast; } VALUE diff --git a/rubyparser.h b/rubyparser.h index 22ac7df088..813ad321e2 100644 --- a/rubyparser.h +++ b/rubyparser.h @@ -1209,6 +1209,10 @@ typedef struct RNode_ERROR { typedef struct node_buffer_struct node_buffer_t; +#ifdef UNIVERSAL_PARSER +typedef struct rb_parser_config_struct rb_parser_config_t; +#endif + typedef struct rb_ast_body_struct { const NODE *root; rb_parser_ary_t *script_lines; @@ -1219,6 +1223,9 @@ typedef struct rb_ast_body_struct { typedef struct rb_ast_struct { node_buffer_t *node_buffer; rb_ast_body_t body; +#ifdef UNIVERSAL_PARSER + const rb_parser_config_t *config; +#endif } rb_ast_t; |