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

Clean the AST alloc interface #2111

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

yannham
Copy link
Member

@yannham yannham commented Nov 27, 2024

A previous PR introduced generic alloc and alloc_iter methods, instead of needing a myriad of xxx_move functions for each and every AST component type. However, this poses a risk of memory leak because this method allocates in the generic bumpalo arena, which doesn't run destructors.

This commit makes this approach safer by using a marker trait which forbids some AST components from being allocated through this method, when they need to be dropped. Anecdotally, alloc_iter is renamed to alloc_many and a specialized version alloc_singleton is added to eliminate a lot of alloc.alloc_many(iter::once(x)).

In a second time, all the legacy xxx_move variants or the like are removed, and replaced with alloc and alloc_many whenever possible, making the interface of AstAlloc quite smaller.

@yannham yannham force-pushed the rfc007/streamline-alloc-interface branch 5 times, most recently from c1dd1c2 to 716244d Compare November 27, 2024 15:14
@yannham yannham requested a review from jneem November 27, 2024 15:14
Copy link
Contributor

github-actions bot commented Nov 27, 2024

A previous PR introduced generic `alloc` and `alloc_iter` methods,
instead of needing a myriad of `xxx_move` functions for each and every
AST component type. However, this poses a risk of memory leak because
this method allocates in the generic bumpalo arena, which doesn't run
destructors.

This commit makes this approach safer by using a marker trait which
forbids some AST components from being allocated through this method,
when they need to be dropped. Anecdotally, `alloc_iter` is renamed to
`alloc_many` and a specialized version `alloc_singleton` is added to
eliminate a lot of `alloc.alloc_many(iter::once(x))`.

In a second time, all the legacy `xxx_move` variants or the like are
removed, and replaced with `alloc` and `alloc_many` whenever possible,
making the interface of `AstAlloc` quite smaller.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant