Introduce a BlockLocalVariableNode

This is a tradeoff that I think is worth it. Right now we have a
location list that tracks the location of each of the block locals.
Instead, I'd like to make that a node list that has a proper node
in each spot in the list. In doing so, we eliminate the need to have
a location list at all, making it simpler on all of the various
consumers as we have one fewer field type. There should be minimal
memory implications here since this syntax is exceedingly rare.

Author: kddnewton

bin/memsize

@@ -16,7 +16,7 @@ results =
           8
         when "location", "location?"
           16
-        when "node[]", "string", "token", "token?", "location[]", "constant[]"
+        when "node[]", "string", "token", "token?", "constant[]"
           24
         when "flags"
           0

config.yml

@@ -520,6 +520,15 @@ nodes:
 
           bar(&args)
           ^^^^^^^^^^
+  - name: BlockLocalVariableNode
+    fields:
+      - name: name
+        type: constant
+    comment: |
+      Represents a block local variable.
+
+          a { |; b| }
+                 ^
   - name: BlockNode
     fields:
       - name: locals
@@ -556,7 +565,7 @@ nodes:
         type: node?
         kind: ParametersNode
       - name: locals
-        type: location[]
+        type: node[]
       - name: opening_loc
         type: location?
       - name: closing_loc

docs/configuration.md

@@ -51,7 +51,6 @@ The available values for `type` are:
 * `constant[]` - A child node that is an array of constants. This is a `yp_constant_id_list_t` in C.
 * `location` - A child node that is a location. This is a `yp_location_t` in C.
 * `location?` - A child node that is a location that is optionally present. This is a `yp_location_t` in C, but if the value is not present then the `start` and `end` fields will be `NULL`.
-* `location[]` - A child node that is an array of locations. This is a `yp_location_list_t` in C.
 * `uint32` - A child node that is a 32-bit unsigned integer. This is a `uint32_t` in C.
 
 If the type is `node` or `node?` then the value also accepts an optional `kind` key (a string). This key is expected to match to the name of another node type within `config.yml`. This changes a couple of places where code is templated out to use the more specific struct name instead of the generic `yp_node_t`. For example, with `kind: StatementsNode` the `yp_node_t *` in C becomes a `yp_statements_node_t *`.

docs/serialization.md

@@ -93,7 +93,6 @@ Depending on the type of child node, it could take a couple of different forms,
 * `constant[]` - A child node that is an array of constants. This is structured as a variable-length integer length, followed by the child constants themselves.
 * `location` - A child node that is a location. This is structured as a variable-length integer start followed by a variable-length integer length.
 * `location?` - A child node that is a location that is optionally present. If the location is not present, then a single `0` byte will be written in its place. If it is present, then it will be structured just like the `location` child node.
-* `location[]` - A child node that is an array of locations. This is structured as a `4` byte length, followed by the locations themselves.
 * `uint32` - A child node that is a 32-bit unsigned integer. This is structured as a variable-length integer.
 
 After the syntax tree, the content pool is serialized.

include/yarp/node.h

@@ -4,9 +4,6 @@
 #include "yarp/defines.h"
 #include "yarp/parser.h"
 
-// Append a token to the given list.
-void yp_location_list_append(yp_location_list_t *list, const yp_token_t *token);
-
 // Append a new node onto the end of the node list.
 void yp_node_list_append(yp_node_list_t *list, yp_node_t *node);
 
@@ -31,7 +28,6 @@ YP_EXPORTED_FUNCTION void yp_node_memsize(yp_node_t *node, yp_memsize_t *memsize
 YP_EXPORTED_FUNCTION const char * yp_node_type_to_str(yp_node_type_t node_type);
 
 #define YP_EMPTY_NODE_LIST ((yp_node_list_t) { .nodes = NULL, .size = 0, .capacity = 0 })
-#define YP_EMPTY_LOCATION_LIST ((yp_location_list_t) { .locations = NULL, .size = 0, .capacity = 0 })
 
 #endif // YARP_NODE_H
 

rust/yarp/build.rs

@@ -34,9 +34,6 @@ enum NodeFieldType {
     #[serde(rename = "location?")]
     OptionalLocation,
 
-    #[serde(rename = "location[]")]
-    LocationList,
-
     #[serde(rename = "uint32")]
     UInt32,
 
@@ -254,12 +251,6 @@ fn write_node(file: &mut File, node: &Node) -> Result<(), Box<dyn std::error::Er
                 writeln!(file, "        }}")?;
                 writeln!(file, "    }}")?;
             },
-            NodeFieldType::LocationList => {
-                writeln!(file, "    pub fn {}(&self) -> LocationList<'pr> {{", field.name)?;
-                writeln!(file, "        let pointer: *mut yp_location_list_t = unsafe {{ &mut (*self.pointer).{} }};", field.name)?;
-                writeln!(file, "        LocationList {{ pointer: unsafe {{ NonNull::new_unchecked(pointer) }}, marker: PhantomData }}")?;
-                writeln!(file, "    }}")?;
-            },
             NodeFieldType::UInt32 => {
                 writeln!(file, "    pub fn {}(&self) -> u32 {{", field.name)?;
                 writeln!(file, "        unsafe {{ (*self.pointer).{} }}", field.name)?;
@@ -454,51 +445,6 @@ impl std::fmt::Debug for Location<'_> {{
     }}
 }}
 
-/// An iterator over the ranges in a list.
-pub struct LocationListIter<'pr> {{
-    pointer: NonNull<yp_location_list_t>,
-    index: usize,
-    marker: PhantomData<&'pr mut yp_location_list_t>
-}}
-
-impl<'pr> Iterator for LocationListIter<'pr> {{
-    type Item = Location<'pr>;
-
-    fn next(&mut self) -> Option<Self::Item> {{
-        if self.index >= unsafe {{ self.pointer.as_ref().size }} {{
-            None
-        }} else {{
-            let pointer: *mut yp_location_t = unsafe {{ self.pointer.as_ref().locations.add(self.index) }};
-            self.index += 1;
-            Some(Location {{ pointer: unsafe {{ NonNull::new_unchecked(pointer) }}, marker: PhantomData }})
-        }}
-    }}
-}}
-
-/// A list of ranges in the source file.
-pub struct LocationList<'pr> {{
-    pointer: NonNull<yp_location_list_t>,
-    marker: PhantomData<&'pr mut yp_location_list_t>
-}}
-
-impl<'pr> LocationList<'pr> {{
-    /// Returns an iterator over the locations.
-    #[must_use]
-    pub fn iter(&self) -> LocationListIter<'pr> {{
-        LocationListIter {{
-            pointer: self.pointer,
-            index: 0,
-            marker: PhantomData
-        }}
-    }}
-}}
-
-impl std::fmt::Debug for LocationList<'_> {{
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {{
-        write!(f, "{{:?}}", self.iter().collect::<Vec<_>>())
-    }}
-}}
-
 /// An iterator over the nodes in a list.
 pub struct NodeListIter<'pr> {{
     parser: NonNull<yp_parser_t>,

rust/yarp/src/lib.rs

@@ -262,29 +262,6 @@ mod tests {
         assert_eq!(slice, "222");
     }
 
-    #[test]
-    fn location_list_test() {
-        use super::{
-            Visit,
-            visit_block_parameters_node, BlockParametersNode
-        };
-
-        struct BlockLocalsVisitor {}
-
-        impl Visit<'_> for BlockLocalsVisitor {
-            fn visit_block_parameters_node(&mut self, node: &BlockParametersNode<'_>) {
-                println!("{:?}", node.locals());
-                visit_block_parameters_node(self, node);
-            }
-        }
-
-        let source = "-> (; foo, bar) {}";
-        let result = parse(source.as_ref());
-
-        let mut visitor = BlockLocalsVisitor {};
-        visitor.visit(&result.node());
-    }
-
     #[test]
     fn visitor_test() {
         use super::{

src/yarp.c

@@ -1175,7 +1175,7 @@ yp_block_parameters_node_create(yp_parser_t *parser, yp_parameters_node_t *param
         .parameters = parameters,
         .opening_loc = YP_OPTIONAL_LOCATION_TOKEN_VALUE(opening),
         .closing_loc = { .start = NULL, .end = NULL },
-        .locals = YP_EMPTY_LOCATION_LIST
+        .locals = YP_EMPTY_NODE_LIST
     };
 
     return node;
@@ -1190,14 +1190,30 @@ yp_block_parameters_node_closing_set(yp_block_parameters_node_t *node, const yp_
     node->closing_loc = YP_LOCATION_TOKEN_VALUE(closing);
 }
 
+// Allocate and initialize a new BlockLocalVariableNode node.
+static yp_block_local_variable_node_t *
+yp_block_local_variable_node_create(yp_parser_t *parser, const yp_token_t *name) {
+    assert(name->type == YP_TOKEN_IDENTIFIER || name->type == YP_TOKEN_MISSING);
+    yp_block_local_variable_node_t *node = YP_ALLOC_NODE(parser, yp_block_local_variable_node_t);
+
+    *node = (yp_block_local_variable_node_t) {
+        {
+            .type = YP_NODE_BLOCK_LOCAL_VARIABLE_NODE,
+            .location = YP_LOCATION_TOKEN_VALUE(name),
+        },
+        .name = yp_parser_constant_id_token(parser, name)
+    };
+
+    return node;
+}
+
 // Append a new block-local variable to a BlockParametersNode node.
 static void
-yp_block_parameters_node_append_local(yp_block_parameters_node_t *node, const yp_token_t *local) {
-    assert(local->type == YP_TOKEN_IDENTIFIER || local->type == YP_TOKEN_MISSING);
+yp_block_parameters_node_append_local(yp_block_parameters_node_t *node, const yp_block_local_variable_node_t *local) {
+    yp_node_list_append(&node->locals, (yp_node_t *) local);
 
-    yp_location_list_append(&node->locals, local);
-    if (node->base.location.start == NULL) node->base.location.start = local->start;
-    node->base.location.end = local->end;
+    if (node->base.location.start == NULL) node->base.location.start = local->base.location.start;
+    node->base.location.end = local->base.location.end;
 }
 
 // Allocate and initialize a new BreakNode node.
@@ -9283,7 +9299,9 @@ parse_block_parameters(
         do {
             expect(parser, YP_TOKEN_IDENTIFIER, "Expected a local variable name.");
             yp_parser_local_add_token(parser, &parser->previous);
-            yp_block_parameters_node_append_local(block_parameters, &parser->previous);
+
+            yp_block_local_variable_node_t *local = yp_block_local_variable_node_create(parser, &parser->previous);
+            yp_block_parameters_node_append_local(block_parameters, local);
         } while (accept(parser, YP_TOKEN_COMMA));
     }
 

templates/ext/yarp/api_node.c.erb

@@ -152,12 +152,6 @@ yp_ast_new(yp_parser_t *parser, yp_node_t *node, rb_encoding *encoding) {
                     }
                     <%- when YARP::StringField -%>
                     argv[<%= index %>] = yp_string_new(&cast-><%= field.name %>, encoding);
-                    <%- when YARP::LocationListField -%>
-                    argv[<%= index %>] = rb_ary_new_capa(cast-><%= field.name %>.size);
-                    for (size_t index = 0; index < cast-><%= field.name %>.size; index++) {
-                        yp_location_t location = cast-><%= field.name %>.locations[index];
-                        rb_ary_push(argv[<%= index %>], yp_location_new(parser, location.start, location.end, source));
-                    }
                     <%- when YARP::ConstantField -%>
                     argv[<%= index %>] = rb_id2sym(constants[cast-><%= field.name %> - 1]);
                     <%- when YARP::ConstantListField -%>

templates/include/yarp/ast.h.erb

@@ -32,12 +32,6 @@ typedef struct {
     const uint8_t *end;
 } yp_location_t;
 
-typedef struct {
-    yp_location_t *locations;
-    size_t size;
-    size_t capacity;
-} yp_location_list_t;
-
 struct yp_node;
 
 typedef struct yp_node_list {
@@ -95,7 +89,6 @@ typedef struct yp_<%= node.human %> {
     <%= case field
     when YARP::NodeField, YARP::OptionalNodeField then "struct #{field.c_type} *#{field.name}"
     when YARP::NodeListField then "struct yp_node_list #{field.name}"
-    when YARP::LocationListField then "yp_location_list_t #{field.name}"
     when YARP::ConstantField then "yp_constant_id_t #{field.name}"
     when YARP::ConstantListField then "yp_constant_id_list_t #{field.name}"
     when YARP::StringField then "yp_string_t #{field.name}"