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

Add Handles #59

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Add Handles #59

wants to merge 7 commits into from

Conversation

Joeoc2001
Copy link

@Joeoc2001 Joeoc2001 commented Feb 24, 2025

The Naga project uses a Handle-Arena approach to AST management rather than a Box-Ownership model. I'm fond of this for a few reasons, including performance and ease of AST mutation (especially with a visitor pattern, where you might first visit immutably and collect handles and then perform modifications).

This PR adds the option to generate an Arena-based grammar. It doesn't change any public signatures of existing generated code, but upon declaration of a grammar with a least one Handle<T> field the rust_sitter::language macro additionally requires a rust_sitter::arena type somewhere in the declaration:

#[rust_sitter::grammar("arithmetic")]
pub mod grammar {
    use rust_sitter::Handle;

    #[rust_sitter::arena]
    #[derive(Default, Debug)]
    pub struct MyArena;

    #[rust_sitter::language]
    #[derive(PartialEq, Eq, Debug)]
    pub enum Expression {
        Number(#[rust_sitter::leaf(pattern = r"\d+", transform = |v| v.parse().unwrap())] i32),
        #[rust_sitter::prec_left(1)]
        Sub(
            Handle<Expression>,
            #[rust_sitter::leaf(text = "-")] (),
            Handle<Expression>,
        ),
        #[rust_sitter::prec_left(2)]
        Mul(
            Handle<Expression>,
            #[rust_sitter::leaf(text = "*")] (),
            Handle<Expression>,
        ),
    }

    #[rust_sitter::extra]
    struct Whitespace {
        #[rust_sitter::leaf(pattern = r"\s")]
        _whitespace: (),
    }
}

Then, parse returns both the root Expression and an instance of MyArena which the handles point in to.

As a note, while this improves the performance of AST transformations it currently this doesn't change the performance of parsing (about a 1-2% speedup). I'd anticipate that a preliminary traversal of the tree for an arena pre-allocation would unlock more performance, so if that's a requirement to get this merged then let me know and I'll give it a go. Otherwise, as it stands, this allows for a new paradigm with associated performance characteristics without changing much else.

@shadaj
Copy link
Member

shadaj commented Mar 8, 2025

Hey! Just wanted to ack this PR, will try to look through it carefully soon. This looks broadly to be in the right direction; would be really awesome to explore a prototype of what incremental updates could look like just to make sure this does indeed allow for that.

@Joeoc2001
Copy link
Author

Thanks for the ack! I've added an example where arithmetic has had one Boxs replaced with a Handle - let me know if that wasn't what you were looking for

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.

2 participants