Skip to content

Conversation

@alexcrocha
Copy link

@alexcrocha alexcrocha commented Nov 20, 2025

This PR implements support for rbs_node and rbs_node_list field types, which were missed in the initial Node/NodeList implementation (#57).

Changes:

  • Adds accessor generation for fields of type rbs_node and rbs_node_list in build.rs.
  • Handles the Rust keyword collision for fields named type by escaping the accessor to r#type() and mapping to the underlying type_ C field.
  • Updates NodeList constructor to work with the generated accessors.

This enables traversal of nested AST structures (e.g. accessing a class's members or a constant's type) which was previously not possible through the bindings.

Copy link
Author

alexcrocha commented Nov 20, 2025

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@alexcrocha alexcrocha marked this pull request as ready for review November 20, 2025 01:43
Copy link
Member

@vinistock vinistock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a point about the type method name, but otherwise it looks good

}
"rbs_node" => {
let (function_name, field_name) = match field.name.as_str() {
"type" => ("r#type", "type_"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean that the method will be created with the name type? If so, I'd vote for us to use type_.

Since type is a reserved keyword, invoking the method might prove a little odd, especially if we end up having to use the raw identifier syntax (r#type()).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We would have to use the raw indentifier syntax. I agree it's odd.

I'll change it to type_ like bindgen does

This adds support for `rbs_node` and `rbs_node_list` field types in the
bindings generator. This enables access to child nodes and node lists
from parent AST nodes.

We handle fields named `type` (which is a Rust keyword) by escaping the
accessor method to `r#type()` and correctly mapping to the underlying
`type_` field in the C struct.
@alexcrocha alexcrocha force-pushed the 11-19-handle_rbs_node_and_rbs_node_list_types branch from 3d63b58 to 245bf8d Compare November 20, 2025 18:23
@alexcrocha alexcrocha merged commit e9eeadb into ar-rust-rbs-build Nov 20, 2025
16 of 27 checks passed
alexcrocha added a commit that referenced this pull request Jan 13, 2026
Enable nested AST traversal by exposing rbs_node and rbs_node_list fields

Nested structure traversal (e.g., class members, constant types) depends on access to rbs_node and rbs_node_list fields. Making these fields accessible aligns the Rust bindings with the C API. Fields named "type" are accessible via type_ to avoid a Rust keyword collision
alexcrocha added a commit that referenced this pull request Jan 13, 2026
Enable nested AST traversal by exposing rbs_node and rbs_node_list fields

Nested structure traversal (e.g., class members, constant types) depends on access to rbs_node and rbs_node_list fields. Making these fields accessible aligns the Rust bindings with the C API. Fields named "type" are accessible via type_ to avoid a Rust keyword collision
alexcrocha added a commit that referenced this pull request Jan 14, 2026
Enable nested AST traversal by exposing rbs_node and rbs_node_list fields

Nested structure traversal (e.g., class members, constant types) depends on access to rbs_node and rbs_node_list fields. Making these fields accessible aligns the Rust bindings with the C API. Fields named "type" are accessible via type_ to avoid a Rust keyword collision
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.

3 participants