From 8bb5d7dc4f7ab81c883d6a5fb20fad281ccfc128 Mon Sep 17 00:00:00 2001 From: ksss Date: Sat, 17 Oct 2026 19:02:53 +0900 Subject: [PATCH] Reconstruct Comment#string lazily from its location The parser used to materialize the text of every comment as a Ruby String, which duplicated the comment bytes that the Buffer already holds. Parsing core/ + stdlib/ allocated 4.1MB of such strings, about 20% of the whole AST, while consumers rarely need all of them at once: Environment and DefinitionBuilder only carry them through, and Steep reads a handful on demand (hover, completion). `RBS::AST::Comment#string` is now computed on first access from `location.source`, mirroring `parse_comment_lines` in src/parser.c (drop indentation, `#`, one following space; CRLF's CR is not part of the token, a bare CR is kept). `Comment.new(string:)` keeps working for callers that build comments by hand. To stop the C translation from building the string, config.yml gains a per-field `expose_to_ruby: false` option: the field stays in the C struct and in the serialized form, but is not passed to the Ruby constructor. The WASM deserializer honors the same flag by reading and discarding the field, so both construction paths produce identical objects. Measured on core/ + stdlib/ (252 files): memsize 20.76MB -> 16.62MB, process RSS delta 45.0MB -> 40.7MB. Co-Authored-By: Claude Fable 5.1 Co-Authored-By: Claude Opus 5.5 --- config.yml | 1 + docs/config.md | 11 +++++++ ext/rbs_extension/ast_translation.c | 2 -- lib/rbs/ast/comment.rb | 19 ++++++++++-- lib/rbs/wasm/deserializer.rb | 3 +- lib/rbs/wasm/serialization_schema.rb | 5 +-- sig/comment.rbs | 15 +++++++-- .../ext/rbs_extension/ast_translation.c.erb | 2 ++ .../lib/rbs/wasm/serialization_schema.rb.erb | 7 +++-- templates/template.rb | 11 +++++-- test/rbs/comment_test.rb | 28 +++++++++++++++++ test/rbs/signature_parsing_test.rb | 31 +++++++++++++++++++ 12 files changed, 122 insertions(+), 13 deletions(-) create mode 100644 test/rbs/comment_test.rb diff --git a/config.yml b/config.yml index 3abd4a2c8a..1cb7aefd2a 100644 --- a/config.yml +++ b/config.yml @@ -18,6 +18,7 @@ nodes: fields: - name: string c_type: rbs_string + expose_to_ruby: false # `Comment#string` is reconstructed lazily from the location in Ruby - name: RBS::AST::Declarations::Class rust_name: ClassNode fields: diff --git a/docs/config.md b/docs/config.md index 0e7bf384c7..947be1a527 100644 --- a/docs/config.md +++ b/docs/config.md @@ -108,6 +108,17 @@ The `rbs_ast_declarations_class` struct is a pure C AST, and `ast_translation.c` } ``` +A field can be excluded from the Ruby translation with `expose_to_ruby: false`. The field is still part of the C struct (and the Rust/serialization side), but `ast_translation.c` does not pass it to the Ruby constructor, so the Ruby class has to compute it by itself. `RBS::AST::Comment#string` uses this to reconstruct the comment text lazily from its `location` instead of holding a copy of it. + +```yaml + - name: RBS::AST::Comment + rust_name: CommentNode + fields: + - name: string + c_type: rbs_string + expose_to_ruby: false +``` + ## enums `enums` defines *enum* data types in C or Rust. diff --git a/ext/rbs_extension/ast_translation.c b/ext/rbs_extension/ast_translation.c index 8926e50a7a..8717d61bc7 100644 --- a/ext/rbs_extension/ast_translation.c +++ b/ext/rbs_extension/ast_translation.c @@ -232,14 +232,12 @@ VALUE rbs_struct_to_ruby_value(rbs_translation_context_t ctx, rbs_node_t *instan // Compute child VALUEs into locals variables first, before any recursion into `rbs_struct_to_ruby_value()`. VALUE arg_location = rbs_location_range_to_ruby_location(ctx, node->base.location); - VALUE arg_string = rbs_string_to_ruby_string(&node->string, ctx.encoding); // Claim the shared kwargs hash, clear it, fill it, and hand it to `.new`. // Must not recurse between `rb_hash_clear()` and `CLASS_NEW_INSTANCE()`. VALUE h = ctx.reusable_kwargs_hash; rb_hash_clear(h); rb_hash_aset(h, ID2SYM(rb_intern("location")), arg_location); - rb_hash_aset(h, ID2SYM(rb_intern("string")), arg_string); return CLASS_NEW_INSTANCE(RBS_AST_Comment, 1, &h); } case RBS_AST_DECLARATIONS_CLASS: { diff --git a/lib/rbs/ast/comment.rb b/lib/rbs/ast/comment.rb index 04cf6b6ec6..24f653a4e5 100644 --- a/lib/rbs/ast/comment.rb +++ b/lib/rbs/ast/comment.rb @@ -3,14 +3,17 @@ module RBS module AST class Comment - attr_reader :string attr_reader :location - def initialize(string:, location:) + def initialize(string: nil, location:) @string = string @location = location end + def string + @string ||= string_from_location + end + def ==(other) other.is_a?(Comment) && other.string == string end @@ -24,6 +27,18 @@ def hash def to_json(state = nil) { string: string, location: location }.to_json(state) end + + private + + def string_from_location + loc = location or raise "Comment#string requires a location when no string is given" + + source = loc.buffer.top_buffer.content[loc.start_pos...loc.end_pos] or raise + + source.each_line.map do |line| + line.lstrip.delete_prefix("#").delete_prefix(" ").sub(/\r?\n\z/, "") + "\n" + end.join + end end end end diff --git a/lib/rbs/wasm/deserializer.rb b/lib/rbs/wasm/deserializer.rb index 29e2fd53be..862c3079dd 100644 --- a/lib/rbs/wasm/deserializer.rb +++ b/lib/rbs/wasm/deserializer.rb @@ -88,7 +88,8 @@ def read_struct(entry) kwargs = {} #: Hash[Symbol, untyped] (fields || []).each do |name, reader| - kwargs[name] = read_field(reader) + value = read_field(reader) + kwargs[name] = value if name end RBS::AST::TypeParam.resolve_variables(kwargs[:type_params]) if resolve_type_params diff --git a/lib/rbs/wasm/serialization_schema.rb b/lib/rbs/wasm/serialization_schema.rb index 3b7e24729e..bfd4c69e7a 100644 --- a/lib/rbs/wasm/serialization_schema.rb +++ b/lib/rbs/wasm/serialization_schema.rb @@ -21,7 +21,8 @@ module WASM # where loc_children is [[name, required?], ...] and fields is # [[name, reader], ...] with reader one of :node, :node_list, :hash, :string, # :bool, :location_range, :location_range_list, :attr_ivar_name, or - # [:enum, [value_or_nil, ...]]. + # [:enum, [value_or_nil, ...]]. A `nil` name means the field is read but not + # passed to the constructor (`expose_to_ruby: false` in config.yml). module SerializationSchema SYMBOL_TAG = 79 @@ -29,7 +30,7 @@ module SerializationSchema nil, # tag 0 is reserved for NULL [:node, "RBS::AST::Annotation", true, nil, [[:string, :string]], false], [:bool], - [:node, "RBS::AST::Comment", true, nil, [[:string, :string]], false], + [:node, "RBS::AST::Comment", true, nil, [[nil, :string]], false], [:node, "RBS::AST::Declarations::Class", true, [[:keyword, true], [:name, true], [:end, true], [:type_params, false], [:lt, false]], [[:name, :node], [:type_params, :node_list], [:super_class, :node], [:members, :node_list], [:annotations, :node_list], [:comment, :node]], true], [:node, "RBS::AST::Declarations::Class::Super", true, [[:name, true], [:args, false]], [[:name, :node], [:args, :node_list]], false], [:node, "RBS::AST::Declarations::ClassAlias", true, [[:keyword, true], [:new_name, true], [:eq, true], [:old_name, true]], [[:new_name, :node], [:old_name, :node], [:comment, :node], [:annotations, :node_list]], false], diff --git a/sig/comment.rbs b/sig/comment.rbs index 5e0ffd3792..f9415fbe32 100644 --- a/sig/comment.rbs +++ b/sig/comment.rbs @@ -9,10 +9,15 @@ module RBS # ``` # class Comment - attr_reader string: String + # Each line with its leading `#` and one following space removed. + # Computed from `location` on first access unless given to `initialize`. + def string: () -> String + + @string: String? + attr_reader location: Location[bot, bot]? - def initialize: (string: String, location: Location[bot, bot]?) -> void + def initialize: (?string: String?, location: Location[bot, bot]?) -> void def ==: (untyped other) -> bool @@ -21,6 +26,12 @@ module RBS def hash: () -> Integer include _ToJson + + private + + # Must stay in sync with `parse_comment_lines` in src/parser.c. + # Slices the buffer instead of using `Location#source` so no copy is memoized on the Location. + def string_from_location: () -> String end end end diff --git a/templates/ext/rbs_extension/ast_translation.c.erb b/templates/ext/rbs_extension/ast_translation.c.erb index 43db45c6b9..173af846b4 100644 --- a/templates/ext/rbs_extension/ast_translation.c.erb +++ b/templates/ext/rbs_extension/ast_translation.c.erb @@ -212,6 +212,7 @@ VALUE rbs_struct_to_ruby_value(rbs_translation_context_t ctx, rbs_node_t *instan <%- end -%> <%- end -%> <%- node.fields.each do |field| -%> + <%- next unless field.expose_to_ruby? -%> <%- case field.type.name -%> <%- when "rbs_node_list" -%> VALUE arg_<%= field.name %> = rbs_node_list_to_ruby_array(ctx, node-><%= field.c_name %>); @@ -262,6 +263,7 @@ VALUE rbs_struct_to_ruby_value(rbs_translation_context_t ctx, rbs_node_t *instan rb_hash_aset(h, ID2SYM(rb_intern("location")), arg_location); <%- end -%> <%- node.fields.each do |field| -%> + <%- next unless field.expose_to_ruby? -%> rb_hash_aset(h, ID2SYM(rb_intern("<%= field.name %>")), arg_<%= field.name %>); <%- end -%> return CLASS_NEW_INSTANCE(<%= node.c_constant_name %>, 1, &h); diff --git a/templates/lib/rbs/wasm/serialization_schema.rb.erb b/templates/lib/rbs/wasm/serialization_schema.rb.erb index df96adcd7f..4053c18aa6 100644 --- a/templates/lib/rbs/wasm/serialization_schema.rb.erb +++ b/templates/lib/rbs/wasm/serialization_schema.rb.erb @@ -41,7 +41,9 @@ when "RBS::TypeName" then "[:type_name]" else children = (node.locations || []).map { |loc| "[:#{loc.name}, #{loc.required?}]" } - fields = node.fields.map { |field| "[:#{field.name}, #{reader_for.call(field)}]" } + # A field with `expose_to_ruby: false` is still in the buffer, so the deserializer + # must read it, but it is not passed to the Ruby constructor: its name is `nil`. + fields = node.fields.map { |field| "[#{field.expose_to_ruby? ? ":#{field.name}" : "nil"}, #{reader_for.call(field)}]" } # Emit `nil` rather than `[]` for empty lists, so the generated file has no # unannotated empty collections for Steep to flag. children_literal = children.empty? ? "nil" : "[#{children.join(", ")}]" @@ -67,7 +69,8 @@ module RBS # where loc_children is [[name, required?], ...] and fields is # [[name, reader], ...] with reader one of :node, :node_list, :hash, :string, # :bool, :location_range, :location_range_list, :attr_ivar_name, or - # [:enum, [value_or_nil, ...]]. + # [:enum, [value_or_nil, ...]]. A `nil` name means the field is read but not + # passed to the constructor (`expose_to_ruby: false` in config.yml). module SerializationSchema SYMBOL_TAG = <%= nodes.size + 1 %> diff --git a/templates/template.rb b/templates/template.rb index c97052872f..3772d16131 100644 --- a/templates/template.rb +++ b/templates/template.rb @@ -75,11 +75,12 @@ def c_type_name class Field attr_reader :name, :type, :c_name #: String - def initialize(name:, type:, optional:, c_name: nil) + def initialize(name:, type:, optional:, c_name: nil, expose_to_ruby: true) @name = name @type = type @c_name = c_name || name @optional = optional + @expose_to_ruby = expose_to_ruby end def self.from_hash(hash) @@ -87,10 +88,15 @@ def self.from_hash(hash) name: hash["name"], type: hash.fetch("c_type", "VALUE"), c_name: hash["c_name"], - optional: hash.fetch("optional", false) + optional: hash.fetch("optional", false), + expose_to_ruby: hash.fetch("expose_to_ruby", true) ) end + def expose_to_ruby? #: bool + @expose_to_ruby + end + def optional? #: bool @optional end @@ -376,6 +382,7 @@ def locals type: type, optional: field.fetch("optional", false), c_name: field["c_name"], + expose_to_ruby: field.fetch("expose_to_ruby", true), ) end diff --git a/test/rbs/comment_test.rb b/test/rbs/comment_test.rb new file mode 100644 index 0000000000..78797e0ad1 --- /dev/null +++ b/test/rbs/comment_test.rb @@ -0,0 +1,28 @@ +require "test_helper" + +class RBS::CommentTest < Test::Unit::TestCase + include RBS + + def test_new_with_string + comment = AST::Comment.new(string: "Hello\n", location: nil) + + assert_equal "Hello\n", comment.string + assert_equal AST::Comment.new(string: "Hello\n", location: nil), comment + end + + def test_new_without_string_requires_location + assert_raises(RuntimeError) do + AST::Comment.new(location: nil).string + end + end + + def test_string_from_location + buffer = Buffer.new(name: Pathname("a.rbs"), content: " # foo\n #bar\n #\nclass Foo end\n") + location = Location.new(buffer, 2, 17) + + comment = AST::Comment.new(location: location) + + assert_equal "foo\nbar\n\n", comment.string + assert_equal AST::Comment.new(string: "foo\nbar\n\n", location: nil), comment + end +end diff --git a/test/rbs/signature_parsing_test.rb b/test/rbs/signature_parsing_test.rb index bcf666f5a6..d86e75ee46 100644 --- a/test/rbs/signature_parsing_test.rb +++ b/test/rbs/signature_parsing_test.rb @@ -1172,6 +1172,37 @@ def bar: () -> void end end + def test_comment_with_crlf + # The CR of a CRLF line ending is not part of the comment, but a bare CR inside a line is. + Parser.parse_signature("# foo\r\n # bar\r\n#baz\r\nclass Foo\r\nend\r\n").yield_self do |_, _, (foo_decl)| + assert_equal "foo\nbar\nbaz\n", foo_decl.comment.string + end + + Parser.parse_signature("# a\rb\nclass Foo end").yield_self do |_, _, (foo_decl)| + assert_equal "a\rb\n", foo_decl.comment.string + end + end + + def test_comment_string_is_reconstructed_from_location + Parser.parse_signature(<<-EOF).yield_self do |_, _, (foo_decl)| +# Doc for Foo. +class Foo + # + # indented, blank, and tab lines + #\tkeep the tab + def bar: () -> void +end +EOF + + comment = foo_decl.comment + assert_nil comment.instance_variable_get(:@string) + assert_equal "Doc for Foo.\n", comment.string + assert_same comment.string, comment.string + + assert_equal "\n indented, blank, and tab lines\n\tkeep the tab\n", foo_decl.members[0].comment.string + end + end + def test_parsing_quoted_method_name Parser.parse_signature(<<-EOF).yield_self do |foo_decl,| module Kernel