Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Universal parser] Improve AST structure #10660

Merged
merged 1 commit into from Apr 28, 2024

Conversation

hasumikin
Copy link
Contributor

This patch moves ast->node_buffer->config to ast->config aiming to improve readability of the source.

Background

We could not add the config field to the rb_ast_t * before due to the five-word restriction of the IMEMO object. But it is now doable by merging #10618

@yui-knk
Copy link
Contributor

yui-knk commented Apr 27, 2024

IIRC, ifdef UNIVERSAL_PARSER branch in rb_ast_free and rb_ast_dispose can be simpler by this change. Will you include the simplification into this PR?

@hasumikin hasumikin marked this pull request as draft April 27, 2024 09:06
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 ruby#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;
}
```
@hasumikin hasumikin marked this pull request as ready for review April 28, 2024 02:46
@yui-knk yui-knk merged commit ddd8da4 into ruby:master Apr 28, 2024
101 checks passed
@hasumikin hasumikin deleted the improve-AST-structure branch April 28, 2024 07:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants