From ddd8da4b6ba3dfcca21ca710e7cef2fa3b9632d7 Mon Sep 17 00:00:00 2001
From: HASUMI Hitoshi <hasumikin@gmail.com>
Date: Sat, 27 Apr 2024 16:28:52 +0900
Subject: [PATCH] [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;
}
```
---
 node.c        | 39 +++++++++------------------------------
 node.h        |  3 ---
 ruby_parser.c | 11 +++++++----
 rubyparser.h  |  7 +++++++
 4 files changed, 23 insertions(+), 37 deletions(-)

diff --git a/node.c b/node.c
index 2d3ef6171d..60e01caa31 100644
--- a/node.c
+++ b/node.c
@@ -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
diff --git a/node.h b/node.h
index 80db9e8403..1e411f211b 100644
--- a/node.h
+++ b/node.h
@@ -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;