Skip to content

Commit

Permalink
[ruby/yarp] Introduce a BlockLocalVariableNode
Browse files Browse the repository at this point in the history
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.

ruby/prism@04d329ddf0
  • Loading branch information
kddnewton authored and matzbot committed Sep 5, 2023
1 parent 9a8398a commit c384ef0
Show file tree
Hide file tree
Showing 23 changed files with 77 additions and 95 deletions.
6 changes: 6 additions & 0 deletions test/yarp/location_test.rb
Expand Up @@ -67,6 +67,12 @@ def test_BlockArgumentNode
assert_location(BlockArgumentNode, "foo(&bar)", 4...8) { |node| node.arguments.arguments.last }
end

def test_BlockLocalVariableNode
assert_location(BlockLocalVariableNode, "foo { |;bar| }", 8...11) do |node|
node.block.parameters.locals.first
end
end

def test_BlockNode
assert_location(BlockNode, "foo {}", 4...6, &:block)
assert_location(BlockNode, "foo do end", 4...10, &:block)
Expand Down
4 changes: 3 additions & 1 deletion test/yarp/snapshots/procs.txt
Expand Up @@ -16,7 +16,9 @@ ProgramNode(0...266)(
nil,
nil
),
[(7...8), (10...11), (13...14)],
[BlockLocalVariableNode(7...8)(:b),
BlockLocalVariableNode(10...11)(:c),
BlockLocalVariableNode(13...14)(:d)],
(3...4),
(14...15)
),
Expand Down
2 changes: 1 addition & 1 deletion test/yarp/snapshots/seattlerb/block_arg_scope.txt
Expand Up @@ -20,7 +20,7 @@ ProgramNode(0...12)(
nil,
nil
),
[(8...9)],
[BlockLocalVariableNode(8...9)(:c)],
(4...5),
(9...10)
),
Expand Down
3 changes: 2 additions & 1 deletion test/yarp/snapshots/seattlerb/block_arg_scope2.txt
Expand Up @@ -20,7 +20,8 @@ ProgramNode(0...14)(
nil,
nil
),
[(7...8), (10...11)],
[BlockLocalVariableNode(7...8)(:c),
BlockLocalVariableNode(10...11)(:d)],
(3...4),
(11...12)
),
Expand Down
7 changes: 6 additions & 1 deletion test/yarp/snapshots/seattlerb/block_scope.txt
Expand Up @@ -10,7 +10,12 @@ ProgramNode(0...10)(
nil,
BlockNode(2...10)(
[:b],
BlockParametersNode(4...8)(nil, [(6...7)], (4...5), (7...8)),
BlockParametersNode(4...8)(
nil,
[BlockLocalVariableNode(6...7)(:b)],
(4...5),
(7...8)
),
nil,
(2...3),
(9...10)
Expand Down
7 changes: 6 additions & 1 deletion test/yarp/snapshots/seattlerb/pipe_semicolon.txt
Expand Up @@ -10,7 +10,12 @@ ProgramNode(0...18)(
nil,
BlockNode(4...18)(
[:c],
BlockParametersNode(7...14)(nil, [(11...12)], (7...8), (13...14)),
BlockParametersNode(7...14)(
nil,
[BlockLocalVariableNode(11...12)(:c)],
(7...8),
(13...14)
),
nil,
(4...6),
(15...18)
Expand Down
2 changes: 1 addition & 1 deletion test/yarp/snapshots/seattlerb/stabby_proc_scope.txt
Expand Up @@ -16,7 +16,7 @@ ProgramNode(0...11)(
nil,
nil
),
[(6...7)],
[BlockLocalVariableNode(6...7)(:b)],
(2...3),
(7...8)
),
Expand Down
9 changes: 5 additions & 4 deletions test/yarp/snapshots/unparser/corpus/literal/block.txt
Expand Up @@ -91,7 +91,7 @@ ProgramNode(0...737)(
nil,
nil
),
[(44...45)],
[BlockLocalVariableNode(44...45)(:x)],
(39...40),
(45...46)
),
Expand Down Expand Up @@ -326,7 +326,7 @@ ProgramNode(0...737)(
nil,
nil
),
[(181...182)],
[BlockLocalVariableNode(181...182)(:b)],
(176...177),
(182...183)
),
Expand Down Expand Up @@ -366,7 +366,7 @@ ProgramNode(0...737)(
nil,
nil
),
[(200...201)],
[BlockLocalVariableNode(200...201)(:b)],
(196...197),
(201...202)
),
Expand Down Expand Up @@ -398,7 +398,8 @@ ProgramNode(0...737)(
[:a, :b],
BlockParametersNode(215...223)(
nil,
[(218...219), (221...222)],
[BlockLocalVariableNode(218...219)(:a),
BlockLocalVariableNode(221...222)(:b)],
(215...216),
(222...223)
),
Expand Down
2 changes: 1 addition & 1 deletion test/yarp/snapshots/unparser/corpus/literal/lambda.txt
Expand Up @@ -110,7 +110,7 @@ ProgramNode(0...80)(
nil,
nil
),
[(74...75)],
[BlockLocalVariableNode(74...75)(:c)],
(67...68),
(75...76)
),
Expand Down
7 changes: 6 additions & 1 deletion test/yarp/snapshots/whitequark/arg_scope.txt
Expand Up @@ -10,7 +10,12 @@ ProgramNode(0...13)(
nil,
BlockNode(6...13)(
[:a],
BlockParametersNode(7...11)(nil, [(9...10)], (7...8), (10...11)),
BlockParametersNode(7...11)(
nil,
[BlockLocalVariableNode(9...10)(:a)],
(7...8),
(10...11)
),
StatementsNode(11...12)([LocalVariableReadNode(11...12)(:a, 0)]),
(6...7),
(12...13)
Expand Down
4 changes: 2 additions & 2 deletions test/yarp/snapshots/whitequark/blockargs.txt
Expand Up @@ -250,7 +250,7 @@ ProgramNode(0...550)(
[:a],
BlockParametersNode(117...123)(
nil,
[(120...121)],
[BlockLocalVariableNode(120...121)(:a)],
(117...118),
(122...123)
),
Expand All @@ -272,7 +272,7 @@ ProgramNode(0...550)(
[:a],
BlockParametersNode(130...134)(
nil,
[(132...133)],
[BlockLocalVariableNode(132...133)(:a)],
(130...131),
(133...134)
),
Expand Down
3 changes: 2 additions & 1 deletion test/yarp/snapshots/whitequark/send_lambda_args_shadow.txt
Expand Up @@ -16,7 +16,8 @@ ProgramNode(0...19)(
nil,
nil
),
[(6...9), (11...14)],
[BlockLocalVariableNode(6...9)(:foo),
BlockLocalVariableNode(11...14)(:bar)],
(2...3),
(14...15)
),
Expand Down
11 changes: 10 additions & 1 deletion yarp/config.yml
Expand Up @@ -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
Expand Down Expand Up @@ -556,7 +565,7 @@ nodes:
type: node?
kind: ParametersNode
- name: locals
type: location[]
type: node[]
- name: opening_loc
type: location?
- name: closing_loc
Expand Down
4 changes: 0 additions & 4 deletions yarp/node.h
Expand Up @@ -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);

Expand All @@ -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

Expand Down
6 changes: 0 additions & 6 deletions yarp/templates/ext/yarp/api_node.c.erb
Expand Up @@ -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 -%>
Expand Down
7 changes: 0 additions & 7 deletions yarp/templates/include/yarp/ast.h.erb
Expand Up @@ -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 {
Expand Down Expand Up @@ -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}"
Expand Down
2 changes: 1 addition & 1 deletion yarp/templates/lib/yarp/node.rb.erb
Expand Up @@ -100,7 +100,7 @@ module YARP
<%- case field -%>
<%- when YARP::NodeListField -%>
inspector << "<%= pointer %><%= field.name %>: #{inspector.list("#{inspector.prefix}<%= preadd %>", <%= field.name %>)}"
<%- when YARP::LocationListField, YARP::ConstantListField -%>
<%- when YARP::ConstantListField -%>
inspector << "<%= pointer %><%= field.name %>: #{<%= field.name %>.inspect}\n"
<%- when YARP::NodeField -%>
inspector << "<%= pointer %><%= field.name %>:\n"
Expand Down
1 change: 0 additions & 1 deletion yarp/templates/lib/yarp/serialize.rb.erb
Expand Up @@ -185,7 +185,6 @@ module YARP
when YARP::OptionalNodeField then "load_optional_node"
when YARP::StringField then "load_string"
when YARP::NodeListField then "Array.new(load_varint) { load_node }"
when YARP::LocationListField then "Array.new(load_varint) { load_location }"
when YARP::ConstantField then "load_constant"
when YARP::ConstantListField then "Array.new(load_varint) { load_constant }"
when YARP::LocationField then "load_location"
Expand Down
28 changes: 0 additions & 28 deletions yarp/templates/src/node.c.erb
Expand Up @@ -8,30 +8,6 @@ void yp_node_clear(yp_node_t *node) {
node->location = location;
}

// Calculate the size of the token list in bytes.
static size_t
yp_location_list_memsize(yp_location_list_t *list) {
return sizeof(yp_location_list_t) + (list->capacity * sizeof(yp_location_t));
}

// Append a token to the given list.
void
yp_location_list_append(yp_location_list_t *list, const yp_token_t *token) {
if (list->size == list->capacity) {
list->capacity = list->capacity == 0 ? 2 : list->capacity * 2;
list->locations = (yp_location_t *) realloc(list->locations, sizeof(yp_location_t) * list->capacity);
}
list->locations[list->size++] = (yp_location_t) { .start = token->start, .end = token->end };
}

// Free the memory associated with the token list.
static void
yp_location_list_free(yp_location_list_t *list) {
if (list->locations != NULL) {
free(list->locations);
}
}

static void
yp_node_memsize_node(yp_node_t *node, yp_memsize_t *memsize);

Expand Down Expand Up @@ -95,8 +71,6 @@ yp_node_destroy(yp_parser_t *parser, yp_node_t *node) {
yp_string_free(&cast-><%= field.name %>);
<%- when YARP::NodeListField -%>
yp_node_list_free(parser, &cast-><%= field.name %>);
<%- when YARP::LocationListField -%>
yp_location_list_free(&cast-><%= field.name %>);
<%- when YARP::ConstantListField -%>
yp_constant_id_list_free(&cast-><%= field.name %>);
<%- else -%>
Expand Down Expand Up @@ -141,8 +115,6 @@ yp_node_memsize_node(yp_node_t *node, yp_memsize_t *memsize) {
memsize->memsize += yp_string_memsize(&cast-><%= field.name %>);
<%- when YARP::NodeListField -%>
yp_node_list_memsize(&cast-><%= field.name %>, memsize);
<%- when YARP::LocationListField -%>
memsize->memsize += yp_location_list_memsize(&cast-><%= field.name %>);
<%- when YARP::ConstantListField -%>
memsize->memsize += yp_constant_id_list_memsize(&cast-><%= field.name %>);
<%- else -%>
Expand Down
7 changes: 0 additions & 7 deletions yarp/templates/src/prettyprint.c.erb
Expand Up @@ -45,13 +45,6 @@ prettyprint_node(yp_buffer_t *buffer, yp_parser_t *parser, yp_node_t *node) {
prettyprint_node(buffer, parser, (yp_node_t *) ((yp_<%= node.human %>_t *) node)-><%= field.name %>.nodes[index]);
}
yp_buffer_append_str(buffer, "]", 1);
<%- when YARP::LocationListField -%>
yp_buffer_append_str(buffer, "[", 1);
for (uint32_t index = 0; index < ((yp_<%= node.human %>_t *)node)-><%= field.name %>.size; index++) {
if (index != 0) yp_buffer_append_str(buffer, ", ", 2);
prettyprint_location(buffer, parser, &((yp_<%= node.human %>_t *)node)-><%= field.name %>.locations[index]);
}
yp_buffer_append_str(buffer, "]", 1);
<%- when YARP::ConstantField -%>
char <%= field.name %>_buffer[12];
snprintf(<%= field.name %>_buffer, sizeof(<%= field.name %>_buffer), "%u", ((yp_<%= node.human %>_t *)node)-><%= field.name %>);
Expand Down
6 changes: 0 additions & 6 deletions yarp/templates/src/serialize.c.erb
Expand Up @@ -86,12 +86,6 @@ yp_serialize_node(yp_parser_t *parser, yp_node_t *node, yp_buffer_t *buffer) {
for (uint32_t index = 0; index < <%= field.name %>_size; index++) {
yp_serialize_node(parser, (yp_node_t *) ((yp_<%= node.human %>_t *)node)-><%= field.name %>.nodes[index], buffer);
}
<%- when YARP::LocationListField -%>
uint32_t <%= field.name %>_size = yp_sizet_to_u32(((yp_<%= node.human %>_t *)node)-><%= field.name %>.size);
yp_buffer_append_u32(buffer, <%= field.name %>_size);
for (uint32_t index = 0; index < <%= field.name %>_size; index++) {
yp_serialize_location(parser, &((yp_<%= node.human %>_t *)node)-><%= field.name %>.locations[index], buffer);
}
<%- when YARP::ConstantField -%>
yp_buffer_append_u32(buffer, yp_sizet_to_u32(((yp_<%= node.human %>_t *)node)-><%= field.name %>));
<%- when YARP::ConstantListField -%>
Expand Down
12 changes: 0 additions & 12 deletions yarp/templates/template.rb
Expand Up @@ -73,17 +73,6 @@ def java_type
end
end

# This represents a field on a node that is a list of locations.
class LocationListField < Field
def rbs_class
"Array[Location]"
end

def java_type
"Location[]"
end
end

# This represents a field on a node that is the ID of a string interned
# through the parser's constant pool.
class ConstantField < Field
Expand Down Expand Up @@ -205,7 +194,6 @@ def field_type_for(name)
when "node?" then OptionalNodeField
when "node[]" then NodeListField
when "string" then StringField
when "location[]" then LocationListField
when "constant" then ConstantField
when "constant[]" then ConstantListField
when "location" then LocationField
Expand Down

0 comments on commit c384ef0

Please sign in to comment.