From 411cfc438d0ac6c3ec345b74dfd02cfa94092eb7 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 5 Aug 2026 17:50:08 -0700 Subject: [PATCH 01/30] Do not resolve dependency names against the build directory (#18212) tool/mkdepend.rb resolved dependency names against the current working directory before falling back to the source root. When configure re-runs in an already-built build directory nested inside the source tree (e.g. .ruby under the checkout), leftover generated files such as builtin_binary.rbbin got emitted as build-dir-relative paths: builtin.$(OBJEXT): .ruby/builtin_binary.rbbin Make satisfies such a prerequisite through VPATH as a plain existing file, so it never matches the builtin_binary.rbbin rule and the file is never regenerated. Linking a stale builtin_binary.rbbin with up-to-date *.rbinc function tables then makes the built ruby fail to boot during make install: : builtin function index (8) mismatch (expect _bi454 but _bi464) (ArgumentError) The same defect also emitted .ruby/probes.h and .ruby/vm_call_iseq_optimized.inc, and dropped the enc/trans/*.trans dependencies of the generated transcoder sources. Convert dependency names against the source root only, never the current working directory, so that generated files keep the bare names their Make rules use and the generated dependencies no longer vary with leftover build artifacts. --- lib/mkmf/depend.rb | 23 ++++++++++++++++++++--- tool/test/test_mkdepend.rb | 26 ++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 3 deletions(-) diff --git a/lib/mkmf/depend.rb b/lib/mkmf/depend.rb index 55719e4511518d..d6c2bd814cc92e 100644 --- a/lib/mkmf/depend.rb +++ b/lib/mkmf/depend.rb @@ -550,6 +550,23 @@ def relative_source(path) expanded.start_with?(prefix) ? expanded.delete_prefix(prefix) : path end + # Makes +path+ relative to #root without consulting the current + # directory. Unlike #relative_source, a relative name that does not + # refer to a source-tree file is kept as-is: generated dependencies + # such as builtin_binary.rbbin live in the build directory, and must + # keep the name their Make rules use even when the tool runs in a + # build directory nested inside the source tree. + def relative_dependency(path) + expanded = File.expand_path(path, @root) + prefix = @root + File::SEPARATOR + if expanded.start_with?(prefix) && + (File.absolute_path?(path) || File.exist?(expanded)) + expanded.delete_prefix(prefix) + else + path + end + end + # Converts an extension dependency to the Make variable path it requires. def extension_dependency(file, source_dir) case file @@ -607,7 +624,7 @@ def depends(files, vpath, source: nil, input: nil, declarations: nil, end files = files.flat_map {|file| expand.call(file, [])} files.each_with_object([]) do |file, deps| - file = relative_source(file) + file = relative_dependency(file) dep = if file.start_with?('$(', '{$(') file elsif target = dependency_target(file, declaration_input) @@ -696,7 +713,7 @@ def dependency_scanner(src, declarations, input) # Appends Make dependency rules for +src+ to +out+ and returns +out+. def makedepend(src, out = [], target: nil, input: nil, project: false) - src = relative_source(src) + src = relative_dependency(src) declaration_input = input || dependency_input(src) declarations = dependency_declarations(declaration_input, source: src) vpath = dependency_vpath(input, src) @@ -1001,7 +1018,7 @@ def run(inputs = ARGV, out: $stdout, err: $stderr, mode: :stdout, changed = false inputs.each do |input| if input.end_with?(".c", ".y") - out.puts makedepend(input) + out.puts makedepend(relative_source(input)) else deps = dependency_file_content(input) || File.read(input) dependency_declarations(input, content: deps) diff --git a/tool/test/test_mkdepend.rb b/tool/test/test_mkdepend.rb index cf3cb71e57ff7c..7cb48630259c2b 100644 --- a/tool/test/test_mkdepend.rb +++ b/tool/test/test_mkdepend.rb @@ -973,6 +973,32 @@ def test_run_removes_vpath_notation_from_build_output end end + def test_run_from_build_directory_keeps_generated_dependency_names + Dir.mktmpdir('mkdepend-builddir') do |dir| + File.write(File.join(dir, 'builtin.c'), <<~SOURCE) + #include "builtin_binary.rbbin" + SOURCE + input = File.join(dir, 'depend') + File.write(input, <<~DEPEND) + #{MARK_START} + builtin.$(OBJEXT): {$(VPATH)}builtin.c + #{MARK_END} + DEPEND + build = File.join(dir, '.build') + FileUtils.mkdir_p(build) + File.write(File.join(build, 'builtin_binary.rbbin'), '') + output = File.join(build, '.deps') + + mkdepend = TestDepend.new(root: dir) + Dir.chdir(build) do + assert_true(mkdepend.run([input], mode: :output, output: output)) + end + generated = File.read(File.join(output, 'depend')) + assert_include(generated, "builtin.$(OBJEXT): builtin_binary.rbbin\n") + assert_not_include(generated, '.build/builtin_binary.rbbin') + end + end + def test_normalize_dependency_rules_removes_vpath_search assert_equal( "one.h two.h\n", From 1db6f0feee55a9ef1b50dd0419268113aafd11d0 Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Wed, 5 Aug 2026 20:20:48 -0400 Subject: [PATCH 02/30] [ruby/prism] Split up newline token There are actually 3 tokens in the Ruby grammar: the newline in a whitespace insensitive position that is ignored, a newline that is in a whitespace sensitive position that acts as a statement terminator, and a newline in a whitespace sensisitive position that acts as the terminator for an expression. The third one doesn't exist in our grammar at the moment, but I want to add it, because it makes working with translating the lex output easier. https://github.com/ruby/prism/commit/3dbe592a76 --- lib/prism/lex_compat.rb | 26 ++++++++++++++++- lib/prism/translation/parser/lexer.rb | 41 ++++++++++----------------- prism/config.yml | 2 ++ prism/prism.c | 30 +++++++++++++++----- prism/templates/src/tokens.c.erb | 2 ++ test/prism/ruby/parser_test.rb | 15 +--------- 6 files changed, 68 insertions(+), 48 deletions(-) diff --git a/lib/prism/lex_compat.rb b/lib/prism/lex_compat.rb index 749f11173a42aa..a2ad69cd2982a6 100644 --- a/lib/prism/lex_compat.rb +++ b/lib/prism/lex_compat.rb @@ -191,6 +191,7 @@ def deconstruct_keys(keys) # :nodoc: MINUS_EQUAL: :on_op, MINUS_GREATER: :on_tlambda, NEWLINE: :on_nl, + NEWLINE_TERMINATOR: :on_ignored_nl, NUMBERED_REFERENCE: :on_backref, PARENTHESIS_LEFT: :on_lparen, PARENTHESIS_LEFT_GROUPING: :on_lparen, @@ -617,6 +618,9 @@ def result bom = source.slice(0, 3) == "\xEF\xBB\xBF" + last_comment_token = nil #: lex_compat_token? + last_comment_end = nil #: Integer? + result_value.each_with_index do |(prism_token, prism_state), index| lineno = prism_token.location.start_line column = prism_token.location.start_column @@ -625,6 +629,16 @@ def result value = prism_token.value lex_state = Translation::Ripper::Lexer::State[prism_state] + # A comment token does not include its terminating newline, but + # ripper's comment value does, so the newline token that directly + # follows a comment is folded back into it. + if last_comment_token && last_comment_end == prism_token.location.start_offset && (event == :on_nl || event == :on_ignored_nl) + last_comment_token[2] += value + last_comment_token = nil + last_comment_end = nil + next + end + # If there's a UTF-8 byte-order mark as the start of the file, then for # certain tokens ripper sets the first token back by 3 bytes. It also # keeps the byte order mark in the first token's value. This is weird, @@ -714,11 +728,16 @@ def result eof_token = prism_token previous_token = result_value[index - 1][0] + # A newline that was folded back into a comment still marks the + # comment boundary for the check below. + comment_boundary = previous_token.type == :COMMENT || + (index >= 2 && %i[NEWLINE NEWLINE_TERMINATOR IGNORED_NEWLINE].include?(previous_token.type) && result_value[index - 2][0].type == :COMMENT && result_value[index - 2][0].location.end_offset == previous_token.location.start_offset) + # If we're at the end of the file and the previous token was a # comment and there is still whitespace after the comment, then # Ripper will append a on_nl token (even though there isn't # necessarily a newline). We mirror that here. - if previous_token.type == :COMMENT + if comment_boundary # If the comment is at the start of a heredoc: <= 0 - next_token, _ = lexed[index] - - is_inline_comment = prev_token&.location&.start_line == token.location.start_line - if is_inline_comment && !is_at_eol && !COMMENT_CONTINUATION_TYPES.include?(next_token&.type) - tokens << [:tCOMMENT, [value, location]] - - nl_location = range(token.location.end_offset - 1, token.location.end_offset) - tokens << [:tNL, [nil, nl_location]] - next - elsif is_inline_comment && next_token&.type == :COMMENT - comment_newline_location = range(token.location.end_offset - 1, token.location.end_offset) - elsif comment_newline_location && !COMMENT_CONTINUATION_TYPES.include?(next_token&.type) - tokens << [:tCOMMENT, [value, location]] - tokens << [:tNL, [nil, comment_newline_location]] - comment_newline_location = nil - next - end + # A carriage return before the terminating newline is part of + # the comment token but not of the comment's value. + location = range(token.location.start_offset, token.location.end_offset - 1) if value.chomp! end when :tNL next_token, _ = lexed[index] @@ -501,6 +486,10 @@ def to_a end end + if comment_newline_location + tokens << [:tNL, [nil, comment_newline_location]] + end + tokens end diff --git a/prism/config.yml b/prism/config.yml index cc5eb7e099c228..4892089c031ab8 100644 --- a/prism/config.yml +++ b/prism/config.yml @@ -359,6 +359,8 @@ tokens: comment: "when" - name: NEWLINE comment: "a newline character outside of other tokens" + - name: NEWLINE_TERMINATOR + comment: "a newline that terminates a construct where a newline is otherwise insignificant" - name: PARENTHESIS_RIGHT comment: ")" - name: PIPE diff --git a/prism/prism.c b/prism/prism.c index bd16a3f2822db4..51980155e4a0f8 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -10228,7 +10228,6 @@ parser_lex(pm_parser_t *parser) { pm_comment_t *comment = parser_comment(parser, PM_COMMENT_INLINE); pm_list_append(&parser->comment_list, (pm_list_node_t *) comment); - if (ending) parser->current.end++; parser->current.type = PM_TOKEN_COMMENT; parser_lex_callback(parser); @@ -10246,7 +10245,16 @@ parser_lex(pm_parser_t *parser) { } } - lexed_comment = true; + /* The comment does not include its terminating newline, + * which lexes through the newline handling below as its + * own token. A comment that ends the file has no newline, + * so the newline handling runs without one to emit. */ + if (ending == NULL) { + lexed_comment = true; + } else { + parser->current.start = ending; + parser->current.end = ending + 1; + } } PRISM_FALLTHROUGH case '\r': @@ -10284,7 +10292,11 @@ parser_lex(pm_parser_t *parser) { break; case PM_IGNORED_NEWLINE_PATTERN: if (parser->pattern_matching_newlines || parser->in_keyword_arg) { - if (!lexed_comment) parser_lex_ignored_newline(parser); + if (!lexed_comment) { + parser->current.type = PM_TOKEN_NEWLINE_TERMINATOR; + parser_lex_callback(parser); + } + lex_state_set(parser, PM_LEX_STATE_BEG); parser->command_start = true; parser->current.type = PM_TOKEN_NEWLINE; @@ -10381,11 +10393,15 @@ parser_lex(pm_parser_t *parser) { // If we hit a . after a newline, then we're in a call chain and // we need to return the call operator. if (next_content[0] == '.') { - // To match ripper, we need to emit an ignored newline even though - // it's a real newline in the case that we have a beginless range - // on a subsequent line. + /* A beginless range on the next line means this + * newline terminates the statement rather than + * continuing a method chain. */ if (peek_at(parser, next_content + 1) == '.') { - if (!lexed_comment) parser_lex_ignored_newline(parser); + if (!lexed_comment) { + parser->current.type = PM_TOKEN_NEWLINE_TERMINATOR; + parser_lex_callback(parser); + } + lex_state_set(parser, PM_LEX_STATE_BEG); parser->command_start = true; parser->current.type = PM_TOKEN_NEWLINE; diff --git a/prism/templates/src/tokens.c.erb b/prism/templates/src/tokens.c.erb index fb71afe217f687..6e88d423c2dd74 100644 --- a/prism/templates/src/tokens.c.erb +++ b/prism/templates/src/tokens.c.erb @@ -275,6 +275,8 @@ pm_token_str(pm_token_type_t token_type) { return "'->'"; case PM_TOKEN_NEWLINE: return "newline"; + case PM_TOKEN_NEWLINE_TERMINATOR: + return "newline"; case PM_TOKEN_NUMBERED_REFERENCE: return "numbered reference"; case PM_TOKEN_PARENTHESIS_LEFT: diff --git a/test/prism/ruby/parser_test.rb b/test/prism/ruby/parser_test.rb index e44bc20d4dea0b..076f84765cbc1b 100644 --- a/test/prism/ruby/parser_test.rb +++ b/test/prism/ruby/parser_test.rb @@ -109,26 +109,13 @@ class ParserTest < TestCase # These files are failing to translate their lexer output into the lexer # output expected by the parser gem, so we'll skip them for now. skip_tokens = [ - "dash_heredocs.txt", "embdoc_no_newline_at_end.txt", - "seattlerb/case_in.txt", - "seattlerb/difficult4__leading_dots2.txt", "seattlerb/heredoc_unicode.txt", "seattlerb/parse_line_heredoc.txt", "seattlerb/pct_w_heredoc_interp_nested.txt", - "seattlerb/required_kwarg_no_value.txt", - "seattlerb/TestRubyParserShared.txt", "unparser/corpus/literal/assignment.txt", "unparser/corpus/literal/literal.txt", - "whitequark/args.txt", - "whitequark/beginless_erange_after_newline.txt", - "whitequark/beginless_irange_after_newline.txt", - "whitequark/forward_arg_with_open_args.txt", - "whitequark/kwarg_no_paren.txt", - "whitequark/multiple_pattern_matches.txt", - "whitequark/newline_in_hash_argument.txt", - "whitequark/pattern_matching_hash.txt", - "whitequark/ruby_bug_9669.txt" + "whitequark/forward_arg_with_open_args.txt" ] Fixture.each_for_version(except: skip_syntax_error, version: "3.3") do |fixture| From 6b93405b2668436620d3a0fc8ad545d9635a3ddb Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Thu, 6 Aug 2026 10:08:35 +0900 Subject: [PATCH 03/30] [ruby/strscan] Return nil for empty integer captures JRuby must match the native implementation when captures become empty. https://github.com/ruby/strscan/commit/535f9b43ba --- ext/strscan/lib/strscan/strscan.rb | 3 ++- test/strscan/test_stringscanner.rb | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/ext/strscan/lib/strscan/strscan.rb b/ext/strscan/lib/strscan/strscan.rb index 5e262f4007b497..8836eea1d15fc1 100644 --- a/ext/strscan/lib/strscan/strscan.rb +++ b/ext/strscan/lib/strscan/strscan.rb @@ -3,7 +3,8 @@ class StringScanner unless method_defined?(:integer_at) # For JRuby def integer_at(specifier, *to_i_args) - self[specifier]&.to_i(*to_i_args) + value = self[specifier] + value.to_i(*to_i_args) unless value.nil? || value.empty? end end diff --git a/test/strscan/test_stringscanner.rb b/test/strscan/test_stringscanner.rb index 966d62b22689b8..df5aa089ff5c92 100644 --- a/test/strscan/test_stringscanner.rb +++ b/test/strscan/test_stringscanner.rb @@ -578,6 +578,13 @@ def test_integer_at_base_auto assert_integer_at(s, 0, 0) # 0xaf end + def test_integer_at_empty + s = create_string_scanner("") + assert_equal("", s.scan(/()/)) + assert_nil(s.integer_at(0)) + assert_nil(s.integer_at(1)) + end + def test_integer_at_shrunk omit("not supported on TruffleRuby") if RUBY_ENGINE == "truffleruby" From 6251e06a029938c99da48ea4f843ca79ca0f2f81 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Thu, 6 Aug 2026 10:20:28 +0900 Subject: [PATCH 04/30] [ruby/strscan] Handle shrunk captures on TruffleRuby `integer_at` must read captures from the scanner current string. https://github.com/ruby/strscan/commit/435095d898 --- test/strscan/test_stringscanner.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/strscan/test_stringscanner.rb b/test/strscan/test_stringscanner.rb index df5aa089ff5c92..47b1e2a1558be2 100644 --- a/test/strscan/test_stringscanner.rb +++ b/test/strscan/test_stringscanner.rb @@ -586,8 +586,6 @@ def test_integer_at_empty end def test_integer_at_shrunk - omit("not supported on TruffleRuby") if RUBY_ENGINE == "truffleruby" - s = create_string_scanner(+"before 29 after") s.skip_until(" ") assert_equal("29", s.scan(/\d+/)) @@ -596,8 +594,6 @@ def test_integer_at_shrunk end def test_integer_at_shrunk_partial - omit("not supported on TruffleRuby") if RUBY_ENGINE == "truffleruby" - s = create_string_scanner(+"before 29 after") s.skip_until(" ") assert_equal("29", s.scan(/\d+/)) From 918fed9de47b02ed00e4edb35074c2307ed7c07a Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Wed, 5 Aug 2026 21:51:02 -0400 Subject: [PATCH 05/30] [ruby/prism] Revert "Split up newline token" This reverts commit https://github.com/ruby/prism/commit/3dbe592a76cb. https://github.com/ruby/prism/commit/115d58f71b --- lib/prism/lex_compat.rb | 26 +---------------- lib/prism/translation/parser/lexer.rb | 41 +++++++++++++++++---------- prism/config.yml | 2 -- prism/prism.c | 30 +++++--------------- prism/templates/src/tokens.c.erb | 2 -- test/prism/ruby/parser_test.rb | 15 +++++++++- 6 files changed, 48 insertions(+), 68 deletions(-) diff --git a/lib/prism/lex_compat.rb b/lib/prism/lex_compat.rb index a2ad69cd2982a6..749f11173a42aa 100644 --- a/lib/prism/lex_compat.rb +++ b/lib/prism/lex_compat.rb @@ -191,7 +191,6 @@ def deconstruct_keys(keys) # :nodoc: MINUS_EQUAL: :on_op, MINUS_GREATER: :on_tlambda, NEWLINE: :on_nl, - NEWLINE_TERMINATOR: :on_ignored_nl, NUMBERED_REFERENCE: :on_backref, PARENTHESIS_LEFT: :on_lparen, PARENTHESIS_LEFT_GROUPING: :on_lparen, @@ -618,9 +617,6 @@ def result bom = source.slice(0, 3) == "\xEF\xBB\xBF" - last_comment_token = nil #: lex_compat_token? - last_comment_end = nil #: Integer? - result_value.each_with_index do |(prism_token, prism_state), index| lineno = prism_token.location.start_line column = prism_token.location.start_column @@ -629,16 +625,6 @@ def result value = prism_token.value lex_state = Translation::Ripper::Lexer::State[prism_state] - # A comment token does not include its terminating newline, but - # ripper's comment value does, so the newline token that directly - # follows a comment is folded back into it. - if last_comment_token && last_comment_end == prism_token.location.start_offset && (event == :on_nl || event == :on_ignored_nl) - last_comment_token[2] += value - last_comment_token = nil - last_comment_end = nil - next - end - # If there's a UTF-8 byte-order mark as the start of the file, then for # certain tokens ripper sets the first token back by 3 bytes. It also # keeps the byte order mark in the first token's value. This is weird, @@ -728,16 +714,11 @@ def result eof_token = prism_token previous_token = result_value[index - 1][0] - # A newline that was folded back into a comment still marks the - # comment boundary for the check below. - comment_boundary = previous_token.type == :COMMENT || - (index >= 2 && %i[NEWLINE NEWLINE_TERMINATOR IGNORED_NEWLINE].include?(previous_token.type) && result_value[index - 2][0].type == :COMMENT && result_value[index - 2][0].location.end_offset == previous_token.location.start_offset) - # If we're at the end of the file and the previous token was a # comment and there is still whitespace after the comment, then # Ripper will append a on_nl token (even though there isn't # necessarily a newline). We mirror that here. - if comment_boundary + if previous_token.type == :COMMENT # If the comment is at the start of a heredoc: <= 0 + next_token, _ = lexed[index] + + is_inline_comment = prev_token&.location&.start_line == token.location.start_line + if is_inline_comment && !is_at_eol && !COMMENT_CONTINUATION_TYPES.include?(next_token&.type) + tokens << [:tCOMMENT, [value, location]] + + nl_location = range(token.location.end_offset - 1, token.location.end_offset) + tokens << [:tNL, [nil, nl_location]] + next + elsif is_inline_comment && next_token&.type == :COMMENT + comment_newline_location = range(token.location.end_offset - 1, token.location.end_offset) + elsif comment_newline_location && !COMMENT_CONTINUATION_TYPES.include?(next_token&.type) + tokens << [:tCOMMENT, [value, location]] + tokens << [:tNL, [nil, comment_newline_location]] + comment_newline_location = nil + next + end end when :tNL next_token, _ = lexed[index] @@ -486,10 +501,6 @@ def to_a end end - if comment_newline_location - tokens << [:tNL, [nil, comment_newline_location]] - end - tokens end diff --git a/prism/config.yml b/prism/config.yml index 4892089c031ab8..cc5eb7e099c228 100644 --- a/prism/config.yml +++ b/prism/config.yml @@ -359,8 +359,6 @@ tokens: comment: "when" - name: NEWLINE comment: "a newline character outside of other tokens" - - name: NEWLINE_TERMINATOR - comment: "a newline that terminates a construct where a newline is otherwise insignificant" - name: PARENTHESIS_RIGHT comment: ")" - name: PIPE diff --git a/prism/prism.c b/prism/prism.c index 51980155e4a0f8..bd16a3f2822db4 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -10228,6 +10228,7 @@ parser_lex(pm_parser_t *parser) { pm_comment_t *comment = parser_comment(parser, PM_COMMENT_INLINE); pm_list_append(&parser->comment_list, (pm_list_node_t *) comment); + if (ending) parser->current.end++; parser->current.type = PM_TOKEN_COMMENT; parser_lex_callback(parser); @@ -10245,16 +10246,7 @@ parser_lex(pm_parser_t *parser) { } } - /* The comment does not include its terminating newline, - * which lexes through the newline handling below as its - * own token. A comment that ends the file has no newline, - * so the newline handling runs without one to emit. */ - if (ending == NULL) { - lexed_comment = true; - } else { - parser->current.start = ending; - parser->current.end = ending + 1; - } + lexed_comment = true; } PRISM_FALLTHROUGH case '\r': @@ -10292,11 +10284,7 @@ parser_lex(pm_parser_t *parser) { break; case PM_IGNORED_NEWLINE_PATTERN: if (parser->pattern_matching_newlines || parser->in_keyword_arg) { - if (!lexed_comment) { - parser->current.type = PM_TOKEN_NEWLINE_TERMINATOR; - parser_lex_callback(parser); - } - + if (!lexed_comment) parser_lex_ignored_newline(parser); lex_state_set(parser, PM_LEX_STATE_BEG); parser->command_start = true; parser->current.type = PM_TOKEN_NEWLINE; @@ -10393,15 +10381,11 @@ parser_lex(pm_parser_t *parser) { // If we hit a . after a newline, then we're in a call chain and // we need to return the call operator. if (next_content[0] == '.') { - /* A beginless range on the next line means this - * newline terminates the statement rather than - * continuing a method chain. */ + // To match ripper, we need to emit an ignored newline even though + // it's a real newline in the case that we have a beginless range + // on a subsequent line. if (peek_at(parser, next_content + 1) == '.') { - if (!lexed_comment) { - parser->current.type = PM_TOKEN_NEWLINE_TERMINATOR; - parser_lex_callback(parser); - } - + if (!lexed_comment) parser_lex_ignored_newline(parser); lex_state_set(parser, PM_LEX_STATE_BEG); parser->command_start = true; parser->current.type = PM_TOKEN_NEWLINE; diff --git a/prism/templates/src/tokens.c.erb b/prism/templates/src/tokens.c.erb index 6e88d423c2dd74..fb71afe217f687 100644 --- a/prism/templates/src/tokens.c.erb +++ b/prism/templates/src/tokens.c.erb @@ -275,8 +275,6 @@ pm_token_str(pm_token_type_t token_type) { return "'->'"; case PM_TOKEN_NEWLINE: return "newline"; - case PM_TOKEN_NEWLINE_TERMINATOR: - return "newline"; case PM_TOKEN_NUMBERED_REFERENCE: return "numbered reference"; case PM_TOKEN_PARENTHESIS_LEFT: diff --git a/test/prism/ruby/parser_test.rb b/test/prism/ruby/parser_test.rb index 076f84765cbc1b..e44bc20d4dea0b 100644 --- a/test/prism/ruby/parser_test.rb +++ b/test/prism/ruby/parser_test.rb @@ -109,13 +109,26 @@ class ParserTest < TestCase # These files are failing to translate their lexer output into the lexer # output expected by the parser gem, so we'll skip them for now. skip_tokens = [ + "dash_heredocs.txt", "embdoc_no_newline_at_end.txt", + "seattlerb/case_in.txt", + "seattlerb/difficult4__leading_dots2.txt", "seattlerb/heredoc_unicode.txt", "seattlerb/parse_line_heredoc.txt", "seattlerb/pct_w_heredoc_interp_nested.txt", + "seattlerb/required_kwarg_no_value.txt", + "seattlerb/TestRubyParserShared.txt", "unparser/corpus/literal/assignment.txt", "unparser/corpus/literal/literal.txt", - "whitequark/forward_arg_with_open_args.txt" + "whitequark/args.txt", + "whitequark/beginless_erange_after_newline.txt", + "whitequark/beginless_irange_after_newline.txt", + "whitequark/forward_arg_with_open_args.txt", + "whitequark/kwarg_no_paren.txt", + "whitequark/multiple_pattern_matches.txt", + "whitequark/newline_in_hash_argument.txt", + "whitequark/pattern_matching_hash.txt", + "whitequark/ruby_bug_9669.txt" ] Fixture.each_for_version(except: skip_syntax_error, version: "3.3") do |fixture| From 40e2c0fb34060a1a409c64c225d9dfec9ad3df4b Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Wed, 5 Aug 2026 20:05:17 +0900 Subject: [PATCH 06/30] [ruby/strscan] Fix `charpos` when the stored string is shrunk https://github.com/ruby/strscan/commit/7b77f30531 --- ext/strscan/strscan.c | 6 ++++-- test/strscan/test_stringscanner.rb | 8 ++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/ext/strscan/strscan.c b/ext/strscan/strscan.c index 1894ed7fb3088a..e611c22c1abceb 100644 --- a/ext/strscan/strscan.c +++ b/ext/strscan/strscan.c @@ -79,7 +79,7 @@ struct strscanner #define CURPTR(s) (S_PBEG(s) + (s)->curr) #define S_RESTLEN(s) (S_LEN(s) - (s)->curr) -#define EOS_P(s) ((s)->curr >= RSTRING_LEN(p->str)) +#define EOS_P(s) ((s)->curr >= RSTRING_LEN((s)->str)) #define GET_SCANNER(obj,var) do {\ (var) = check_strscan(obj);\ @@ -573,10 +573,12 @@ static VALUE strscan_get_charpos(VALUE self) { struct strscanner *p; + const char *s; GET_SCANNER(self, p); - return LONG2NUM(rb_enc_strlen(S_PBEG(p), CURPTR(p), rb_enc_get(p->str))); + s = EOS_P(p) ? S_PEND(p) : CURPTR(p); + return LONG2NUM(rb_enc_strlen(S_PBEG(p), s, rb_enc_get(p->str))); } /* diff --git a/test/strscan/test_stringscanner.rb b/test/strscan/test_stringscanner.rb index 47b1e2a1558be2..79784b59f50b45 100644 --- a/test/strscan/test_stringscanner.rb +++ b/test/strscan/test_stringscanner.rb @@ -231,6 +231,14 @@ class << string assert_equal(8, scanner.charpos) end + def test_charpos_when_shrunk + s = "\u{e9}" * 64 + sc = StringScanner.new(s) + sc.scan(/(?:\u{e9})+/) + s.replace("z") + assert_equal(s.length, sc.charpos) + end + def test_concat s = create_string_scanner('a'.dup) s.scan(/a/) From a57cc37dc251f8bff44e3014229b9c17b7257a82 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Wed, 5 Aug 2026 19:24:52 -0700 Subject: [PATCH 07/30] [ruby/time] Require colon in timezone offset for Time.rfc3339 Fixes https://github.com/ruby/time/pull/77 https://github.com/ruby/time/commit/b126827a31 --- lib/time.rb | 2 +- test/test_time.rb | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/time.rb b/lib/time.rb index cb9c304e28f9a4..95f8a30106058b 100644 --- a/lib/time.rb +++ b/lib/time.rb @@ -660,7 +660,7 @@ def rfc3339(time) [T\s] (\d\d):(\d\d):(\d\d) (\.\d+)? - (Z|[+-]\d\d:?\d\d) + (Z|[+-]\d\d:\d\d) \s*\z/ix _xmlschema(pattern, time) end diff --git a/test/test_time.rb b/test/test_time.rb index 2bdb35d3e1c017..53ac856d974a46 100644 --- a/test/test_time.rb +++ b/test/test_time.rb @@ -123,10 +123,11 @@ def subtest_xmlschema_alias(method) t = Time.utc(1996, 12, 20, 0, 39, 57) s = "1996-12-19T16:39:57-08:00" assert_equal(t, Time.__send__(method, s)) - assert_equal(t, Time.__send__(method, s.sub(/:(?=00\z)/, ''))) if method == :rfc3339 + assert_raise(ArgumentError) { Time.rfc3339(s.sub(/:(?=00\z)/, '')) } assert_raise(ArgumentError) { Time.rfc3339(s.sub(/:00\z/, '')) } else + assert_equal(t, Time.__send__(method, s.sub(/:(?=00\z)/, ''))) assert_equal(t, Time.__send__(method, s.sub(/:00\z/, ''))) end # There is no way to generate time string with arbitrary timezone. From 10fa3a51bea77dce31b0df398deaac3e437470c3 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 6 Aug 2026 02:12:09 +0000 Subject: [PATCH 08/30] Bump the github-actions group across 1 directory with 3 updates Bumps the github-actions group with 3 updates in the / directory: [github/codeql-action/init](https://github.com/github/codeql-action), [github/codeql-action/analyze](https://github.com/github/codeql-action) and [github/codeql-action/upload-sarif](https://github.com/github/codeql-action). Updates `github/codeql-action/init` from 4.37.4 to 4.37.5 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/f205ea1c3313d32999d8d6a48b4f6530d4437b38...d1ba80a13dd99fba24a470575428917156a28b43) Updates `github/codeql-action/analyze` from 4.37.4 to 4.37.5 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/f205ea1c3313d32999d8d6a48b4f6530d4437b38...d1ba80a13dd99fba24a470575428917156a28b43) Updates `github/codeql-action/upload-sarif` from 4.37.4 to 4.37.5 - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/f205ea1c3313d32999d8d6a48b4f6530d4437b38...d1ba80a13dd99fba24a470575428917156a28b43) --- updated-dependencies: - dependency-name: github/codeql-action/init dependency-version: 4.37.5 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: github/codeql-action/analyze dependency-version: 4.37.5 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions - dependency-name: github/codeql-action/upload-sarif dependency-version: 4.37.5 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: github-actions ... Signed-off-by: dependabot[bot] --- .github/workflows/check_sast.yml | 6 +++--- .github/workflows/scorecards.yml | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check_sast.yml b/.github/workflows/check_sast.yml index 9ef649bb64be27..9e8020ca6d7766 100644 --- a/.github/workflows/check_sast.yml +++ b/.github/workflows/check_sast.yml @@ -78,14 +78,14 @@ jobs: persist-credentials: false - name: Initialize CodeQL - uses: github/codeql-action/init@f205ea1c3313d32999d8d6a48b4f6530d4437b38 # v4.37.4 + uses: github/codeql-action/init@d1ba80a13dd99fba24a470575428917156a28b43 # v4.37.5 with: languages: ${{ matrix.language }} build-mode: none config-file: .github/codeql/codeql-config.yml - name: Perform CodeQL Analysis - uses: github/codeql-action/analyze@f205ea1c3313d32999d8d6a48b4f6530d4437b38 # v4.37.4 + uses: github/codeql-action/analyze@d1ba80a13dd99fba24a470575428917156a28b43 # v4.37.5 with: category: '/language:${{ matrix.language }}' upload: False @@ -127,7 +127,7 @@ jobs: continue-on-error: true - name: Upload SARIF - uses: github/codeql-action/upload-sarif@f205ea1c3313d32999d8d6a48b4f6530d4437b38 # v4.37.4 + uses: github/codeql-action/upload-sarif@d1ba80a13dd99fba24a470575428917156a28b43 # v4.37.5 with: sarif_file: sarif-results/${{ matrix.language }}.sarif continue-on-error: true diff --git a/.github/workflows/scorecards.yml b/.github/workflows/scorecards.yml index 09f7798f8abc11..e59ba6bc55c9a7 100644 --- a/.github/workflows/scorecards.yml +++ b/.github/workflows/scorecards.yml @@ -73,6 +73,6 @@ jobs: # Upload the results to GitHub's code scanning dashboard (optional). # Commenting out will disable upload of results to your repo's Code Scanning dashboard - name: "Upload to code-scanning" - uses: github/codeql-action/upload-sarif@f205ea1c3313d32999d8d6a48b4f6530d4437b38 # v4.37.4 + uses: github/codeql-action/upload-sarif@d1ba80a13dd99fba24a470575428917156a28b43 # v4.37.5 with: sarif_file: results.sarif From 82294a714fe3d4cacf54fae631291df908ecb1ab Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Wed, 5 Aug 2026 17:07:10 +0900 Subject: [PATCH 09/30] [Bug #22223] Check SO_ERROR after waiting for nonblocking connect Darwin 27 answers the retry connect(2) on a refused nonblocking socket with EISCONN, so the retry idiom in Addrinfo#connect_internal returned an unconnected socket. SO_ERROR still holds the real error, so consult it after wait_writable, as wait_connectable() in init.c already does. Co-Authored-By: Claude Fable 5 --- ext/socket/lib/socket.rb | 7 +++++++ test/socket/test_socket.rb | 10 ++++++++++ 2 files changed, 17 insertions(+) diff --git a/ext/socket/lib/socket.rb b/ext/socket/lib/socket.rb index a091320c486531..0ade75c2fc028f 100644 --- a/ext/socket/lib/socket.rb +++ b/ext/socket/lib/socket.rb @@ -58,6 +58,13 @@ def connect_internal(local_addrinfo, timeout=nil) # :yields: socket when :wait_writable sock.wait_writable(timeout) or raise Errno::ETIMEDOUT, "user specified timeout for #{self.ip_address}:#{self.ip_port}" + # Check SO_ERROR instead of relying on the connect_nonblock retry; + # some kernels (e.g. Darwin 27) answer the retry connect(2) with + # EISCONN even when the connection has failed. [Bug #22223] + err = sock.getsockopt(Socket::SOL_SOCKET, Socket::SO_ERROR).int + unless err.zero? + raise SystemCallError.new("connect(2) for #{self.ip_address}:#{self.ip_port}", err) + end end while true else sock.connect(self) diff --git a/test/socket/test_socket.rb b/test/socket/test_socket.rb index 3b5f5b9d74c979..b286ee30c3eff4 100644 --- a/test/socket/test_socket.rb +++ b/test/socket/test_socket.rb @@ -604,6 +604,16 @@ def test_connect_timeout sock.close if sock && ! sock.closed? end + def test_connect_timeout_connection_refused + server = TCPServer.new("127.0.0.1", 0) + port = server.addr[1] + server.close + + assert_raise(Errno::ECONNREFUSED) do + Socket.tcp("127.0.0.1", port, connect_timeout: 5) + end + end unless /mswin|mingw/ =~ RUBY_PLATFORM + def test_getifaddrs begin list = Socket.getifaddrs From ef6b630cdb51d3223299a4ddc9c97b2444235f59 Mon Sep 17 00:00:00 2001 From: Paul Barker Date: Fri, 31 Jul 2026 15:25:50 +0100 Subject: [PATCH 10/30] Finish fix for cross-compilation build race condition Commit bb235bddd4ce ("Avoid build-time race condition when cross-compiling from a source tarball") turned out to be only a partial fix, we also need to cover the case where git is present but we're not building from a git checkout. We can simply check if the `git log` command returned any output, if it didn't then we're not in a git checkout. --- defs/gmake.mk | 2 ++ 1 file changed, 2 insertions(+) diff --git a/defs/gmake.mk b/defs/gmake.mk index 7316774a115a89..088de0e6774723 100644 --- a/defs/gmake.mk +++ b/defs/gmake.mk @@ -436,6 +436,7 @@ endif ifeq ($(HAVE_GIT),yes) REVISION_LATEST := $(shell $(GIT_IN_SRC) rev-parse HEAD 2>/dev/null) +ifneq ($(REVISION_LATEST),) REVISION_IN_HEADER := $(shell sed '/^\#define RUBY_FULL_REVISION "\(.*\)"/!d;s//\1/;q' $(wildcard $(srcdir)/revision.h revision.h) /dev/null 2>/dev/null) ifeq ($(REVISION_IN_HEADER),) REVISION_IN_HEADER := none @@ -444,6 +445,7 @@ ifneq ($(REVISION_IN_HEADER),$(REVISION_LATEST)) $(REVISION_H): PHONY endif endif +endif include $(top_srcdir)/yjit/yjit.mk include $(top_srcdir)/zjit/zjit.mk From c3c0e280d03561e21fc0894ea693ce3c9ebada0e Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Thu, 6 Aug 2026 14:26:09 +0900 Subject: [PATCH 11/30] Do not consume BUNDLER_SETUP outside the main box [Bug #22123] Bundler evaluates gemspecs through TOPLEVEL_BINDING, which always belongs to the main box, so consuming BUNDLER_SETUP while the root box loads RubyGems runs Bundler code in the main box before RubyGems has finished loading there. Skip it in every box once error_highlight, did_you_mean and syntax_suggest are autoloaded, since nothing loaded here needs the bundle and RUBYOPT=-rbundler/setup sets Bundler up after the boot sequence. Co-Authored-By: Claude Opus 5 --- gem_prelude.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/gem_prelude.rb b/gem_prelude.rb index 1a0af96aedff20..77bae5bbef3b3e 100644 --- a/gem_prelude.rb +++ b/gem_prelude.rb @@ -1,4 +1,15 @@ begin + # rubygems.rb requires ENV["BUNDLER_SETUP"] at its end so that bundler/setup + # runs before error_highlight, did_you_mean and syntax_suggest are loaded. + # That is unnecessary once they are autoloaded ([Feature #21951]), and it + # must not happen outside the main box: Bundler evaluates gemspecs through + # TOPLEVEL_BINDING, which always belongs to the main box, so it would run + # Bundler code there before RubyGems finishes loading. Either way Bundler is + # set up by RUBYOPT=-rbundler/setup after the boot sequence. + if %i[ErrorHighlight DidYouMean SyntaxSuggest].any? {|c| Object.autoload?(c) } || + (defined?(Ruby::Box) && Ruby::Box.enabled? && !Ruby::Box.current.main?) + bundler_setup = ENV.delete("BUNDLER_SETUP") + end require 'rubygems' rescue LoadError => e raise unless e.path == 'rubygems' @@ -6,4 +17,6 @@ warn "`RubyGems' were not loaded." else require 'bundled_gems' +ensure + ENV["BUNDLER_SETUP"] = bundler_setup if bundler_setup end if defined?(Gem) From 8268411788012d92958836fd6ecfe12332283202 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Thu, 6 Aug 2026 14:44:41 +0900 Subject: [PATCH 12/30] Note when the BUNDLER_SETUP require can be dropped Ruby 4.1 autoloads error_highlight, did_you_mean and syntax_suggest and deletes BUNDLER_SETUP while loading RubyGems, so the early bundler/setup is only there for Ruby 3.2 through 4.0. Co-Authored-By: Claude Opus 5 --- lib/rubygems.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/rubygems.rb b/lib/rubygems.rb index d289cab0fd627e..1cac0433cd8101 100644 --- a/lib/rubygems.rb +++ b/lib/rubygems.rb @@ -1471,4 +1471,9 @@ def default_gem_load_paths end eval File.read(path), nil, file +# bundler/setup has to run before error_highlight, did_you_mean and +# syntax_suggest are loaded, so that the Gemfile controls their versions +# ([Bug #19089]). Ruby 4.1 autoloads them ([Feature #21951]) and deletes this +# variable while loading RubyGems, so this can go once 4.1 is the oldest +# supported version. require ENV["BUNDLER_SETUP"] if ENV["BUNDLER_SETUP"] && !defined?(Bundler) From eec8cf0bc1e41bf8cdf0d6d85a4a4eeb192b35c8 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Thu, 6 Aug 2026 15:14:01 +0900 Subject: [PATCH 13/30] Test BUNDLER_SETUP loading in boxes Based on the test in https://github.com/ruby/ruby/pull/17323, extended to cover user boxes and the configuration where the decorator gems are not autoloaded, which is how Ruby 3.2 through 4.0 behave. Co-Authored-By: Claude Opus 5 --- test/ruby/test_box.rb | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/test/ruby/test_box.rb b/test/ruby/test_box.rb index ab6f53b92ad72e..e65cd1d11a62f2 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -938,6 +938,47 @@ def test_boxes_have_different_rubygems end end + def test_bundler_setup_not_loaded_while_decorator_gems_are_autoloaded + with_bundler_setup_log do |env| + # assert_separately w/ ENV_ENABLE_BOX and --enable=gems causes timeouts on CI @ Windows + assert_in_out_err([env, "--enable=gems"], "#{<<-"begin;"}\n#{<<-'end;'}") do |output, error| + begin; + Ruby::Box.new + puts File.readlines(ENV["BUNDLER_SETUP_LOG"], chomp: true) + end; + assert_equal [], output + end + end + end + + def test_bundler_setup_loaded_only_in_main_box + with_bundler_setup_log do |env| + opts = [env, "--enable=gems", "--disable=error_highlight", "--disable=did_you_mean", "--disable=syntax_suggest"] + assert_in_out_err(opts, "#{<<-"begin;"}\n#{<<-'end;'}") do |output, error| + begin; + Ruby::Box.new + puts File.readlines(ENV["BUNDLER_SETUP_LOG"], chomp: true) + end; + assert_equal ["true"], output + end + end + end + + # Runs a BUNDLER_SETUP script that records the box it was loaded in, after + # touching RubyGems through TOPLEVEL_BINDING as Bundler does for gemspecs. + def with_bundler_setup_log + Tempfile.create(["bundler_setup", ".rb"]) do |setup| + Tempfile.create(["bundler_setup_log", ".txt"]) do |log| + setup.puts 'eval("Gem::Specification", TOPLEVEL_BINDING.dup)' + setup.puts 'File.write(ENV["BUNDLER_SETUP_LOG"], "#{Ruby::Box.current.main?}\n", mode: "a")' + setup.close + log.close + + yield ENV_ENABLE_BOX.merge("BUNDLER_SETUP" => setup.path, "BUNDLER_SETUP_LOG" => log.path) + end + end + end + def test_require_list_loaded_only_in_main_box Tempfile.create(["req_a", ".rb"]) do |t1| Tempfile.create(["req_b", ".rb"]) do |t2| From 4c5ba84ba226ac7da1f28040eabe524baa5a227f Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Thu, 6 Aug 2026 15:19:01 +0900 Subject: [PATCH 14/30] Make the autoload test portable to Ruby 4.0 error_highlight, did_you_mean and syntax_suggest are still loaded eagerly on Ruby 3.2 through 4.0, where BUNDLER_SETUP is loaded in the main box rather than skipped everywhere, so derive the expectation from the actual autoload state. Co-Authored-By: Claude Opus 5 --- test/ruby/test_box.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/ruby/test_box.rb b/test/ruby/test_box.rb index e65cd1d11a62f2..4583db804117a8 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -944,9 +944,11 @@ def test_bundler_setup_not_loaded_while_decorator_gems_are_autoloaded assert_in_out_err([env, "--enable=gems"], "#{<<-"begin;"}\n#{<<-'end;'}") do |output, error| begin; Ruby::Box.new - puts File.readlines(ENV["BUNDLER_SETUP_LOG"], chomp: true) + autoloaded = %i[ErrorHighlight DidYouMean SyntaxSuggest].any? {|c| Object.autoload?(c) } + loaded = File.readlines(ENV["BUNDLER_SETUP_LOG"], chomp: true) + puts loaded == (autoloaded ? [] : ["true"]) ? "ok" : "loaded in #{loaded.inspect}" end; - assert_equal [], output + assert_equal ["ok"], output end end end From 1d9b1291dc068ed7a475709de64b1937e7d00b84 Mon Sep 17 00:00:00 2001 From: Shugo Maeda Date: Fri, 24 Jul 2026 10:26:18 +0900 Subject: [PATCH 15/30] [DOC] Deprecate ruby2_keywords Add documentation-only deprecation notices to Module#ruby2_keywords, main.ruby2_keywords, Proc#ruby2_keywords, Hash.ruby2_keywords_hash?, and Hash.ruby2_keywords_hash, as the first phase of the schedule proposed at https://bugs.ruby-lang.org/issues/22205. --- NEWS.md | 13 +++++++++++++ hash.c | 8 ++++++++ proc.c | 18 +++++------------- vm_method.c | 27 ++++++++++++--------------- 4 files changed, 38 insertions(+), 28 deletions(-) diff --git a/NEWS.md b/NEWS.md index e5eca2441cd127..e3e2665e67fa15 100644 --- a/NEWS.md +++ b/NEWS.md @@ -39,6 +39,11 @@ Note: We're only listing outstanding class updates. given names, raising `KeyError` for missing names unless a block is given. [[Feature #21781]] +* Hash + + * `Hash.ruby2_keywords_hash?` and `Hash.ruby2_keywords_hash` are + deprecated and will be removed in Ruby 4.5. [[Feature #22205]] + * Integer * `Integer#bit_count` is added. It returns the number of `1` bits in the @@ -59,6 +64,11 @@ Note: We're only listing outstanding class updates. * `MatchData#integer_at` is added. It converts the matched substring to integer and return the result. [[Feature #21932]] +* Module + + * `Module#ruby2_keywords` and top-level `ruby2_keywords` are + deprecated and will be removed in Ruby 4.4. [[Feature #22205]] + * ObjectSpace * `ObjectSpace._id2ref` was removed. [[Feature #22135]] @@ -69,6 +79,8 @@ Note: We're only listing outstanding class updates. receiver but with the refinements activated by the given modules in effect inside its body, without affecting the original `Proc`. [[Feature #22097]] + * `Proc#ruby2_keywords` is deprecated and will be removed in Ruby 4.4. + [[Feature #22205]] * Range @@ -300,6 +312,7 @@ A lot of work has gone into making Ractors more stable, performant, and usable. [Feature #22139]: https://bugs.ruby-lang.org/issues/22139 [Feature #22175]: https://bugs.ruby-lang.org/issues/22175 [Feature #22185]: https://bugs.ruby-lang.org/issues/22185 +[Feature #22205]: https://bugs.ruby-lang.org/issues/22205 [PR #17201]: https://github.com/ruby/ruby/pull/17201 [GH-psych #805]: https://github.com/ruby/psych/pull/805 [RubyGems-v4.0.4]: https://github.com/rubygems/rubygems/releases/tag/v4.0.4 diff --git a/hash.c b/hash.c index c898327870bc79..6524bc47ba9dab 100644 --- a/hash.c +++ b/hash.c @@ -1924,6 +1924,10 @@ rb_hash_s_try_convert(VALUE dummy, VALUE hash) * call-seq: * Hash.ruby2_keywords_hash?(hash) -> true or false * + * Deprecated: will be removed in Ruby 4.5, one version after the + * removal of the ruby2_keywords mechanism. See + * https://bugs.ruby-lang.org/issues/22205 for the schedule. + * * Checks if a given hash is flagged by Module#ruby2_keywords (or * Proc#ruby2_keywords). * This method is not for casual use; debugging, researching, and @@ -1946,6 +1950,10 @@ rb_hash_s_ruby2_keywords_hash_p(VALUE dummy, VALUE hash) * call-seq: * Hash.ruby2_keywords_hash(hash) -> hash * + * Deprecated: will be removed in Ruby 4.5, one version after the + * removal of the ruby2_keywords mechanism. See + * https://bugs.ruby-lang.org/issues/22205 for the schedule. + * * Duplicates a given hash and adds a ruby2_keywords flag. * This method is not for casual use; debugging, researching, and * some truly necessary cases like deserialization of arguments. diff --git a/proc.c b/proc.c index f57196fff157f9..8996538138146b 100644 --- a/proc.c +++ b/proc.c @@ -4644,6 +4644,11 @@ rb_method_compose_to_right(VALUE self, VALUE g) * call-seq: * proc.ruby2_keywords -> proc * + * Deprecated: will be removed in Ruby 4.4. Use explicit delegation + * (*args, **kwargs) instead; it works correctly on Ruby 3.0 + * and later. See https://bugs.ruby-lang.org/issues/22205 for the + * schedule. + * * Marks the proc as passing keywords through a normal argument splat. * This should only be called on procs that accept an argument splat * (*args) but not explicit keywords or a keyword splat. It @@ -4657,19 +4662,6 @@ rb_method_compose_to_right(VALUE self, VALUE g) * This should only be used for procs that delegate keywords to another * method, and only for backwards compatibility with Ruby versions before * 2.7. - * - * This method will probably be removed at some point, as it exists only - * for backwards compatibility. As it does not exist in Ruby versions - * before 2.7, check that the proc responds to this method before calling - * it. Also, be aware that if this method is removed, the behavior of the - * proc will change so that it does not pass through keywords. - * - * module Mod - * foo = ->(meth, *args, &block) do - * send(:"do_#{meth}", *args, &block) - * end - * foo.ruby2_keywords if foo.respond_to?(:ruby2_keywords) - * end */ static VALUE diff --git a/vm_method.c b/vm_method.c index 7d3610f60b0b42..cf4998f477c331 100644 --- a/vm_method.c +++ b/vm_method.c @@ -3121,6 +3121,12 @@ rb_mod_private(int argc, VALUE *argv, VALUE module) * call-seq: * ruby2_keywords(method_name, ...) -> nil * + * Deprecated: will be removed in Ruby 4.4. Use ... + * {argument forwarding}[rdoc-ref:syntax/methods.rdoc@Argument+Forwarding] + * or other delegation styles instead; they work correctly on Ruby 3.0 + * and later. See https://bugs.ruby-lang.org/issues/22205 for the + * schedule. + * * For the given method names, marks the method as passing keywords through * a normal argument splat. This should only be called on methods that * accept an argument splat (*args) but not explicit keywords or @@ -3136,21 +3142,6 @@ rb_mod_private(int argc, VALUE *argv, VALUE module) * method, and only for backwards compatibility with Ruby versions before 3.0. * See https://www.ruby-lang.org/en/news/2019/12/12/separation-of-positional-and-keyword-arguments-in-ruby-3-0/ * for details on why +ruby2_keywords+ exists and when and how to use it. - * - * This method will probably be removed at some point, as it exists only - * for backwards compatibility. As it does not exist in Ruby versions before - * 2.7, check that the module responds to this method before calling it: - * - * module Mod - * def foo(meth, *args, &block) - * send(:"do_#{meth}", *args, &block) - * end - * ruby2_keywords(:foo) if respond_to?(:ruby2_keywords, true) - * end - * - * However, be aware that if the +ruby2_keywords+ method is removed, the - * behavior of the +foo+ method using the above approach will change so that - * the method does not pass through keywords. */ static VALUE @@ -3323,6 +3314,12 @@ top_private(int argc, VALUE *argv, VALUE _) * call-seq: * ruby2_keywords(method_name, ...) -> self * + * Deprecated: will be removed in Ruby 4.4. Use ... + * {argument forwarding}[rdoc-ref:syntax/methods.rdoc@Argument+Forwarding] + * or other delegation styles instead; they work correctly on Ruby 3.0 + * and later. See https://bugs.ruby-lang.org/issues/22205 for the + * schedule. + * * For the given method names, marks the method as passing keywords through * a normal argument splat. See Module#ruby2_keywords in detail. */ From a3c7ff63ea5ade187ec87d2344ca296a9e90cf64 Mon Sep 17 00:00:00 2001 From: largo Date: Tue, 16 Jun 2026 17:46:39 +0000 Subject: [PATCH 16/30] win32: single-syscall fast path for winnt_stat (Win10-safe) winnt_stat opened a real file handle (open_special + GetFileInformationByHandle + GetFileType + get_handle_pathname + CloseHandle, ~5 syscalls) for every existing file just to stat it. require does this thousands of times per startup. Add a fast path that returns size/timestamps/attributes from a single metadata syscall for regular files and directories: - GetFileInformationByName (Windows 11 24H2+): one syscall, also gives real FileId and link count -> accurate st_ino/st_nlink. Resolved via GetProcAddress; Windows 10 and earlier fall through. - GetFileAttributesExW (all supported Windows): one syscall for size+times+attrs; st_ino/st_nlink left 0/1 (same compromise as the existing stat_by_find fallback). Reparse points (symlinks, AF_UNIX sockets) and unusual errors fall through to the original handle-based path unchanged. Single file, one function, no new dependencies. Measured on Ruby 4.0.5: require "nokogiri" 224 ms -> 81 ms (2.77x); File.stat of an existing file 112 us -> 21 us (5.4x). --- win32/win32.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/win32/win32.c b/win32/win32.c index f3cda7bf804718..2cd37204e55771 100644 --- a/win32/win32.c +++ b/win32/win32.c @@ -5822,6 +5822,103 @@ path_drive(const WCHAR *path) static int winnt_stat(const WCHAR *path, struct stati128 *st, BOOL lstat) { + /* ---- Fast path: avoid opening a file handle for the common case ---- + * + * The original code below opens every existing file + * (open_special + GetFileInformationByHandle + GetFileType + + * get_handle_pathname + CloseHandle) just to stat it. `require` does + * this thousands of times per startup. For a regular file or directory + * a single metadata syscall is enough: + * + * - GetFileInformationByName (Windows 11 24H2+): one syscall returning + * size, all timestamps, attributes, real FileId and link count. + * Resolved at runtime via GetProcAddress, so Windows 10 / older + * simply falls through to GetFileAttributesExW below. + * - GetFileAttributesExW (every supported Windows): one syscall for + * size + timestamps + attributes. st_ino/st_nlink are left 0/1, + * matching the existing stat_by_find fallback's compromise. + * + * Reparse points (symlinks, AF_UNIX sockets) fall through to the + * original handle-based path, which inspects the reparse tag. */ + { + typedef struct { + LARGE_INTEGER FileId, CreationTime, LastAccessTime, LastWriteTime, + ChangeTime, AllocationSize, EndOfFile; + ULONG FileAttributes, ReparseTag, NumberOfLinks; + ACCESS_MASK EffectiveAccess; + } RB_FILE_STAT_INFO; + typedef BOOL (WINAPI *gfibn_t)(PCWSTR, int, PVOID, ULONG); + static gfibn_t pGFIBN = (gfibn_t)-1; + if (pGFIBN == (gfibn_t)-1) { + HMODULE k = GetModuleHandleW(L"kernel32.dll"); + pGFIBN = k ? (gfibn_t)GetProcAddress(k, "GetFileInformationByName") : NULL; + } + DWORD fp_attr = (DWORD)-1; + int fp_filled = 0; + if (pGFIBN) { + RB_FILE_STAT_INFO info; + if (pGFIBN(path, 0 /*FileStatByNameInfo*/, &info, sizeof(info))) { + fp_attr = info.FileAttributes; + if (!(fp_attr & FILE_ATTRIBUTE_REPARSE_POINT)) { + memset(st, 0, sizeof(*st)); + st->st_size = info.EndOfFile.QuadPart; + st->st_atime = filetime_to_unixtime((FILETIME *)&info.LastAccessTime); + st->st_atimensec = filetime_to_nsec((FILETIME *)&info.LastAccessTime); + st->st_mtime = filetime_to_unixtime((FILETIME *)&info.LastWriteTime); + st->st_mtimensec = filetime_to_nsec((FILETIME *)&info.LastWriteTime); + st->st_ctime = filetime_to_unixtime((FILETIME *)&info.CreationTime); + st->st_ctimensec = filetime_to_nsec((FILETIME *)&info.CreationTime); + st->st_nlink = info.NumberOfLinks; + st->st_ino = info.FileId.QuadPart; + fp_filled = 1; + } + } + else { + DWORD e = GetLastError(); + if (e == ERROR_FILE_NOT_FOUND || e == ERROR_INVALID_NAME || + e == ERROR_PATH_NOT_FOUND || e == ERROR_BAD_NETPATH) { + errno = map_errno(e); + return -1; + } + } + } + else { + WIN32_FILE_ATTRIBUTE_DATA fad; + if (GetFileAttributesExW(path, GetFileExInfoStandard, &fad)) { + fp_attr = fad.dwFileAttributes; + if (!(fp_attr & FILE_ATTRIBUTE_REPARSE_POINT)) { + memset(st, 0, sizeof(*st)); + st->st_size = ((__int64)fad.nFileSizeHigh << 32) | fad.nFileSizeLow; + st->st_atime = filetime_to_unixtime(&fad.ftLastAccessTime); + st->st_atimensec = filetime_to_nsec(&fad.ftLastAccessTime); + st->st_mtime = filetime_to_unixtime(&fad.ftLastWriteTime); + st->st_mtimensec = filetime_to_nsec(&fad.ftLastWriteTime); + st->st_ctime = filetime_to_unixtime(&fad.ftCreationTime); + st->st_ctimensec = filetime_to_nsec(&fad.ftCreationTime); + st->st_nlink = 1; + fp_filled = 1; + } + } + else { + DWORD e = GetLastError(); + if (e == ERROR_FILE_NOT_FOUND || e == ERROR_INVALID_NAME || + e == ERROR_PATH_NOT_FOUND || e == ERROR_BAD_NETPATH) { + errno = map_errno(e); + return -1; + } + } + } + if (fp_filled) { + if (fp_attr & FILE_ATTRIBUTE_DIRECTORY) { + if (check_valid_dir(path)) return -1; + } + st->st_mode = fileattr_to_unixmode(fp_attr, path, 0); + st->st_dev = st->st_rdev = path_drive(path); + return 0; + } + } + /* ---- end fast path; original handle-based path follows ---- */ + DWORD flags = lstat ? FILE_FLAG_OPEN_REPARSE_POINT : 0; HANDLE f; WCHAR *finalname = 0; From a7512f063c7ec35b617803dd2f17515416f2c493 Mon Sep 17 00:00:00 2001 From: Hiroshi SHIBATA Date: Tue, 4 Aug 2026 12:35:07 +0900 Subject: [PATCH 17/30] [Bug #19378] win32: Rework stat fast path Query FileStatBasicByNameInfo instead of FileStatByNameInfo for the 128-bit file ID; the 64-bit ID does not match the handle-based result on ReFS. Fall back to the handle-based path unless the volume serial matches the drive of the path, to keep st_dev of files behind cross-volume junctions and mount points, and also for non-disk devices and filesystems without file IDs. Drop the GetFileAttributesExW path that loses st_ino. Co-Authored-By: Claude Fable 5 --- win32/win32.c | 227 +++++++++++++++++++++++++++++--------------------- 1 file changed, 131 insertions(+), 96 deletions(-) diff --git a/win32/win32.c b/win32/win32.c index 2cd37204e55771..4f2504781466b1 100644 --- a/win32/win32.c +++ b/win32/win32.c @@ -5818,113 +5818,148 @@ path_drive(const WCHAR *path) return _getdrive() - 1; } +#if !defined(NTDDI_WIN11_ZN) || NTDDI_VERSION < NTDDI_WIN11_ZN +/* FileStatBasicByNameInfo in FILE_INFO_BY_NAME_CLASS and + * FILE_STAT_BASIC_INFORMATION, in SDKs since Windows 11 24H2 */ +#define FileStatBasicByNameInfo 3 + +typedef struct { + LARGE_INTEGER FileId; + LARGE_INTEGER CreationTime; + LARGE_INTEGER LastAccessTime; + LARGE_INTEGER LastWriteTime; + LARGE_INTEGER ChangeTime; + LARGE_INTEGER AllocationSize; + LARGE_INTEGER EndOfFile; + DWORD FileAttributes; + DWORD ReparseTag; + DWORD NumberOfLinks; + DWORD DeviceType; + DWORD DeviceCharacteristics; + DWORD Reserved; + LARGE_INTEGER VolumeSerialNumber; + FILE_ID_128 FileId128; +} FILE_STAT_BASIC_INFORMATION; +#endif + +#ifndef FILE_DEVICE_DISK +#define FILE_DEVICE_DISK 7 +#endif + +typedef BOOL (WINAPI *get_file_information_by_name_func) + (PCWSTR, int /* FILE_INFO_BY_NAME_CLASS */, PVOID, ULONG); +static get_file_information_by_name_func get_file_information_by_name = + (get_file_information_by_name_func)-1; + /* License: Ruby's */ -static int -winnt_stat(const WCHAR *path, struct stati128 *st, BOOL lstat) +static time_t +large_integer_to_unixtime(const LARGE_INTEGER *at, long *nsecp) { - /* ---- Fast path: avoid opening a file handle for the common case ---- - * - * The original code below opens every existing file - * (open_special + GetFileInformationByHandle + GetFileType + - * get_handle_pathname + CloseHandle) just to stat it. `require` does - * this thousands of times per startup. For a regular file or directory - * a single metadata syscall is enough: - * - * - GetFileInformationByName (Windows 11 24H2+): one syscall returning - * size, all timestamps, attributes, real FileId and link count. - * Resolved at runtime via GetProcAddress, so Windows 10 / older - * simply falls through to GetFileAttributesExW below. - * - GetFileAttributesExW (every supported Windows): one syscall for - * size + timestamps + attributes. st_ino/st_nlink are left 0/1, - * matching the existing stat_by_find fallback's compromise. - * - * Reparse points (symlinks, AF_UNIX sockets) fall through to the - * original handle-based path, which inspects the reparse tag. */ - { - typedef struct { - LARGE_INTEGER FileId, CreationTime, LastAccessTime, LastWriteTime, - ChangeTime, AllocationSize, EndOfFile; - ULONG FileAttributes, ReparseTag, NumberOfLinks; - ACCESS_MASK EffectiveAccess; - } RB_FILE_STAT_INFO; - typedef BOOL (WINAPI *gfibn_t)(PCWSTR, int, PVOID, ULONG); - static gfibn_t pGFIBN = (gfibn_t)-1; - if (pGFIBN == (gfibn_t)-1) { - HMODULE k = GetModuleHandleW(L"kernel32.dll"); - pGFIBN = k ? (gfibn_t)GetProcAddress(k, "GetFileInformationByName") : NULL; - } - DWORD fp_attr = (DWORD)-1; - int fp_filled = 0; - if (pGFIBN) { - RB_FILE_STAT_INFO info; - if (pGFIBN(path, 0 /*FileStatByNameInfo*/, &info, sizeof(info))) { - fp_attr = info.FileAttributes; - if (!(fp_attr & FILE_ATTRIBUTE_REPARSE_POINT)) { - memset(st, 0, sizeof(*st)); - st->st_size = info.EndOfFile.QuadPart; - st->st_atime = filetime_to_unixtime((FILETIME *)&info.LastAccessTime); - st->st_atimensec = filetime_to_nsec((FILETIME *)&info.LastAccessTime); - st->st_mtime = filetime_to_unixtime((FILETIME *)&info.LastWriteTime); - st->st_mtimensec = filetime_to_nsec((FILETIME *)&info.LastWriteTime); - st->st_ctime = filetime_to_unixtime((FILETIME *)&info.CreationTime); - st->st_ctimensec = filetime_to_nsec((FILETIME *)&info.CreationTime); - st->st_nlink = info.NumberOfLinks; - st->st_ino = info.FileId.QuadPart; - fp_filled = 1; - } - } - else { - DWORD e = GetLastError(); - if (e == ERROR_FILE_NOT_FOUND || e == ERROR_INVALID_NAME || - e == ERROR_PATH_NOT_FOUND || e == ERROR_BAD_NETPATH) { - errno = map_errno(e); - return -1; - } - } - } - else { - WIN32_FILE_ATTRIBUTE_DATA fad; - if (GetFileAttributesExW(path, GetFileExInfoStandard, &fad)) { - fp_attr = fad.dwFileAttributes; - if (!(fp_attr & FILE_ATTRIBUTE_REPARSE_POINT)) { - memset(st, 0, sizeof(*st)); - st->st_size = ((__int64)fad.nFileSizeHigh << 32) | fad.nFileSizeLow; - st->st_atime = filetime_to_unixtime(&fad.ftLastAccessTime); - st->st_atimensec = filetime_to_nsec(&fad.ftLastAccessTime); - st->st_mtime = filetime_to_unixtime(&fad.ftLastWriteTime); - st->st_mtimensec = filetime_to_nsec(&fad.ftLastWriteTime); - st->st_ctime = filetime_to_unixtime(&fad.ftCreationTime); - st->st_ctimensec = filetime_to_nsec(&fad.ftCreationTime); - st->st_nlink = 1; - fp_filled = 1; - } - } - else { - DWORD e = GetLastError(); - if (e == ERROR_FILE_NOT_FOUND || e == ERROR_INVALID_NAME || - e == ERROR_PATH_NOT_FOUND || e == ERROR_BAD_NETPATH) { - errno = map_errno(e); - return -1; - } - } - } - if (fp_filled) { - if (fp_attr & FILE_ATTRIBUTE_DIRECTORY) { - if (check_valid_dir(path)) return -1; - } - st->st_mode = fileattr_to_unixmode(fp_attr, path, 0); - st->st_dev = st->st_rdev = path_drive(path); - return 0; + FILETIME ft; + + ft.dwLowDateTime = at->LowPart; + ft.dwHighDateTime = at->HighPart; + *nsecp = filetime_to_nsec(&ft); + return filetime_to_unixtime(&ft); +} + +/* License: Ruby's */ +static LONG_LONG +path_drive_serial(const WCHAR *path) +{ + static LONG_LONG serials[26]; + int drive; + + if (path[0] && path[1] == L':') { + if (!iswalpha(path[0])) return 0; + drive = towupper(path[0]) - L'A'; + } + else { + drive = _getdrive() - 1; + } + if (drive < 0 || (int)numberof(serials) <= drive) return 0; + if (!serials[drive]) { + FILE_STAT_BASIC_INFORMATION info; + WCHAR root[] = L"_:\\"; + root[0] = L'A' + drive; + if (get_file_information_by_name(root, FileStatBasicByNameInfo, + &info, sizeof(info))) + serials[drive] = info.VolumeSerialNumber.QuadPart; + } + return serials[drive]; +} + +/* License: Ruby's */ +static int +stat_by_name(const WCHAR *path, struct stati128 *st) +{ + /* Fill the stat result from a single metadata syscall, without + * opening a file handle. Returns 1 to fall back to the + * handle-based path. */ + FILE_STAT_BASIC_INFORMATION info; + unsigned __int64 ino; + __int64 inohigh; + + if (get_file_information_by_name == (get_file_information_by_name_func)-1) { + /* Since Windows 11 24H2 */ + get_file_information_by_name = (get_file_information_by_name_func) + get_proc_address("kernel32", "GetFileInformationByName", NULL); + } + if (!get_file_information_by_name) return 1; + if (!get_file_information_by_name(path, FileStatBasicByNameInfo, + &info, sizeof(info))) { + DWORD e = GetLastError(); + switch (e) { + case ERROR_FILE_NOT_FOUND: + case ERROR_INVALID_NAME: + case ERROR_PATH_NOT_FOUND: + case ERROR_BAD_NETPATH: + errno = map_errno(e); + return -1; } + return 1; /* devices, UNC paths, unusual errors */ } - /* ---- end fast path; original handle-based path follows ---- */ + if (info.FileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) + return 1; /* symlinks, junctions, AF_UNIX sockets */ + if (info.DeviceType != FILE_DEVICE_DISK) + return 1; + if (info.VolumeSerialNumber.QuadPart != path_drive_serial(path)) + return 1; /* reparse point in intermediate components */ + ino = *((unsigned __int64 *)&info.FileId128); + inohigh = *((__int64 *)&info.FileId128 + 1); + if (!ino && !inohigh) + return 1; /* file ID is not available */ + if (info.FileAttributes & FILE_ATTRIBUTE_DIRECTORY) { + if (check_valid_dir(path)) return -1; + } + st->st_ino = ino; + st->st_inohigh = inohigh; + st->st_size = info.EndOfFile.QuadPart; + st->st_atime = large_integer_to_unixtime(&info.LastAccessTime, &st->st_atimensec); + st->st_mtime = large_integer_to_unixtime(&info.LastWriteTime, &st->st_mtimensec); + st->st_ctime = large_integer_to_unixtime(&info.CreationTime, &st->st_ctimensec); + st->st_nlink = info.NumberOfLinks; + st->st_mode = fileattr_to_unixmode(info.FileAttributes, path, 0); + st->st_dev = st->st_rdev = path_drive(path); + return 0; +} +/* License: Ruby's */ +static int +winnt_stat(const WCHAR *path, struct stati128 *st, BOOL lstat) +{ DWORD flags = lstat ? FILE_FLAG_OPEN_REPARSE_POINT : 0; HANDLE f; WCHAR *finalname = 0; int open_error; memset(st, 0, sizeof(*st)); + switch (stat_by_name(path, st)) { + case 0: + return 0; + case -1: + return -1; + } f = open_special(path, 0, flags); open_error = GetLastError(); if (f == INVALID_HANDLE_VALUE && !lstat) { From 39e43e1c203cda09f0cbbfc1991f895a91043ddf Mon Sep 17 00:00:00 2001 From: Shugo Maeda Date: Fri, 24 Jul 2026 11:32:12 +0900 Subject: [PATCH 18/30] [Feature #22213] Allow no-argument and chained calls of Proc#refined Since `prc.refined(*ms).refined(*ns)` is behaviorally equivalent to `prc.refined(*ms, *ns)`, the copy of the block is deferred until the first call and the memo is shared by these behaviorally equivalent Procs. A Proc that is never called is no longer copied at all. Co-Authored-By: Claude Opus 5 Co-Authored-By: Claude Fable 5 --- compile.c | 2 + cont.c | 6 +- iseq.h | 2 + jit.c | 2 +- proc.c | 370 ++++++++++++++++++++-------- spec/ruby/core/proc/refined_spec.rb | 48 +++- test/ruby/test_proc.rb | 230 ++++++++++++++--- thread.c | 2 +- vm.c | 15 +- vm_core.h | 12 +- vm_eval.c | 4 +- vm_insnhelper.c | 2 +- 12 files changed, 535 insertions(+), 160 deletions(-) diff --git a/compile.c b/compile.c index 0e73114c35572d..46d00686b9903b 100644 --- a/compile.c +++ b/compile.c @@ -15209,6 +15209,8 @@ rb_iseq_dup_with_independent_caches(const rb_iseq_t *src_root) rb_ibf_load_iseq_complete(copy); } + FL_SET((VALUE)copy, ISEQ_REFINED_COPY); + struct rb_iseq_constant_body *cb = ISEQ_BODY(copy); if (!cb->local_iseq) RB_OBJ_WRITE(copy, &cb->local_iseq, sb->local_iseq); RB_OBJ_WRITE(copy, &cb->location.pathobj, sb->location.pathobj); diff --git a/cont.c b/cont.c index 1a39918c215ac5..016cc8f10f2924 100644 --- a/cont.c +++ b/cont.c @@ -2637,7 +2637,6 @@ rb_fiber_start(rb_fiber_t *fiber_arg) rb_fiber_t * volatile fiber = fiber_arg; rb_thread_t * volatile th = fiber->cont.saved_ec.thread_ptr; - rb_proc_t *proc; enum ruby_tag_type state; VM_ASSERT(th->ec == GET_EC()); @@ -2647,12 +2646,10 @@ rb_fiber_start(rb_fiber_t *fiber_arg) th->blocking += 1; } - /* resolved before EC_PUSH_TAG to keep the setjmp region minimal */ - const rb_cref_t *cref = rb_proc_refinements_cref(fiber->first_proc); - EC_PUSH_TAG(th->ec); if ((state = EC_EXEC_TAG()) == TAG_NONE) { rb_context_t *cont = &fiber->cont; + rb_proc_t *proc; int argc; const VALUE *argv, args = cont->value; GetProcPtr(fiber->first_proc, proc); @@ -2663,6 +2660,7 @@ rb_fiber_start(rb_fiber_t *fiber_arg) th->ec->root_svar = Qfalse; EXEC_EVENT_HOOK(th->ec, RUBY_EVENT_FIBER_SWITCH, th->self, 0, 0, 0, Qnil); + const rb_cref_t *cref = rb_proc_refinements_cref_for_call(fiber->first_proc); cont->value = rb_vm_invoke_proc(th->ec, proc, argc, argv, cont->kw_splat, VM_BLOCK_HANDLER_NONE, cref); } EC_POP_TAG(); diff --git a/iseq.h b/iseq.h index b346ad0da013e6..8308260fedfd68 100644 --- a/iseq.h +++ b/iseq.h @@ -91,6 +91,8 @@ ISEQ_ORIGINAL_ISEQ_CLEAR(const rb_iseq_t *iseq) #define ISEQ_NOT_LOADED_YET IMEMO_FL_USER1 #define ISEQ_USE_COMPILE_DATA IMEMO_FL_USER2 #define ISEQ_TRANSLATED IMEMO_FL_USER3 +/* set on every iseq of a subtree copied for Proc#refined */ +#define ISEQ_REFINED_COPY IMEMO_FL_USER4 #define ISEQ_EXECUTABLE_P(iseq) (FL_TEST_RAW(((VALUE)iseq), ISEQ_NOT_LOADED_YET | ISEQ_USE_COMPILE_DATA) == 0) diff --git a/jit.c b/jit.c index e142ab44c4e45a..086c207a81a332 100644 --- a/jit.c +++ b/jit.c @@ -235,7 +235,7 @@ rb_optimized_call(VALUE recv, rb_execution_context_t *ec, int argc, VALUE *argv, rb_proc_t *proc; GetProcPtr(recv, proc); return rb_vm_invoke_proc(ec, proc, argc, argv, kw_splat, block_handler, - rb_proc_refinements_cref(recv)); + rb_proc_refinements_cref_for_call(recv)); } unsigned int diff --git a/proc.c b/proc.c index 8996538138146b..0f501699acd9fb 100644 --- a/proc.c +++ b/proc.c @@ -268,8 +268,7 @@ block_mark_and_move(struct rb_block *block) } } -/* hidden ivar holding a refined proc's cref; see Proc#refined */ -static ID id_refinements_cref; +static ID id_refinements_recipe; static void proc_mark_and_move(void *ptr) @@ -278,21 +277,44 @@ proc_mark_and_move(void *ptr) block_mark_and_move((struct rb_block *)&proc->block); } -const rb_cref_t * -rb_proc_refinements_cref(VALUE procval) +enum refinement_recipe_index { + REFINEMENT_RECIPE_BASE_CREF, /* key: cref the modules are activated on */ + REFINEMENT_RECIPE_CREF, /* value: cref with the refinements activated */ + REFINEMENT_RECIPE_SRC_ISEQ, /* key: iseq of the block the Proc came from */ + REFINEMENT_RECIPE_MODS /* key: modules, in the order given */ +}; + +static bool +refinement_recipe_eq(VALUE r1, VALUE r2) +{ + if (r1 == r2) return true; + long len = RARRAY_LEN(r1); + if (RARRAY_LEN(r2) != len) return false; + if (RARRAY_AREF(r1, REFINEMENT_RECIPE_BASE_CREF) != + RARRAY_AREF(r2, REFINEMENT_RECIPE_BASE_CREF)) return false; + if (RARRAY_AREF(r1, REFINEMENT_RECIPE_SRC_ISEQ) != + RARRAY_AREF(r2, REFINEMENT_RECIPE_SRC_ISEQ)) return false; + for (long i = REFINEMENT_RECIPE_MODS; i < len; i++) { + if (RARRAY_AREF(r1, i) != RARRAY_AREF(r2, i)) return false; + } + return true; +} + +VALUE +rb_proc_refinements_recipe(VALUE procval) { rb_proc_t *proc; GetProcPtr(procval, proc); - if (!proc->is_refined) return NULL; - return (const rb_cref_t *)rb_ivar_get(procval, id_refinements_cref); + if (!proc->is_refined) return Qnil; + return rb_ivar_get(procval, id_refinements_recipe); } void -rb_proc_set_refinements_cref(VALUE procval, const rb_cref_t *cref) +rb_proc_set_refinements_recipe(VALUE procval, VALUE recipe) { rb_proc_t *proc; GetProcPtr(procval, proc); - rb_ivar_set(procval, id_refinements_cref, (VALUE)cref); + rb_ivar_set(procval, id_refinements_recipe, recipe); proc->is_refined = 1; } @@ -353,88 +375,50 @@ proc_dup(VALUE self) } rb_cref_t *rb_vm_get_cref(const VALUE *ep); -VALUE rb_proc_dup_with_iseq_and_cref(VALUE self, const rb_iseq_t *iseq, const rb_cref_t *cref); +VALUE rb_proc_dup_with_iseq_and_recipe(VALUE self, const rb_iseq_t *iseq, VALUE recipe); -/* Proc#refined memoizes the most recent {copied iseq, cref} pair per - * source iseq, since rb_iseq_dup_with_independent_caches is expensive. - * The memo lives in a hidden identity Hash (source iseq -> frozen Array): +/* Proc#refined memoizes the most recent recipe copied for a source iseq, with + * its copy. The memo lives in a hidden identity Hash: * - * [base_cref, copied_iseq, cref, mod1, mod2, ...] + * source iseq -> [recipe, copied_iseq] * - * keyed by (base_cref, modules). - * An entry is retained for the VM's lifetime */ + * An entry is written when the copy is made, that is on the first call of a + * Proc built from the recipe, not when Proc#refined is called: a chain of + * calls then memoizes the chain as a whole, since the recipe of the last link + * carries all of the modules. It also means one entry per source iseq is + * enough for prc.refined(a).refined(b), which shares its entry with + * prc.refined(a, b). + * + * An entry is retained for the VM's lifetime, so a block that is itself a copy + * is never used as a key; such a Proc is copied by Proc#refined instead. */ enum refinement_memo_index { - REFINEMENT_MEMO_BASE_CREF, /* key: captured cref of the source proc */ - REFINEMENT_MEMO_COPIED_ISEQ, /* value: copied iseq with independent caches */ - REFINEMENT_MEMO_CREF, /* value: cref with refinements activated */ - REFINEMENT_MEMO_MODS /* key: modules, in argument order */ + REFINEMENT_MEMO_RECIPE, + REFINEMENT_MEMO_COPIED_ISEQ }; static VALUE refinement_memo_map; /* set once under the VM lock */ -static bool -refinement_memo_key_match(VALUE memo, const rb_cref_t *base_cref, long argc, const VALUE *mods) -{ - const VALUE *p = RARRAY_CONST_PTR(memo); - if (p[REFINEMENT_MEMO_BASE_CREF] != (VALUE)base_cref) return false; - if (RARRAY_LEN(memo) - REFINEMENT_MEMO_MODS != argc) return false; - for (long i = 0; i < argc; i++) { - if (p[REFINEMENT_MEMO_MODS + i] != mods[i]) return false; - } - return true; -} - -static bool -refinement_memo_lookup(const rb_iseq_t *src_iseq, const rb_cref_t *base_cref, - long argc, const VALUE *mods, - const rb_iseq_t **iseq_out, const rb_cref_t **cref_out) +static VALUE +refinement_memo_get(const rb_iseq_t *src_iseq) { - VM_ASSERT(ISEQ_BODY(src_iseq)->type == ISEQ_TYPE_BLOCK); VALUE memo = Qnil; RB_VM_LOCKING() { if (refinement_memo_map) { memo = rb_hash_lookup(refinement_memo_map, (VALUE)src_iseq); } } - if (!NIL_P(memo)) { - const VALUE *p = RARRAY_CONST_PTR(memo); - if (refinement_memo_key_match(memo, base_cref, argc, mods)) { - const rb_iseq_t *copied_iseq = (const rb_iseq_t *)p[REFINEMENT_MEMO_COPIED_ISEQ]; - if (ISEQ_BODY(copied_iseq)->param.flags.ruby2_keywords == - ISEQ_BODY(src_iseq)->param.flags.ruby2_keywords) { - *iseq_out = copied_iseq; - *cref_out = (const rb_cref_t *)p[REFINEMENT_MEMO_CREF]; - return true; - } - rb_category_warn( - RB_WARN_CATEGORY_PERFORMANCE, - "Proc#refined re-copies the block because the ruby2_keywords flag changed after the copy was memoized" - ); - return false; - } - rb_category_warn( - RB_WARN_CATEGORY_PERFORMANCE, - "Proc#refined called with different modules for the same block disables memoization" - ); - } - return false; + return memo; } static void -refinement_memo_store(const rb_iseq_t *src_iseq, const rb_cref_t *base_cref, - long argc, const VALUE *mods, - const rb_iseq_t *copied_iseq, const rb_cref_t *cref) +refinement_memo_set(const rb_iseq_t *src_iseq, VALUE recipe, const rb_iseq_t *copied_iseq) { VM_ASSERT(ISEQ_BODY(src_iseq)->type == ISEQ_TYPE_BLOCK); - VALUE memo = rb_ary_hidden_new(REFINEMENT_MEMO_MODS + argc); - rb_ary_push(memo, (VALUE)base_cref); + VALUE memo = rb_ary_hidden_new(2); + rb_ary_push(memo, recipe); rb_ary_push(memo, (VALUE)copied_iseq); - rb_ary_push(memo, (VALUE)cref); - for (long i = 0; i < argc; i++) { - rb_ary_push(memo, mods[i]); - } OBJ_FREEZE(memo); /* Every element is shareable, so mark the memo array shareable too for * reuse from any Ractor. */ @@ -455,9 +439,132 @@ refinement_memo_store(const rb_iseq_t *src_iseq, const rb_cref_t *base_cref, } } +static long +refinement_recipe_modc(VALUE recipe) +{ + return NIL_P(recipe) ? 0 : RARRAY_LEN(recipe) - REFINEMENT_RECIPE_MODS; +} + +static bool +refinement_recipe_match(VALUE recipe, const rb_cref_t *base_cref, VALUE src_recipe, + long argc, const VALUE *mods) +{ + long inherited = refinement_recipe_modc(src_recipe); + if (RARRAY_AREF(recipe, REFINEMENT_RECIPE_BASE_CREF) != (VALUE)base_cref) return false; + if (refinement_recipe_modc(recipe) != inherited + argc) return false; + for (long i = 0; i < inherited; i++) { + if (RARRAY_AREF(recipe, REFINEMENT_RECIPE_MODS + i) != + RARRAY_AREF(src_recipe, REFINEMENT_RECIPE_MODS + i)) return false; + } + for (long i = 0; i < argc; i++) { + if (RARRAY_AREF(recipe, REFINEMENT_RECIPE_MODS + inherited + i) != mods[i]) return false; + } + return true; +} + +static VALUE +refinement_recipe_new(const rb_cref_t *base_cref, const rb_cref_t *cref, + const rb_iseq_t *src_iseq, VALUE src_recipe, + long argc, const VALUE *mods) +{ + long inherited = refinement_recipe_modc(src_recipe); + VALUE recipe = rb_ary_hidden_new(REFINEMENT_RECIPE_MODS + inherited + argc); + rb_ary_push(recipe, (VALUE)base_cref); + rb_ary_push(recipe, (VALUE)cref); + rb_ary_push(recipe, (VALUE)src_iseq); + for (long i = 0; i < inherited; i++) { + rb_ary_push(recipe, RARRAY_AREF(src_recipe, REFINEMENT_RECIPE_MODS + i)); + } + for (long i = 0; i < argc; i++) { + rb_ary_push(recipe, mods[i]); + } + OBJ_FREEZE(recipe); + RB_OBJ_SET_SHAREABLE(recipe); + return recipe; +} + +static VALUE +refinement_memo_lookup(const rb_iseq_t *src_iseq, const rb_cref_t *base_cref, VALUE src_recipe, + long argc, const VALUE *mods) +{ + VM_ASSERT(ISEQ_BODY(src_iseq)->type == ISEQ_TYPE_BLOCK); + VALUE memo = refinement_memo_get(src_iseq); + if (NIL_P(memo)) return Qnil; + VALUE recipe = RARRAY_AREF(memo, REFINEMENT_MEMO_RECIPE); + if (!refinement_recipe_match(recipe, base_cref, src_recipe, argc, mods)) return Qnil; + return recipe; +} + +static const rb_iseq_t * +refinement_iseq_copy(VALUE recipe) +{ + const rb_iseq_t *src_iseq = + (const rb_iseq_t *)RARRAY_AREF(recipe, REFINEMENT_RECIPE_SRC_ISEQ); + VALUE memo = refinement_memo_get(src_iseq); + if (!NIL_P(memo)) { + if (refinement_recipe_eq(RARRAY_AREF(memo, REFINEMENT_MEMO_RECIPE), recipe)) { + const rb_iseq_t *copied_iseq = + (const rb_iseq_t *)RARRAY_AREF(memo, REFINEMENT_MEMO_COPIED_ISEQ); + if (ISEQ_BODY(copied_iseq)->param.flags.ruby2_keywords == + ISEQ_BODY(src_iseq)->param.flags.ruby2_keywords) { + return copied_iseq; + } + rb_category_warn( + RB_WARN_CATEGORY_PERFORMANCE, + "Proc#refined re-copies the block because the ruby2_keywords flag changed after the copy was memoized" + ); + } + else { + rb_category_warn( + RB_WARN_CATEGORY_PERFORMANCE, + "Proc#refined called with different modules for the same block disables memoization" + ); + } + } + + /* copy outside the lock; losing a race just discards the extra copy */ + const rb_iseq_t *copied_iseq = rb_iseq_dup_with_independent_caches(src_iseq); + refinement_memo_set(src_iseq, recipe, copied_iseq); + return copied_iseq; +} + +NOINLINE(static void refinement_iseq_install(VALUE procval, rb_proc_t *proc)); +static void +refinement_iseq_install(VALUE procval, rb_proc_t *proc) +{ + VALUE recipe = rb_ivar_get(procval, id_refinements_recipe); + const rb_iseq_t *copied_iseq = refinement_iseq_copy(recipe); + + RB_VM_LOCKING() { + if (!FL_TEST_RAW((VALUE)proc->block.as.captured.code.iseq, ISEQ_REFINED_COPY)) { + RB_OBJ_WRITE(procval, &proc->block.as.captured.code.val, (VALUE)copied_iseq); + } + } +} + +static inline void +refinement_iseq_ensure(VALUE procval, rb_proc_t *proc) +{ + if (UNLIKELY(!FL_TEST_RAW((VALUE)proc->block.as.captured.code.iseq, ISEQ_REFINED_COPY))) { + refinement_iseq_install(procval, proc); + } +} + +const rb_cref_t * +rb_proc_refinements_cref_for_call(VALUE procval) +{ + rb_proc_t *proc; + GetProcPtr(procval, proc); + if (!proc->is_refined) return NULL; + + refinement_iseq_ensure(procval, proc); + VALUE recipe = rb_ivar_get(procval, id_refinements_recipe); + return (const rb_cref_t *)RARRAY_AREF(recipe, REFINEMENT_RECIPE_CREF); +} + /* * call-seq: - * prc.refined(mod, ...) -> a_proc + * prc.refined(*modules) -> a_proc * * Returns a new Proc that behaves like the receiver but with the refinements * activated by the given modules in effect inside its body. The receiver is @@ -474,14 +581,13 @@ refinement_memo_store(const rb_iseq_t *src_iseq, const rb_cref_t *base_cref, * refined_proc.call("hi") #=> "HI!" * original.call("hi") #=> NoMethodError * - * Only Procs created from a Ruby block are supported; calling this on a Proc - * backed by a C function, a Symbol, or a method raises ArgumentError. + * If no modules are given, returns the receiver. + * Otherwise, only Procs created from a Ruby block are supported; calling this + * on a Proc backed by a C function, a Symbol, or a method raises ArgumentError. * - * Calling this method on a Proc that already has refinements applied by this - * method also raises ArgumentError. To activate the refinements of multiple - * modules, pass them all in a single call: - * - * refined_proc = original.refined(StringRefinement, OtherRefinement) + * When calls of this method are chained, all the given modules are activated + * in the order they are given, so refinements activated by a later call take + * precedence. * * The refinement set of the returned Proc is fixed when it is created: * calling +using+ inside its body raises RuntimeError. @@ -500,11 +606,14 @@ refinement_memo_store(const rb_iseq_t *src_iseq, const rb_cref_t *base_cref, * obj.shout_hi #=> "HI!" * }.refined(StringRefinement) * - * This method copies the instruction sequence of the block and of all of its - * nested blocks so that the copy can resolve methods through the refinements - * without affecting the original Proc. Applying refinements therefore - * increases memory use roughly in proportion to the size of the block. The - * copy is cached and reused for the same block and the same modules. + * Running the returned Proc requires a copy of the instruction sequence of the + * block and of all of its nested blocks, so that the copy can resolve methods + * through the refinements without affecting the original Proc. The copy is + * made when the Proc is first called, and is cached and reused for the same + * block and the same modules, whether they were given in one call or in a + * chain of calls; a Proc that is never called is never copied. Applying + * refinements therefore increases memory use roughly in proportion to the size + * of the block, once the Proc runs. */ static VALUE proc_refined(int argc, VALUE *argv, VALUE self) @@ -512,28 +621,46 @@ proc_refined(int argc, VALUE *argv, VALUE self) rb_proc_t *src; GetProcPtr(self, src); - rb_check_arity(argc, 1, UNLIMITED_ARGUMENTS); + if (argc == 0) { + return self; + } if (vm_block_type(&src->block) != block_type_iseq || src->is_from_method) { rb_raise(rb_eArgError, "can't apply refinements to a Proc without a Ruby block"); } - if (src->is_refined) { - rb_raise(rb_eArgError, "can't apply refinements to a Proc that already has refinements"); - } - for (int i = 0; i < argc; i++) { Check_Type(argv[i], T_MODULE); } - const rb_cref_t *base_cref = rb_vm_get_cref(src->block.as.captured.ep); const rb_iseq_t *src_iseq = src->block.as.captured.code.iseq; + VALUE src_recipe = rb_proc_refinements_recipe(self); + const rb_cref_t *src_cref, *base_cref; + if (NIL_P(src_recipe)) { + src_cref = base_cref = rb_vm_get_cref(src->block.as.captured.ep); + } + else { + /* keep asking for the modules of the whole chain, so that a chained + * call ends up with the recipe of a single call of all of them */ + src_cref = (const rb_cref_t *)RARRAY_AREF(src_recipe, REFINEMENT_RECIPE_CREF); + base_cref = (const rb_cref_t *)RARRAY_AREF(src_recipe, REFINEMENT_RECIPE_BASE_CREF); + } + + /* A block that is itself a copy is short-lived, so it is not memoized, and + * it has to be copied here: ISEQ_REFINED_COPY has to keep meaning "the + * copy of this Proc". */ + bool copied_src = FL_TEST_RAW((VALUE)src_iseq, ISEQ_REFINED_COPY); + if (copied_src) { + rb_category_warn( + RB_WARN_CATEGORY_PERFORMANCE, + "Proc#refined on a Proc whose block was already copied by Proc#refined is not memoized" + ); + } - const rb_iseq_t *new_iseq; - const rb_cref_t *new_cref; - if (!refinement_memo_lookup(src_iseq, base_cref, argc, argv, &new_iseq, &new_cref)) { - new_iseq = rb_iseq_dup_with_independent_caches(src_iseq); - rb_cref_t *cref = rb_vm_cref_dup(base_cref); + VALUE recipe = copied_src ? Qnil : + refinement_memo_lookup(src_iseq, base_cref, src_recipe, argc, argv); + if (NIL_P(recipe)) { + rb_cref_t *cref = rb_vm_cref_dup(src_cref); /* rb_using_module_recursive modifies shared subclass lists */ RB_VM_LOCKING() { for (int i = 0; i < argc; i++) { @@ -549,11 +676,13 @@ proc_refined(int argc, VALUE *argv, VALUE self) } CREF_OMOD_SHARED_SET(cref); CREF_REFINED_PROC_SET(cref); - new_cref = cref; - refinement_memo_store(src_iseq, base_cref, argc, argv, new_iseq, new_cref); + recipe = refinement_recipe_new(base_cref, cref, src_iseq, src_recipe, argc, argv); } - return rb_proc_dup_with_iseq_and_cref(self, new_iseq, new_cref); + const rb_iseq_t *new_iseq = copied_src ? + rb_iseq_dup_with_independent_caches(src_iseq) : src_iseq; + + return rb_proc_dup_with_iseq_and_recipe(self, new_iseq, recipe); } /* @@ -1569,7 +1698,7 @@ rb_proc_call_kw(VALUE self, VALUE args, int kw_splat) GetProcPtr(self, proc); vret = rb_vm_invoke_proc(GET_EC(), proc, argc, argv, kw_splat, VM_BLOCK_HANDLER_NONE, - rb_proc_refinements_cref(self)); + rb_proc_refinements_cref_for_call(self)); RB_GC_GUARD(self); RB_GC_GUARD(args); return vret; @@ -1595,7 +1724,7 @@ rb_proc_call_with_block_kw(VALUE self, int argc, const VALUE *argv, VALUE passed rb_proc_t *proc; GetProcPtr(self, proc); vret = rb_vm_invoke_proc(ec, proc, argc, argv, kw_splat, proc_to_block_handler(passed_procval), - rb_proc_refinements_cref(self)); + rb_proc_refinements_cref_for_call(self)); RB_GC_GUARD(self); return vret; } @@ -1899,7 +2028,8 @@ proc_eq(VALUE self, VALUE other) GetProcPtr(other, other_proc); if (self_proc->is_from_method != other_proc->is_from_method || - self_proc->is_lambda != other_proc->is_lambda) { + self_proc->is_lambda != other_proc->is_lambda || + self_proc->is_refined != other_proc->is_refined) { return Qfalse; } @@ -1913,8 +2043,18 @@ proc_eq(VALUE self, VALUE other) switch (vm_block_type(self_block)) { case block_type_iseq: if (self_block->as.captured.ep != \ - other_block->as.captured.ep || - self_block->as.captured.code.iseq != \ + other_block->as.captured.ep) { + return Qfalse; + } + /* a refined Proc's block iseq flips from the source to the copy on + * the first call; compare what the Procs were built from instead */ + if (self_proc->is_refined) { + if (!refinement_recipe_eq(rb_proc_refinements_recipe(self), + rb_proc_refinements_recipe(other))) { + return Qfalse; + } + } + else if (self_block->as.captured.code.iseq != \ other_block->as.captured.code.iseq) { return Qfalse; } @@ -2074,7 +2214,20 @@ rb_hash_proc(st_index_t hash, VALUE prc) switch (vm_block_type(&proc->block)) { case block_type_iseq: - hash = rb_st_hash_uint(hash, (st_index_t)proc->block.as.captured.code.iseq->body); + if (proc->is_refined) { + /* from the recipe, not the block iseq: the latter flips from the + * source to the copy on the first call, and the hash must not */ + VALUE recipe = rb_proc_refinements_recipe(prc); + long len = RARRAY_LEN(recipe); + hash = rb_st_hash_uint(hash, (st_index_t)RARRAY_AREF(recipe, REFINEMENT_RECIPE_BASE_CREF)); + hash = rb_st_hash_uint(hash, (st_index_t)((const rb_iseq_t *)RARRAY_AREF(recipe, REFINEMENT_RECIPE_SRC_ISEQ))->body); + for (long i = REFINEMENT_RECIPE_MODS; i < len; i++) { + hash = rb_st_hash_uint(hash, (st_index_t)RARRAY_AREF(recipe, i)); + } + } + else { + hash = rb_st_hash_uint(hash, (st_index_t)proc->block.as.captured.code.iseq->body); + } break; case block_type_ifunc: hash = rb_st_hash_uint(hash, (st_index_t)proc->block.as.captured.code.ifunc->func); @@ -4683,7 +4836,20 @@ proc_ruby2_keywords(VALUE procval) !ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_post && !ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_kw && !ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.has_kwrest) { - ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.ruby2_keywords = 1; + if (proc->is_refined) { + /* on a copy of this Proc's own: the block is shared with the + * source Proc until the first call, and the copy installed by + * it may be memoized and shared with sibling Procs */ + const rb_iseq_t *copy = + rb_iseq_dup_with_independent_caches(proc->block.as.captured.code.iseq); + ISEQ_BODY(copy)->param.flags.ruby2_keywords = 1; + RB_VM_LOCKING() { + RB_OBJ_WRITE(procval, &proc->block.as.captured.code.val, (VALUE)copy); + } + } + else { + ISEQ_BODY(proc->block.as.captured.code.iseq)->param.flags.ruby2_keywords = 1; + } } else { rb_warn("Skipping set of ruby2_keywords flag for proc (proc accepts keywords or post arguments or proc does not accept argument splat)"); @@ -5129,7 +5295,7 @@ void Init_Proc(void) { #undef rb_intern - id_refinements_cref = rb_make_internal_id(); + id_refinements_recipe = rb_make_internal_id(); VALUE mRuby = rb_define_module("Ruby"); diff --git a/spec/ruby/core/proc/refined_spec.rb b/spec/ruby/core/proc/refined_spec.rb index 989a9fd4b7d23c..36b787b28b1c2e 100644 --- a/spec/ruby/core/proc/refined_spec.rb +++ b/spec/ruby/core/proc/refined_spec.rb @@ -125,8 +125,10 @@ def shout_hi Class.new.class_eval(&refined).should == "HI!" end - it "raises ArgumentError when called with no modules" do - -> { -> {}.refined }.should.raise(ArgumentError) + it "returns the receiver when called with no modules" do + original = -> {} + refined = original.refined + refined.should.equal?(original) end it "raises TypeError when called with a non-Module argument" do @@ -140,17 +142,49 @@ def shout_hi -> { method_proc.refined(ProcRefinedSpecs::StringShout) }.should.raise(ArgumentError) end - it "raises ArgumentError for a Proc that already has refinements applied" do - refined = -> s { s.shout }.refined(ProcRefinedSpecs::StringShout) - -> { refined.refined(ProcRefinedSpecs::StringQuiet) }.should.raise(ArgumentError) + it "activates the refinements of all the given modules when chained" do + pr = -> s { [s.shout, s.quiet] } + refined = pr.refined(ProcRefinedSpecs::StringShout).refined(ProcRefinedSpecs::StringQuiet) + refined.call("Hi").should == ["hi", "..."] + end + + it "gives precedence to the module applied last when chained" do + pr = -> s { s.shout } + pr.refined(ProcRefinedSpecs::StringShout).refined(ProcRefinedSpecs::StringQuiet).call("Hi").should == "hi" + pr.refined(ProcRefinedSpecs::StringQuiet).refined(ProcRefinedSpecs::StringShout).call("Hi").should == "HI!" end it "keeps the refinements on dup and clone" do refined = -> s { s.shout }.refined(ProcRefinedSpecs::StringShout) refined.dup.call("hi").should == "HI!" refined.clone.call("hi").should == "HI!" - -> { refined.dup.refined(ProcRefinedSpecs::StringQuiet) }.should.raise(ArgumentError) - -> { refined.clone.refined(ProcRefinedSpecs::StringQuiet) }.should.raise(ArgumentError) + end + + it "returns a Proc that is not equal to the receiver" do + pr = -> s { s.shout } + refined = pr.refined(ProcRefinedSpecs::StringShout) + refined.should_not == pr + refined.should_not.eql?(pr) + refined.call("hi") + refined.should_not == pr + end + + it "returns Procs that are not equal for different modules" do + pr = -> s { s.shout } + r1 = pr.refined(ProcRefinedSpecs::StringShout) + r2 = pr.refined(ProcRefinedSpecs::StringQuiet) + r1.should_not == r2 + end + + it "keeps its hash and equality when first called, so it stays usable as a Hash key" do + pr = -> s { s.shout } + refined = pr.refined(ProcRefinedSpecs::StringShout) + h = { pr => :source, refined => :refined } + h.size.should == 2 + hash_before = refined.hash + refined.call("hi") + refined.hash.should == hash_before + h[refined].should == :refined end it "raises ArgumentError when the result is passed to define_method" do diff --git a/test/ruby/test_proc.rb b/test/ruby/test_proc.rb index 1caf8a41032775..6eba9735b17a64 100644 --- a/test/ruby/test_proc.rb +++ b/test/ruby/test_proc.rb @@ -607,8 +607,12 @@ def test_refined_preserved_by_dup assert_equal("Z!", refined.clone.call("z")) end + def test_refined_no_arguments + original = -> {} + assert_same(original, original.refined) + end + def test_refined_errors - assert_raise(ArgumentError) { ->(s) { s }.refined } assert_raise(TypeError) { ->(s) { s }.refined(42) } # non-iseq Procs are not supported assert_raise(ArgumentError) { :upcase.to_proc.refined(RefinementsModule) } @@ -649,6 +653,21 @@ module RefHolder RUBY end + def test_refined_shareable_refined_proc_first_called_in_ractor + assert_separately([], <<~'RUBY') + Warning[:experimental] = false + module RefMod; refine(String) { def shout = upcase + "!" }; end + module RefHolder + REFINED = Ractor.make_shareable(->(s) { s.shout }.refined(RefMod)) + end + refined = RefHolder::REFINED + # the first call, and so the deferred copy, happens in another Ractor + r = Ractor.new(refined) { |pr| pr.call("hi") } + assert_equal("HI!", r.value) + assert_equal("YO!", refined.call("yo")) + RUBY + end + def test_refined_coverage assert_separately(%w[-rcoverage -rtempfile], <<~'RUBY') f = Tempfile.open(["refined_coverage", ".rb"]) @@ -674,18 +693,6 @@ def shout = upcase + "!" RUBY end - def test_refined_chain_rejected - # Chaining would need merge-or-replace semantics for the refinement sets; - # both are confusing, so a refined proc rejects further refined. - # Multiple modules can be activated by passing them in a single call. - refined = ->(s) { s.shout }.refined(RefinementsModule) - assert_raise(ArgumentError) { refined.refined(RefinementsModule2) } - # the refinement state survives dup, so the dup is rejected too - assert_raise(ArgumentError) { refined.dup.refined(RefinementsModule2) } - # the receiver remains usable - assert_equal("HI!", refined.call("hi")) - end - def test_refined_using_in_body_rejected # The refinement set of a refined proc is fixed at refined() time: procs # derived from the same source and modules share the copied iseq (and its @@ -737,13 +744,7 @@ def doubled = self * 2 end end - def test_refined_nested_proc_is_not_a_chain - # A Proc created lexically INSIDE a refined Proc is not itself "a - # Proc that already has refinements": it only inherits the enclosing - # refinements lexically. refined (and define_method) must therefore - # be accepted on it, and the inner Proc must see both the enclosing - # refinement and the one it adds. Only the Proc returned by refined - # is rejected for chaining. + def test_refined_nested_proc result = -> { inner = ->(s, n) { [s.shout, n.doubled] } inner.refined(RefinementsStringOnly).call("hi", 3) @@ -759,6 +760,75 @@ def test_refined_nested_proc_is_not_a_chain end end + def test_refined_chain + refined = ->(s, n) { [s.shout, n.doubled] }.refined(RefinementsStringOnly).refined(RefinementsIntegerOnly) + assert_equal(["HI!", 6], refined.call("hi", 3)) + + refined2 = ->(s) { s.shout }.refined(RefinementsModule).refined(RefinementsModule2) + assert_equal("?", refined2.call("hi")) + refined3 = ->(s) { s.shout }.refined(RefinementsModule2).refined(RefinementsModule) + assert_equal("HI!", refined3.call("hi")) + end + + def test_refined_chain_after_call + # The block of a Proc that has already run is the copy it is running, so + # chaining from it must not hand that copy to the new Proc: the two have + # different refinements for the same method. + pr = ->(s) { s.shout } + p1 = pr.refined(RefinementsModule) + assert_equal("HI!", p1.call("hi")) + p2 = p1.refined(RefinementsModule2) + assert_equal("?", p2.call("hi")) + assert_equal("HI!", p1.call("hi")) + end + + def test_refined_inner_proc_after_call + # Likewise for a Proc created inside a refined Proc: its block is part of + # the enclosing copy. + outer = -> { + inner = ->(s) { s.shout } + [inner.refined(RefinementsModule2).call("hi"), inner.call("hi")] + }.refined(RefinementsModule) + assert_equal(["?", "HI!"], outer.call) + end + + def test_refined_eq_and_hash + # Equality and hash come from what the Proc was built from (block, captured + # cref, modules), so they do not depend on whether the deferred copy has + # been made yet, and never alias a refined Proc with its source. + prc = ->(s) { s.shout } + rp = prc.refined(RefinementsModule) + rq = prc.refined(RefinementsModule2) + rr = prc.refined(RefinementsModule) + assert_not_equal(prc, rp) + assert_not_equal(rp, rq) + assert_equal(rp, rr) + assert_equal(rp.hash, rr.hash) + hash_before = rp.hash + rp.call("hi") + assert_equal(hash_before, rp.hash, "hash must not change on the first call") + assert_equal(rp, rr, "equality must not change on the first call") + assert_not_equal(prc, rp) + h = { prc => 1, rp => 2 } + assert_equal(2, h.size) + assert_equal(2, h[rr]) + end + + def test_refined_first_call_error_in_fiber + # The deferred copy runs inside the fiber's tag: an exception raised there + # (here from a Warning.warn override) must come out of Fiber#resume. + assert_separately([], <<~'RUBY') + module M1; refine(String) { def shout = "1" }; end + module M2; refine(String) { def shout = "2" }; end + Warning[:performance] = true + def Warning.warn(msg, category: nil) = raise "boom" + pr = ->(s) { s.shout } + pr.refined(M1).call("hi") + q = pr.refined(M2) # the first call will warn about the memo miss + assert_raise_with_message(RuntimeError, "boom") { Fiber.new(&q).resume("hi") } + RUBY + end + def test_refined_gc assert_normal_exit(<<~RUBY) module M @@ -822,12 +892,24 @@ def test_refined_memoized assert_equal("HI!", orig.refined(RefinementsModule).call("hi")) end + def test_refined_memo_replaced_before_first_call + # The copy of the block is made on the first call, out of the memo entry + # that produced the proc's refinements. A proc whose entry has since been + # replaced by another module set makes its own copy instead. + orig = ->(s) { s.shout } + q1 = orig.refined(RefinementsModule) + q2 = orig.refined(RefinementsModule2) # replaces the memo entry + assert_equal("?", q2.call("hi")) + assert_equal("HI!", q1.call("hi")) + end + def test_refined_ruby2_keywords_memo # Proc#ruby2_keywords marks the shared block iseq, possibly after a copy # was memoized. The stale memo is rebuilt (with a warning naming the # cause) rather than reused or mutated: the new proc delegates keywords - # like its source, while procs built before the mark keep their - # creation-time behavior. + # like its source, while a proc already running a copy keeps it. A proc + # that has not been called yet has no copy of its own, so it picks the mark + # up like any other proc made from the same block. assert_separately([], <<~'RUBY') module M; refine(String) { def shout = upcase + "!" }; end Warning[:performance] = true @@ -835,13 +917,14 @@ module M; refine(String) { def shout = upcase + "!" }; end def Warning.warn(msg, category: nil) = $warned << msg target = ->(a, k: nil) { [a, k] } pr = proc { |*args| target.call(*args) } - q1 = pr.refined(M) # memoize a copy before the mark + q1 = pr.refined(M) + assert_raise(ArgumentError) { q1.call(1, k: 2) } # memoizes a copy before the mark pr.ruby2_keywords assert_equal([1, 2], pr.call(1, k: 2)) q2 = pr.refined(M) assert_equal([1, 2], q2.call(1, k: 2)) assert_equal(1, $warned.grep(/ruby2_keywords/).size) - # the copy made before the mark is not retroactively changed + # the copy q1 is running is not retroactively changed assert_raise(ArgumentError) { q1.call(1, k: 2) } # the rebuilt memo is hit from now on; no further warnings assert_equal([1, 2], pr.refined(M).call(1, k: 2)) @@ -849,6 +932,36 @@ def Warning.warn(msg, category: nil) = $warned << msg RUBY end + def test_refined_ruby2_keywords_does_not_leak_to_siblings + # Proc#ruby2_keywords on a refined Proc marks a copy of its own, since its + # block may be the memoized copy shared with sibling Procs, or, before the + # first call, the source block itself. + assert_separately([], <<~'RUBY') + module M; refine(String) { def shout = upcase + "!" }; end + Warning[:performance] = true + $warned = [] + def Warning.warn(msg, category: nil) = $warned << msg + target = ->(a, k: nil) { [a, k] } + pr = proc { |*args| target.call(*args) } + q1 = pr.refined(M) + q2 = pr.refined(M) + q1.call(1); q2.call(1) # both run the memoized copy + q1.ruby2_keywords + assert_equal([1, 2], q1.call(1, k: 2)) + assert_raise(ArgumentError) { q2.call(1, k: 2) } + assert_raise(ArgumentError) { pr.call(1, k: 2) } + # the memoized copy is untouched, so new procs neither warn nor delegate + $warned.clear + assert_raise(ArgumentError) { pr.refined(M).call(1, k: 2) } + assert_equal([], $warned) + # likewise before the first call: the mark must not reach the source + r1 = pr.refined(M) + r1.ruby2_keywords + assert_equal([1, 2], r1.call(1, k: 2)) + assert_raise(ArgumentError) { pr.call(1, k: 2) } + RUBY + end + def test_refined_memo_distinct_environments # Procs sharing the same block iseq but capturing different closure # environments hit the same memo entry (env is not part of the key), yet each @@ -866,16 +979,16 @@ def test_refined_memo_distinct_environments def test_refined_memo_avoids_recopy orig = ->(s) { s.shout } - orig.refined(RefinementsModule) # warm the memo + orig.refined(RefinementsModule).call("hi") # warm the memo GC.disable begin before = GC.stat(:total_allocated_objects) - 100.times { orig.refined(RefinementsModule) } + 100.times { orig.refined(RefinementsModule).call("hi") } hits = GC.stat(:total_allocated_objects) - before before = GC.stat(:total_allocated_objects) 100.times do |i| - orig.refined(i.even? ? RefinementsModule : RefinementsModule2) + orig.refined(i.even? ? RefinementsModule : RefinementsModule2).call("hi") end misses = GC.stat(:total_allocated_objects) - before ensure @@ -894,9 +1007,66 @@ module M2; refine(String) { def shout = "2" }; end $warned = [] def Warning.warn(msg, category: nil) = $warned << msg pr = ->(s) { s.shout } - pr.refined(M1) - pr.refined(M2) # evicts the M1 entry + pr.refined(M1).call("hi") + pr.refined(M2).call("hi") # evicts the M1 entry assert_equal(1, $warned.grep(/different modules/).size) + # nothing is memoized until the copy is made, so creating the procs + # without calling them warns about nothing + $warned.clear + pr.refined(M1) + pr.refined(M2) + assert_equal([], $warned) + RUBY + end + + def test_refined_memo_shared_by_procs_built_before_first_call + # Procs built before any of them ran hold equal but distinct recipes; the + # first call must still share one copy among them, without the + # different-modules warning. + assert_separately([], <<~'RUBY') + module M1; refine(String) { def shout = "1" }; end + Warning[:performance] = true + $warned = [] + def Warning.warn(msg, category: nil) = $warned << msg + pr = ->(s) { s.shout } + procs = 5.times.map { pr.refined(M1) } + procs.each { |q| assert_equal("1", q.call("hi")) } + assert_equal([], $warned) + RUBY + end + + def test_refined_chain_memoized + # A chain is memoized as a whole: the recipe of the last link carries the + # modules of all of them, so it shares its memo entry with a single call of + # the same modules, and repeating the chain hits it. + assert_separately([], <<~'RUBY') + module M1; refine(String) { def shout = upcase }; end + module M2; refine(Integer) { def dbl = self * 2 }; end + Warning[:performance] = true + $warned = [] + def Warning.warn(msg, category: nil) = $warned << msg + pr = ->(s, i) { [s.shout, i.dbl] } + 3.times { assert_equal(["A", 2], pr.refined(M1).refined(M2).call("a", 1)) } + assert_equal(["A", 2], pr.refined(M1, M2).call("a", 1)) + assert_equal([], $warned) + RUBY + end + + def test_refined_chain_warning + # Only a block that is already a copy is left out of the memo. + assert_separately([], <<~'RUBY') + module M1; refine(String) { def shout = "1" }; end + module M2; refine(String) { def shout = "2" }; end + Warning[:performance] = true + $warned = [] + def Warning.warn(msg, category: nil) = $warned << msg + pr = ->(s) { s.shout } + p1 = pr.refined(M1) + p1.refined(M2) + assert_equal([], $warned) + p1.call("hi") # p1 now runs its copy + assert_equal("2", p1.refined(M2).call("hi")) + assert_equal(1, $warned.grep(/already copied/).size) RUBY end @@ -1061,8 +1231,6 @@ def test_refined_preserves_lambda def test_refined_preserved_by_clone refined = ->(s) { s.shout }.refined(RefinementsModule) assert_equal("Z!", refined.clone.call("z")) - # the refinement state survives clone, so chaining on the clone is rejected too - assert_raise(ArgumentError) { refined.clone.refined(RefinementsModule2) } end def test_refined_module_precedence diff --git a/thread.c b/thread.c index ea9f2823953080..5f84c0399418da 100644 --- a/thread.c +++ b/thread.c @@ -601,7 +601,7 @@ thread_do_start_proc(rb_thread_t *th) VALUE procval = th->invoke_arg.proc.proc; rb_proc_t *proc; GetProcPtr(procval, proc); - const rb_cref_t *cref = rb_proc_refinements_cref(procval); + const rb_cref_t *cref = rb_proc_refinements_cref_for_call(procval); th->ec->errinfo = Qnil; th->ec->root_lep = rb_vm_proc_local_ep(procval); diff --git a/vm.c b/vm.c index 59b5e01f2b3d3e..1be43c67a0afb4 100644 --- a/vm.c +++ b/vm.c @@ -1371,15 +1371,16 @@ VALUE rb_proc_dup(VALUE self) { VALUE procval = rb_proc_dup_0(self); - const rb_cref_t *cref = rb_proc_refinements_cref(self); - if (cref) rb_proc_set_refinements_cref(procval, cref); + VALUE recipe = rb_proc_refinements_recipe(self); + if (!NIL_P(recipe)) rb_proc_set_refinements_recipe(procval, recipe); return procval; } -/* Proc#refined: build a Proc that runs `iseq` (a copy of self's block iseq) - * with `cref` as its refinement cref, sharing self's environment. */ +/* Proc#refined: build a Proc that runs `iseq` with the refinements of + * `recipe`, sharing self's environment. `iseq` is normally self's own block + * iseq, which the copy replaces on the first call. */ VALUE -rb_proc_dup_with_iseq_and_cref(VALUE self, const rb_iseq_t *iseq, const rb_cref_t *cref) +rb_proc_dup_with_iseq_and_recipe(VALUE self, const rb_iseq_t *iseq, VALUE recipe) { rb_proc_t *src; GetProcPtr(self, src); @@ -1389,7 +1390,7 @@ rb_proc_dup_with_iseq_and_cref(VALUE self, const rb_iseq_t *iseq, const rb_cref_ block.as.captured.code.iseq = iseq; VALUE procval = proc_create(rb_obj_class(self), &block, src->is_from_method, src->is_lambda); - rb_proc_set_refinements_cref(procval, cref); + rb_proc_set_refinements_recipe(procval, recipe); RB_GC_GUARD(self); return procval; @@ -1881,7 +1882,7 @@ invoke_block_from_c_bh(rb_execution_context_t *ec, VALUE block_handler, VALUE procval = VM_BH_TO_PROC(block_handler); rb_proc_t *po; GetProcPtr(procval, po); - if (po->is_refined) cref = rb_proc_refinements_cref(procval); + if (po->is_refined) cref = rb_proc_refinements_cref_for_call(procval); if (force_blockarg == FALSE) { is_lambda = po->is_lambda; } diff --git a/vm_core.h b/vm_core.h index 02c366e5f3ec07..75907eabd6aa33 100644 --- a/vm_core.h +++ b/vm_core.h @@ -1331,10 +1331,14 @@ typedef struct { unsigned int is_refined: 1; /* bool: Proc#refined */ } rb_proc_t; -/* A refined proc's cref lives in a hidden ivar on the proc object; - * rb_proc_refinements_cref returns NULL unless is_refined is set. */ -const rb_cref_t *rb_proc_refinements_cref(VALUE procval); -void rb_proc_set_refinements_cref(VALUE procval, const rb_cref_t *cref); +/* A refined proc's refinements recipe (see Proc#refined) lives in a hidden + * ivar on the proc object; the accessors return nil/NULL unless is_refined is + * set. rb_proc_refinements_cref_for_call also makes the copy of the block + * that Proc#refined defers until the first call, so it can raise and must not + * be called outside a tag. */ +VALUE rb_proc_refinements_recipe(VALUE procval); +void rb_proc_set_refinements_recipe(VALUE procval, VALUE recipe); +const rb_cref_t *rb_proc_refinements_cref_for_call(VALUE procval); RUBY_SYMBOL_EXPORT_BEGIN VALUE rb_proc_isolate(VALUE self); diff --git a/vm_eval.c b/vm_eval.c index f1c8ba88b00b35..372a9549d58e42 100644 --- a/vm_eval.c +++ b/vm_eval.c @@ -291,7 +291,7 @@ vm_call0_body(rb_execution_context_t *ec, struct rb_calling_info *calling, const rb_proc_t *proc; GetProcPtr(calling->recv, proc); ret = rb_vm_invoke_proc(ec, proc, calling->argc, argv, calling->kw_splat, calling->block_handler, - rb_proc_refinements_cref(calling->recv)); + rb_proc_refinements_cref_for_call(calling->recv)); goto success; } case OPTIMIZED_METHOD_TYPE_STRUCT_AREF: @@ -2240,7 +2240,7 @@ yield_under(VALUE self, int singleton, int argc, const VALUE *argv, int kw_splat rb_proc_t *po; GetProcPtr(procval, po); is_lambda = po->is_lambda; - if (po->is_refined) proc_cref = rb_proc_refinements_cref(procval); + if (po->is_refined) proc_cref = rb_proc_refinements_cref_for_call(procval); block_handler = vm_block_to_block_handler(&po->block); } goto again; diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 48e7900b6cab7f..43bdfadb9354dd 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -5427,7 +5427,7 @@ vm_invoke_proc_block_with_cref(rb_execution_context_t *ec, rb_control_frame_t *r struct rb_calling_info *calling, const struct rb_callinfo *ci, bool is_lambda, VALUE block_handler, VALUE refined_procval) { - const rb_cref_t *cref = rb_proc_refinements_cref(refined_procval); + const rb_cref_t *cref = rb_proc_refinements_cref_for_call(refined_procval); return vm_invoke_iseq_block_with_cref(ec, reg_cfp, calling, ci, is_lambda, block_handler, cref); } From 1ba809e748e04f03450a7f2148e5e860c64c57f0 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 31 Jul 2026 10:28:13 +0200 Subject: [PATCH 19/30] C API: Expose `rb_iseq_load_from_binary` [Feature #22222] This allows to efficiently load iseq from mmaped files and other buffers without having to copy the bytes into a Ruby string an then invoke `RubyVM::InstructionSequence.load_from_binary`. --- ext/-test-/eval/eval.c | 7 +++++++ include/ruby/internal/eval.h | 15 +++++++++++++++ iseq.c | 8 +++++++- test/-ext-/eval/test_iseq_load.rb | 10 ++++++++++ 4 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/-ext-/eval/test_iseq_load.rb diff --git a/ext/-test-/eval/eval.c b/ext/-test-/eval/eval.c index 983468fc347c7d..f6bea17ea980ba 100644 --- a/ext/-test-/eval/eval.c +++ b/ext/-test-/eval/eval.c @@ -6,8 +6,15 @@ eval_string(VALUE self, VALUE str) return rb_eval_string(StringValueCStr(str)); } +static VALUE +iseq_load_from_binary(VALUE self, VALUE str) +{ + return rb_iseq_load_from_binary(RSTRING_PTR(str), RSTRING_LEN(str)); +} + void Init_eval(void) { rb_define_global_function("rb_eval_string", eval_string, 1); + rb_define_global_function("rb_iseq_load_from_binary", iseq_load_from_binary, 1); } diff --git a/include/ruby/internal/eval.h b/include/ruby/internal/eval.h index 23aa1d958076fe..f9fa4e5465f736 100644 --- a/include/ruby/internal/eval.h +++ b/include/ruby/internal/eval.h @@ -400,6 +400,21 @@ RBIMPL_ATTR_NONNULL(()) */ VALUE rb_extract_keywords(VALUE *orighash); +/** + * Load an iseq object from binary format String object + * created by RubyVM::InstructionSequence.to_binary. + * + * @warning This loader does not have a verifier, so that loading broken/modified + * binary causes critical problem. + * @warning You should not load binary data provided by others. + * You should only use binary data translated by yourself. + * @param[in] ptr A memory region of `len` bytes length. + * @param[in] len Length of `ptr`, in bytes, not including the + * optional terminating NUL character. + * @return An instance of RubyVM::InstructionSequence. + */ +VALUE rb_iseq_load_from_binary(const char *ptr, size_t len); + RBIMPL_SYMBOL_EXPORT_END() #endif /* RBIMPL_EVAL_H */ diff --git a/iseq.c b/iseq.c index f2db484f59f03e..15a78349caadbb 100644 --- a/iseq.c +++ b/iseq.c @@ -4361,7 +4361,7 @@ iseqw_to_binary(int argc, VALUE *argv, VALUE self) * binary causes critical problem. * * You should not load binary data provided by others. - * You should use binary data translated by yourself. + * You should only use binary data translated by yourself. */ static VALUE iseqw_s_load_from_binary(VALUE self, VALUE str) @@ -4369,6 +4369,12 @@ iseqw_s_load_from_binary(VALUE self, VALUE str) return iseqw_new(rb_iseq_ibf_load(str)); } +VALUE +rb_iseq_load_from_binary(const char *ptr, size_t len) +{ + return iseqw_new(rb_iseq_ibf_load_bytes(ptr, len)); +} + /* * call-seq: * RubyVM::InstructionSequence.load_from_binary_extra_data(binary) -> str diff --git a/test/-ext-/eval/test_iseq_load.rb b/test/-ext-/eval/test_iseq_load.rb new file mode 100644 index 00000000000000..927e263377a45d --- /dev/null +++ b/test/-ext-/eval/test_iseq_load.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: false +require 'test/unit' +require "-test-/eval" + +class IseqLoadTest < Test::Unit::TestCase + def test_rb_iseq_load_from_binary + binary = RubyVM::InstructionSequence.compile('1 + 1').to_binary + assert_equal 2, rb_iseq_load_from_binary(binary).eval + end +end From 5b505f860b11bae6d6b7daa41abb5b04d252789f Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 6 Aug 2026 12:10:07 +0200 Subject: [PATCH 20/30] Reduce RHASH_TBL_RAW usage --- hash.c | 15 +-------------- process.c | 8 ++------ 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/hash.c b/hash.c index 6524bc47ba9dab..cef358c21ee2c7 100644 --- a/hash.c +++ b/hash.c @@ -5102,20 +5102,7 @@ add_new_i(st_data_t *key, st_data_t *val, st_data_t arg, int existing) int rb_hash_add_new_element(VALUE hash, VALUE key, VALUE val) { - st_table *tbl; - int ret = -1; - - if (RHASH_AR_TABLE_P(hash)) { - ret = ar_update(hash, (st_data_t)key, add_new_i, (st_data_t)val); - if (ret == -1) { - ar_force_convert_table(hash, __FILE__, __LINE__); - } - } - - if (ret == -1) { - tbl = RHASH_TBL_RAW(hash); - ret = st_update(tbl, (st_data_t)key, add_new_i, (st_data_t)val); - } + int ret = rb_hash_stlike_update(hash, key, add_new_i, val); if (!ret) { // Newly inserted RB_OBJ_WRITTEN(hash, Qundef, key); diff --git a/process.c b/process.c index 9c2659af07d251..8ac52cdfb7f17c 100644 --- a/process.c +++ b/process.c @@ -2784,20 +2784,16 @@ rb_execarg_parent_start1(VALUE execarg_obj) } hide_obj(envtbl); if (envopts != Qfalse) { - st_table *stenv = RHASH_TBL_RAW(envtbl); long i; for (i = 0; i < RARRAY_LEN(envopts); i++) { VALUE pair = RARRAY_AREF(envopts, i); VALUE key = RARRAY_AREF(pair, 0); VALUE val = RARRAY_AREF(pair, 1); if (NIL_P(val)) { - st_data_t stkey = (st_data_t)key; - st_delete(stenv, &stkey, NULL); + rb_hash_delete(envtbl, key); } else { - st_insert(stenv, (st_data_t)key, (st_data_t)val); - RB_OBJ_WRITTEN(envtbl, Qundef, key); - RB_OBJ_WRITTEN(envtbl, Qundef, val); + rb_hash_aset(envtbl, key, val); } } } From 66c00fd53679f60130f3f56d7b3932643351a469 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Wed, 5 Aug 2026 21:40:05 +0200 Subject: [PATCH 21/30] rb_hash_init: don't spill to `st_table` unless needed Up to `RHASH_AR_TABLE_MAX_SIZE` there's no need to initialize an `st_table`. --- hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hash.c b/hash.c index cef358c21ee2c7..8799bb9b887c4d 100644 --- a/hash.c +++ b/hash.c @@ -1767,7 +1767,7 @@ rb_hash_init(rb_execution_context_t *ec, VALUE hash, VALUE capa_value, VALUE ifn if (capa_value != INT2FIX(0)) { long capa = NUM2LONG(capa_value); - if (capa > 0 && RHASH_SIZE(hash) == 0 && RHASH_AR_TABLE_P(hash)) { + if (capa > RHASH_AR_TABLE_MAX_SIZE && RHASH_SIZE(hash) == 0 && RHASH_AR_TABLE_P(hash)) { hash_st_table_init(hash, &objhash, capa); } } From 84a9e6bed71d009fa9ba272cfad96820127e097d Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Fri, 24 Jul 2026 00:31:08 +0200 Subject: [PATCH 22/30] [Feature #22212] Add Thread::Backtrace::Location#source_range --- NEWS.md | 5 + ast.c | 82 +++- internal/proc.h | 3 + internal/ruby_parser.h | 3 + prism_compile.c | 57 +++ prism_compile.h | 3 + proc.c | 37 +- spec/ruby/core/method/shared/source_range.rb | 2 +- .../proc/fixtures/source_range_helpers.rb | 31 -- spec/ruby/core/proc/source_range_spec.rb | 2 +- .../backtrace/location/source_range_spec.rb | 393 ++++++++++++++++++ spec/ruby/fixtures/source_range_helpers.rb | 111 +++++ vm_backtrace.c | 155 +++++++ 13 files changed, 829 insertions(+), 55 deletions(-) delete mode 100644 spec/ruby/core/proc/fixtures/source_range_helpers.rb create mode 100644 spec/ruby/core/thread/backtrace/location/source_range_spec.rb create mode 100644 spec/ruby/fixtures/source_range_helpers.rb diff --git a/NEWS.md b/NEWS.md index e3e2665e67fa15..ff0bf0129926d6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -113,6 +113,11 @@ Note: We're only listing outstanding class updates. * `Symbol#to_s` now returns a frozen string. [[Feature #22137]] +* Thread::Backtrace::Location + + * `Thread::Backtrace::Location#source_range` is added. It returns a + `Ruby::SourceRange` for the Ruby expression associated with the frame. + ## Stdlib updates * Psych diff --git a/ast.c b/ast.c index 9a1c08dad79982..ed2bcdb2b94888 100644 --- a/ast.c +++ b/ast.c @@ -179,15 +179,24 @@ rb_ast_parse_array(VALUE array, VALUE keep_script_lines, VALUE error_tolerant, V static VALUE node_children(VALUE, const NODE*); -static VALUE -node_find(VALUE self, const int node_id) +struct node_find_result { + VALUE node; + VALUE parent; +}; + +static bool +node_find_with_parent(VALUE self, VALUE parent, const int node_id, struct node_find_result *result) { VALUE ary; long i; struct ASTNodeData *data; TypedData_Get_Struct(self, struct ASTNodeData, &rb_node_type, data); - if (nd_node_id(data->node) == node_id) return self; + if (nd_node_id(data->node) == node_id) { + result->node = self; + result->parent = parent; + return true; + } ary = node_children(data->ast_value, data->node); @@ -195,12 +204,73 @@ node_find(VALUE self, const int node_id) VALUE child = RARRAY_AREF(ary, i); if (CLASS_OF(child) == rb_cNode) { - VALUE result = node_find(child, node_id); - if (RTEST(result)) return result; + if (node_find_with_parent(child, self, node_id, result)) return true; + } + } + + return false; +} + +static VALUE +node_find(VALUE self, const int node_id) +{ + struct node_find_result result = { Qnil, Qnil }; + node_find_with_parent(self, Qnil, node_id, &result); + return result.node; +} + +bool +rb_ast_node_source_location(VALUE source, VALUE path, int first_lineno, + int node_id, bool block_iseq, int iseq_node_id, + rb_code_location_t *location) +{ + VALUE ast; + + if (NIL_P(source)) { + ast = rb_ast_parse_file(path, Qfalse, Qfalse, Qfalse); + } + else { + VALUE ast_value; + VALUE vparser = setup_vparser(Qfalse, Qfalse, Qfalse); + + if (RB_TYPE_P(source, T_ARRAY)) { + ast_value = rb_parser_compile_array(vparser, path, source, first_lineno); + } + else { + StringValue(source); + ast_value = rb_parser_compile_string_path(vparser, path, source, first_lineno); + } + ast = ast_parse_done(ast_value); + } + + struct node_find_result result = { Qnil, Qnil }; + if (!node_find_with_parent(ast, Qnil, node_id, &result)) return false; + + struct ASTNodeData *data; + TypedData_Get_Struct(result.node, struct ASTNodeData, &rb_node_type, data); + const NODE *node = data->node; + + if (!NIL_P(result.parent)) { + struct ASTNodeData *parent_data; + TypedData_Get_Struct(result.parent, struct ASTNodeData, &rb_node_type, parent_data); + const NODE *parent = parent_data->node; + + /* Prism's call node includes its literal block. */ + if (nd_type(parent) == NODE_ITER && RNODE_ITER(parent)->nd_iter == node) { + node = parent; + } + } + + /* Prism's block node excludes the call that produced the block. */ + if (block_iseq && node_id == iseq_node_id && nd_type(node) == NODE_ITER) { + const NODE *scope = RNODE_ITER(node)->nd_body; + if (scope && nd_type(scope) == NODE_SCOPE) { + node = scope; } } - return Qnil; + *location = *nd_code_loc(node); + return true; } extern VALUE rb_e_script; diff --git a/internal/proc.h b/internal/proc.h index 24a077ca6d8eda..4528926471c664 100644 --- a/internal/proc.h +++ b/internal/proc.h @@ -11,6 +11,7 @@ #include "ruby/ruby.h" /* for rb_block_call_func_t */ #include "ruby/st.h" /* for st_index_t */ struct rb_block; /* in vm_core.h */ +struct rb_code_location_struct; /* in rubyparser.h */ struct rb_iseq_struct; /* in vm_core.h */ /* proc.c */ @@ -21,6 +22,8 @@ int rb_block_arity(void); int rb_block_min_max_arity(int *max); VALUE rb_block_to_s(VALUE self, const struct rb_block *block, const char *additional_info); VALUE rb_callable_receiver(VALUE); +VALUE rb_source_range_new(VALUE path, VALUE absolute_path, + const struct rb_code_location_struct *location); VALUE rb_func_proc_dup(VALUE src_obj); VALUE rb_func_lambda_new(rb_block_call_func_t func, VALUE val, int min_argc, int max_argc); diff --git a/internal/ruby_parser.h b/internal/ruby_parser.h index 8e306d18decd35..097ad3d291f678 100644 --- a/internal/ruby_parser.h +++ b/internal/ruby_parser.h @@ -56,6 +56,9 @@ VALUE rb_parser_compile_string(VALUE, const char*, VALUE, int); VALUE rb_parser_compile_file_path(VALUE vparser, VALUE fname, VALUE input, int line); VALUE rb_parser_compile_generic(VALUE vparser, rb_parser_lex_gets_func *lex_gets, VALUE fname, VALUE input, int line); VALUE rb_parser_compile_array(VALUE vparser, VALUE fname, VALUE array, int start); +bool rb_ast_node_source_location(VALUE source, VALUE path, int first_lineno, + int node_id, bool block_iseq, int iseq_node_id, + rb_code_location_t *location); enum lex_state_bits { EXPR_BEG_bit, /* ignore newline, +/- is a sign. */ diff --git a/prism_compile.c b/prism_compile.c index 29f266dfecf8c5..50dadbb80bfb3b 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -11051,6 +11051,63 @@ pm_parse_string(pm_parse_result_t *result, VALUE source, VALUE filepath, VALUE * return error; } +typedef struct { + uint32_t node_id; + const pm_node_t *node; +} pm_node_find_context_t; + +static bool +pm_node_find(const pm_node_t *node, void *data) +{ + pm_node_find_context_t *context = data; + + if (context->node == NULL && node->node_id == context->node_id) { + context->node = node; + return false; + } + + return context->node == NULL; +} + +bool +pm_node_source_location(VALUE source, VALUE filepath, int start_line, + int node_id, rb_code_location_t *location) +{ + pm_parse_result_t result; + pm_parse_result_init(&result); + + VALUE error; + if (NIL_P(source)) { + error = pm_load_parse_file(&result, filepath, NULL); + } + else { + pm_options_line_set(result.options, start_line); + error = pm_parse_string(&result, source, filepath, NULL); + } + + if (!NIL_P(error)) { + pm_parse_result_free(&result); + rb_exc_raise(error); + } + + pm_node_find_context_t context = { + .node_id = (uint32_t) node_id, + .node = NULL + }; + pm_visit_node(result.node.ast_node, pm_node_find, &context); + + bool found = context.node != NULL; + if (found) { + *location = pm_code_location(&result.node, context.node); + } + + RB_GC_GUARD(source); + RB_GC_GUARD(filepath); + + pm_parse_result_free(&result); + return found; +} + VALUE rb_io_gets_limit_internal(VALUE io, long limit); /** diff --git a/prism_compile.h b/prism_compile.h index 448579390259b6..783a167d798a28 100644 --- a/prism_compile.h +++ b/prism_compile.h @@ -15,6 +15,7 @@ typedef struct pm_local_index_struct { // A declaration for the struct that lives in compile.c. struct iseq_link_anchor; +struct rb_code_location_struct; /** * A direct-indexed lookup table mapping constant IDs to local variable indices. @@ -187,6 +188,8 @@ VALUE pm_parse_string(pm_parse_result_t *result, VALUE source, VALUE filepath, V VALUE pm_parse_stdin(pm_parse_result_t *result); void pm_options_version_for_current_ruby_set(pm_options_t *options); void pm_parse_result_free(pm_parse_result_t *result); +bool pm_node_source_location(VALUE source, VALUE filepath, int start_line, + int node_id, struct rb_code_location_struct *location); rb_iseq_t *pm_iseq_new(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, enum rb_iseq_type, int *error_state); rb_iseq_t *pm_iseq_new_top(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, const rb_iseq_t *parent, int *error_state); diff --git a/proc.c b/proc.c index 0f501699acd9fb..599b68ca7f2140 100644 --- a/proc.c +++ b/proc.c @@ -82,6 +82,22 @@ static const rb_data_type_t source_range_data_type = { 0, 0, RUBY_TYPED_FREE_IMMEDIATELY | RUBY_TYPED_WB_PROTECTED | RUBY_TYPED_DECL_MARKING }; +VALUE +rb_source_range_new(VALUE path, VALUE absolute_path, const rb_code_location_t *location) +{ + struct source_range_data *data; + VALUE obj = TypedData_Make_Struct( + rb_cSourceRange, struct source_range_data, &source_range_data_type, data); + RB_OBJ_WRITE(obj, &data->path, path); + RB_OBJ_WRITE(obj, &data->absolute_path, absolute_path); + data->start_line = location->beg_pos.lineno; + data->start_column = location->beg_pos.column; + data->end_line = location->end_pos.lineno; + data->end_column = location->end_pos.column; + + return obj; +} + static VALUE source_range_new(const rb_iseq_t *iseq) { @@ -96,19 +112,7 @@ source_range_new(const rb_iseq_t *iseq) return Qnil; } - int start_line, start_column, end_line, end_column; - rb_iseq_code_location(iseq, &start_line, &start_column, &end_line, &end_column); - - struct source_range_data *data; - VALUE obj = TypedData_Make_Struct(rb_cSourceRange, struct source_range_data, &source_range_data_type, data); - RB_OBJ_WRITE(obj, &data->path, path); - RB_OBJ_WRITE(obj, &data->absolute_path, absolute_path); - data->start_line = start_line; - data->start_column = start_column; - data->end_line = end_line; - data->end_column = end_column; - - return obj; + return rb_source_range_new(path, absolute_path, &ISEQ_BODY(iseq)->location.code_location); } static struct source_range_data * @@ -4909,11 +4913,12 @@ proc_ruby2_keywords(VALUE procval) /* * Document-class: Ruby::SourceRange * - * An object representing the source-code range for a Ruby callable. + * An object representing a range of Ruby source code. * * Source ranges are returned by Proc#source_range, Method#source_range, and - * UnboundMethod#source_range. They include the source path, absolute path when - * available, start line, start byte column, end line, and end byte column. + * UnboundMethod#source_range, as well as Thread::Backtrace::Location#source_range. + * They include the source path, absolute path when available, + * start line, start byte column, end line, and end byte column. * * The primary purpose of this class is to implement `Prism.find` precisely and cleanly on all Ruby implementations, * in a way which does not depend on implementation details like `node_id`. diff --git a/spec/ruby/core/method/shared/source_range.rb b/spec/ruby/core/method/shared/source_range.rb index 70884484e09029..a85f7120ecb02d 100644 --- a/spec/ruby/core/method/shared/source_range.rb +++ b/spec/ruby/core/method/shared/source_range.rb @@ -1,4 +1,4 @@ -require_relative '../../proc/fixtures/source_range_helpers' +require_relative '../../../fixtures/source_range_helpers' describe :method_source_range, shared: true do it "sets absolute_path to the real path of the source file" do diff --git a/spec/ruby/core/proc/fixtures/source_range_helpers.rb b/spec/ruby/core/proc/fixtures/source_range_helpers.rb deleted file mode 100644 index d5a12d28ee7636..00000000000000 --- a/spec/ruby/core/proc/fixtures/source_range_helpers.rb +++ /dev/null @@ -1,31 +0,0 @@ -def source_range_values(range) - [range.start_line, range.start_column, range.end_line, range.end_column] -end - -# Use <<-RUBY and not <<~RUBY to keep some spaces in front to make it more representative of a Proc in some file -def check_source_range(source) - raise "Expected 2 '$' to mark start and end of source_range" unless source.count('$') == 2 - from = source.byteindex('$') - to = source.byteindex('$', from+1) - lines = source.lines - from_line = 1 + source.byteslice(0, from).count("\n") - from_column = lines[from_line-1].byteindex('$') - to_line = 1 + source.byteslice(0, to).count("\n") - if from_line == to_line - to_column = lines[to_line-1].byteindex('$', from_column + 1) - 1 - else - to_column = lines[to_line-1].byteindex('$') - end - - eval_source = source.gsub('$', '') - result = eval(eval_source) - source_range = result.source_range - source_range.should.instance_of?(Ruby::SourceRange) - source_range.start_line.should == from_line - source_range.start_column.should == from_column - source_range.end_line.should == to_line - source_range.end_column.should == to_column - - # Check consistency with source_location start line - result.source_location[1].should == from_line -end diff --git a/spec/ruby/core/proc/source_range_spec.rb b/spec/ruby/core/proc/source_range_spec.rb index 81c803cc6bfd36..c0cf699a56e63c 100644 --- a/spec/ruby/core/proc/source_range_spec.rb +++ b/spec/ruby/core/proc/source_range_spec.rb @@ -1,5 +1,5 @@ require_relative '../../spec_helper' -require_relative 'fixtures/source_range_helpers' +require_relative '../../fixtures/source_range_helpers' ruby_version_is "4.1" do describe "Proc#source_range" do diff --git a/spec/ruby/core/thread/backtrace/location/source_range_spec.rb b/spec/ruby/core/thread/backtrace/location/source_range_spec.rb new file mode 100644 index 00000000000000..7246eec99ec91e --- /dev/null +++ b/spec/ruby/core/thread/backtrace/location/source_range_spec.rb @@ -0,0 +1,393 @@ +require_relative '../../../../spec_helper' +require_relative '../../../../fixtures/source_range_helpers' + +ruby_version_is "4.1" do + describe "Thread::Backtrace::Location#source_range" do + it "returns a Ruby::SourceRange with the location paths" do + location, range, path, absolute_path = capture_backtrace_location_source_range(<<-RUBY) + $nil.foo$ + RUBY + + range.should.instance_of?(Ruby::SourceRange) + range.path.should == path + range.absolute_path.should == absolute_path + location.path.should == path + location.absolute_path.should == absolute_path + end + + { + "receiver calls with arguments" => <<-RUBY, + $nil.foo(42)$ + RUBY + + "receiver calls split across lines" => <<-RUBY, + $nil + .foo( + 42 + )$ + RUBY + + "safe navigation calls" => <<-RUBY, + $1&.foo(42)$ + RUBY + + ".() call syntax" => <<-RUBY, + $nil.(42)$ + RUBY + + "calls to send" => <<-RUBY, + $nil.send(:foo, 42)$ + RUBY + + "index reads" => <<-RUBY, + $nil[0]$ + RUBY + + "index writes" => <<-RUBY, + $nil[0] = 42$ + RUBY + + "explicit index write calls" => <<-RUBY, + $nil.[]=$ + RUBY + + "attribute writes" => <<-RUBY, + $nil.foo = 42$ + RUBY + + "binary operator calls split by a comment" => <<-RUBY, + $nil + # comment + 42$ + RUBY + + "unary operator calls" => <<-RUBY, + $+nil$ + RUBY + + "function calls" => <<-RUBY, + "str".instance_eval { $gsub("foo", :sym)$ } + RUBY + + "function calls without ()" => <<-RUBY, + "str".instance_eval { $gsub "foo", :sym$ } + RUBY + + "variable calls" => <<-RUBY, + nil.instance_eval { $foo$ } + RUBY + + "local variable operator assignments" => <<-RUBY, + value = nil + $value += 42$ + RUBY + + "index operator assignments failing while reading" => <<-RUBY, + value = nil + $value[0] += 42$ + RUBY + + "index operator assignments failing in the operator" => <<-RUBY, + value = Object.new + def value.[](index) = nil + $value[0] += 42$ + RUBY + + "index operator assignments failing while writing" => <<-RUBY, + value = Object.new + def value.[](index) = 1 + $value[0] += 42$ + RUBY + + "index operator assignments failing on an argument" => <<-RUBY, + value = [] + $value[nil] += 42$ + RUBY + + "attribute operator assignments failing while reading" => <<-RUBY, + value = nil + $value.foo += 42$ + RUBY + + "attribute operator assignments failing in the operator" => <<-RUBY, + value = Object.new + def value.foo = nil + $value.foo += 42$ + RUBY + + "attribute operator assignments failing while writing" => <<-RUBY, + value = Object.new + def value.foo = 1 + $value.foo += 42$ + RUBY + + "attribute operator assignments failing on the value" => <<-RUBY, + value = Object.new + def value.foo = 1 + def value.foo=(new_value) + new_value + end + $value.foo += nil$ + RUBY + + "bare constants" => <<-RUBY, + $SourceRangeNotDefined$ + RUBY + + "qualified constants" => <<-RUBY, + $Object::SourceRangeNotDefined$ + RUBY + + "qualified constants split across lines" => <<-RUBY, + $Object:: + SourceRangeNotDefined$ + RUBY + + "top-level constants" => <<-RUBY, + $::SourceRangeNotDefined$ + RUBY + + "constant operator assignments" => <<-RUBY, + namespace = Module.new + namespace.const_set(:Nil, nil) + $namespace::Nil += 1$ + RUBY + + "constant operator assignments failing while reading" => <<-RUBY, + namespace = Module.new + $namespace::NotDefined += 1$ + RUBY + + "top-level constant operator assignments" => <<-RUBY, + $::SourceRangeNotDefined += 1$ + RUBY + + "explicit raises" => <<-RUBY, + $raise NameError$ + RUBY + + "calls failing while converting arguments" => <<-RUBY, + $1.+(nil)$ + RUBY + + "calls with brace blocks" => <<-RUBY, + $nil.foo(1) { 2 }$ + RUBY + + "calls with do-end blocks" => <<-RUBY, + $nil.foo(1) do + 2 + end$ + RUBY + + "calls with heredoc arguments" => <<-RUBY, + $nil.foo(<<~TEXT)$ + heredoc + TEXT + RUBY + + "multibyte identifiers with byte columns" => <<-RUBY, + value = "été" + $value.あいうえお$ + RUBY + + "hard tabs" => "\t \t$1.time {}$\n", + + "a missing final newline" => "$1.time {}$", + + "very long source lines" => ("1" * 100) + " + $1.time {}$\n", + }.each do |description, source| + it "returns the precise range for #{description}" do + capture_backtrace_location_source_range(source) + end + end + + it "returns the method definition for a method arity error" do + capture_backtrace_location_source_range(<<-RUBY) + target = Class.new do + $def source_range_target(first, second) + first + second + end$ + end.new + target.source_range_target(1) + RUBY + end + + it "returns the call for the caller frame of a method arity error" do + capture_backtrace_location_source_range(<<-RUBY, frame: 1) + target = Class.new do + def source_range_target(first, second) + first + second + end + end.new + $target.source_range_target(1)$ + RUBY + end + + it "returns a multiline method definition for a method arity error" do + capture_backtrace_location_source_range(<<-RUBY) + target = Class.new do + $def source_range_target( + first, + second + ) + first + second + end$ + end.new + target.source_range_target(1) + RUBY + end + + it "returns a singleton method definition with spacing for a keyword arity error" do + capture_backtrace_location_source_range(<<-RUBY) + target = Object.new + $def target . source_range_target(value:) + value + end$ + target.source_range_target + RUBY + end + + it "returns a stabby lambda for an arity error" do + capture_backtrace_location_source_range(<<-RUBY) + value = $->(argument) {}$ + value.call + RUBY + end + + it "returns only the block for an arity error in a Kernel#lambda" do + capture_backtrace_location_source_range(<<-RUBY) + value = lambda ${ |argument| }$ + value.call + RUBY + end + + it "returns only the block for an arity error in a define_method" do + capture_backtrace_location_source_range(<<-RUBY) + target = Class.new do + define_method(:source_range_target) $do |first, second| + first + second + end$ + end.new + target.source_range_target(1) + RUBY + end + + it "propagates an error when the absolute source file no longer exists" do + keep_source(false) do + location, path = capture_backtrace_location_from_source("nil.foo\n") + rm_r path + + -> { + location.source_range + }.should.raise(Errno::ENOENT) + ensure + rm_r path if path + end + end + + it "propagates a syntax error from changed source" do + keep_source(false) do + location, path = capture_backtrace_location_from_source("nil.foo\n") + File.binwrite(path, "(\n") + + -> { + location.source_range + }.should.raise(SyntaxError) + ensure + rm_r path if path + end + end + + it "raises when changed source no longer contains the node ID" do + keep_source(false) do + location, path = capture_backtrace_location_from_source("first = 1\nsecond = 2\nnil.foo\n") + File.binwrite(path, "nil\n") + + -> { + location.source_range + }.should.raise(RuntimeError, /cannot find node ID \d+ in parsed source/) + ensure + rm_r path if path + end + end + + it "uses retained eval source and preserves its starting line" do + keep_source do + path = File.realpath(__FILE__) + + location, range = capture_eval_backtrace_location_source_range( + "$nil.foo$", + path, + 100 + ) + + range.path.should == path + range.absolute_path.should == nil + range.start_line.should == 100 + location.lineno.should == 100 + end + end + + it "preserves the starting line for blocks in retained eval source" do + keep_source do + location, range = capture_eval_backtrace_location_source_range(<<-RUBY, "source_range_eval.rb", 100) + value = lambda ${ |argument| }$ + value.call + RUBY + + range.start_line.should == 100 + location.lineno.should == 100 + end + end + + it "does not open an eval path even when it names an existing absolute file" do + keep_source(false) do + path = File.realpath(__FILE__) + + exception = nil + begin + eval("nil.foo", binding, path) + rescue Exception => error + exception = error + end + + -> { + exception.backtrace_locations.first.source_range + }.should.raise(ArgumentError, "cannot get source range for location in eval") + end + end + + it "does not treat an eval path named -e as command-line source" do + keep_source(false) do + exception = nil + begin + eval("nil.foo", binding, "-e") + rescue Exception => error + exception = error + end + + -> { + exception.backtrace_locations.first.source_range + }.should.raise(ArgumentError, "cannot get source range for location in eval") + end + end + + it "does not treat a method from eval named -e as command-line source" do + code = "eval(%q{def spoofed_source_range_target; nil.foo; end}, binding, %q{-e}); " \ + "begin; spoofed_source_range_target; rescue => e; " \ + "begin; e.backtrace_locations.first.source_range; rescue => source_error; " \ + "p source_error; end; end" + ruby_exe(code, escape: false).should == "#\n" + end + + it "works for -e source" do + code = "def source_range_target; nil.foo; end; " \ + "begin; source_range_target; rescue => e; " \ + "r = e.backtrace_locations.first.source_range; " \ + "p [r.path, r.absolute_path, r.start_line, r.start_column, r.end_line, r.end_column]; end" + start_column = code.byteindex("nil.foo") + expected = ["-e", nil, 1, start_column, 1, start_column + "nil.foo".bytesize] + ruby_exe(code, escape: false).should == "#{expected.inspect}\n" + end + end +end diff --git a/spec/ruby/fixtures/source_range_helpers.rb b/spec/ruby/fixtures/source_range_helpers.rb new file mode 100644 index 00000000000000..aeed5cc8944f34 --- /dev/null +++ b/spec/ruby/fixtures/source_range_helpers.rb @@ -0,0 +1,111 @@ +def source_range_values(range) + [range.start_line, range.start_column, range.end_line, range.end_column] +end + +def keep_source(value = true) + return yield unless defined?(RubyVM.keep_script_lines) + + previous = RubyVM.keep_script_lines + begin + RubyVM.keep_script_lines = value + yield + ensure + RubyVM.keep_script_lines = previous + end +end + +def source_range_source(source) + raise "Expected 2 '$' to mark start and end of source_range" unless source.count('$') == 2 + from = source.byteindex('$') + to = source.byteindex('$', from + 1) + lines = source.lines + from_line = 1 + source.byteslice(0, from).count("\n") + from_column = lines[from_line-1].byteindex('$') + to_line = 1 + source.byteslice(0, to).count("\n") + if from_line == to_line + to_column = lines[to_line-1].byteindex('$', from_column + 1) - 1 + else + to_column = lines[to_line-1].byteindex('$') + end + + eval_source = source.gsub('$', '') + [eval_source, [from_line, from_column, to_line, to_column]] +end + +# Use <<-RUBY and not <<~RUBY to keep some spaces in front to make it more representative of a Proc in some file +def check_source_range(marked_source) + source, expected = source_range_source(marked_source) + result = eval(source) + range = result.source_range + range.should.instance_of?(Ruby::SourceRange) + source_range_values(range).should == expected + + # Check consistency with source_location start line + result.source_location[1].should == expected[0] +end + +def capture_backtrace_location_source_range(marked_source, frame: 0) + source, expected = source_range_source(marked_source) + path = tmp("backtrace_location_source_range.rb") + File.binwrite(path, source) + absolute_path = File.realpath(path) + + exception = nil + begin + load path + rescue Exception => error + exception = error + end + + raise "Expected source to raise an exception" unless exception + + location = exception.backtrace_locations.fetch(frame) + range = location.source_range + range.should.instance_of?(Ruby::SourceRange) + source_range_values(range).should == expected + + [location, range, path, absolute_path] +ensure + rm_r path if path +end + +def capture_backtrace_location_from_source(source, frame: 0) + path = tmp("backtrace_location_source_range.rb") + File.binwrite(path, source) + + exception = nil + begin + load path + rescue Exception => error + exception = error + end + + raise "Expected source to raise an exception" unless exception + + [exception.backtrace_locations.fetch(frame), path] +end + +def capture_eval_backtrace_location_source_range(marked_source, path, first_lineno) + source, expected = source_range_source(marked_source) + exception = nil + + begin + eval(source, binding, path, first_lineno) + rescue Exception => error + exception = error + end + + raise "Expected source to raise an exception" unless exception + + location = exception.backtrace_locations.first + range = location.source_range + range.should.instance_of?(Ruby::SourceRange) + source_range_values(range).should == [ + expected[0] + first_lineno - 1, + expected[1], + expected[2] + first_lineno - 1, + expected[3] + ] + + [location, range] +end diff --git a/vm_backtrace.c b/vm_backtrace.c index 5af6cc341a8237..c987b44f1f07b3 100644 --- a/vm_backtrace.c +++ b/vm_backtrace.c @@ -14,6 +14,8 @@ #include "internal/class.h" #include "internal/error.h" #include "internal/object.h" +#include "internal/proc.h" +#include "internal/ruby_parser.h" #include "internal/vm.h" #include "iseq.h" #include "ruby/debug.h" @@ -407,8 +409,160 @@ location_node_id(rb_backtrace_location_t *loc) } return -1; } + +extern VALUE rb_e_script; + +static bool +location_code_location_equal(const rb_code_location_t *left, const rb_code_location_t *right) +{ + return left->beg_pos.lineno == right->beg_pos.lineno && + left->beg_pos.column == right->beg_pos.column && + left->end_pos.lineno == right->end_pos.lineno && + left->end_pos.column == right->end_pos.column; +} + +static bool +iseq_from_e_script_p(const rb_iseq_t *iseq, VALUE path) +{ + if (!RB_TYPE_P(path, T_STRING) || + RSTRING_LEN(path) != 2 || + memcmp(RSTRING_PTR(path), "-e", 2) != 0 || + !RTEST(rb_e_script)) { + return false; + } + + const rb_iseq_t *source_iseq = iseq; + for (; source_iseq; source_iseq = ISEQ_BODY(source_iseq)->parent_iseq) { + if (ISEQ_BODY(source_iseq)->type == ISEQ_TYPE_EVAL) return false; + if (ISEQ_BODY(source_iseq)->type == ISEQ_TYPE_MAIN) return true; + } + + int node_id = ISEQ_BODY(iseq)->location.node_id; + if (node_id == -1) return false; + + rb_code_location_t source_location; + bool found; + if (ISEQ_BODY(iseq)->prism) { + found = pm_node_source_location(rb_e_script, path, 1, node_id, &source_location); + } + else { + found = rb_ast_node_source_location( + rb_e_script, + path, + 1, + node_id, + ISEQ_BODY(iseq)->type == ISEQ_TYPE_BLOCK, + node_id, + &source_location + ); + } + + return found && location_code_location_equal( + &source_location, &ISEQ_BODY(iseq)->location.code_location); +} + +static int +location_source_first_lineno(const rb_iseq_t *iseq, VALUE script_lines) +{ + const rb_iseq_t *source_iseq = iseq; + + while (ISEQ_BODY(source_iseq)->parent_iseq) { + const rb_iseq_t *parent = ISEQ_BODY(source_iseq)->parent_iseq; + if (ISEQ_BODY(parent)->variable.script_lines != script_lines) break; + source_iseq = parent; + } + + return ISEQ_BODY(source_iseq)->location.first_lineno; +} #endif +/* + * call-seq: + * location.source_range -> Ruby::SourceRange or nil + * + * Returns the Ruby::SourceRange for the Ruby expression associated with this + * backtrace location, or +nil+ when the location is not available + * (e.g., the source is not Ruby code). + * + * This method requires re-reading the source file from the filesystem + * (since this information is not kept in the bytecode to avoid memory overhead). + * Errno::ENOENT if the source file no longer exists. + * RuntimeError is raised if the file has been modified. + * + * On CRuby, `RubyVM.keep_script_lines = true` can be used to avoid to re-read + * source files from the filesystem, however this will increase memory usage, + * by keeping all source files in memory. + * + * Locations from eval'd code are only available with `RubyVM.keep_script_lines = true`. + */ +static VALUE +location_source_range_m(VALUE self) +{ +#ifdef USE_ISEQ_NODE_ID + rb_backtrace_location_t *backtrace_location = location_ptr(self); + const rb_iseq_t *iseq = location_iseq(backtrace_location); + if (!iseq) return Qnil; + + rb_iseq_check(iseq); + int node_id = location_node_id(backtrace_location); + if (node_id == -1) return Qnil; + + VALUE path = rb_iseq_path(iseq); + VALUE absolute_path = rb_iseq_realpath(iseq); + VALUE script_lines = ISEQ_BODY(iseq)->variable.script_lines; + VALUE source = script_lines; + VALUE parser_path = path; + int first_lineno = 1; + + if (!NIL_P(script_lines)) { + first_lineno = location_source_first_lineno(iseq, script_lines); + } + else if (iseq_from_e_script_p(iseq, path)) { + source = rb_e_script; + } + else if (!NIL_P(absolute_path)) { + source = Qnil; + parser_path = absolute_path; + } + else { + rb_raise(rb_eArgError, "cannot get source range for location in eval"); + } + + if (NIL_P(parser_path)) { + parser_path = rb_str_new_cstr("(eval)"); + } + + rb_code_location_t code_location; + bool found; + + if (ISEQ_BODY(iseq)->prism) { + if (RB_TYPE_P(source, T_ARRAY)) { + source = rb_ary_join(source, Qnil); + } + found = pm_node_source_location(source, parser_path, first_lineno, node_id, &code_location); + } + else { + found = rb_ast_node_source_location( + source, + parser_path, + first_lineno, + node_id, + ISEQ_BODY(iseq)->type == ISEQ_TYPE_BLOCK, + ISEQ_BODY(iseq)->location.node_id, + &code_location + ); + } + + if (!found) { + rb_raise(rb_eRuntimeError, "cannot find node ID %d in parsed source", node_id); + } + + return rb_source_range_new(path, absolute_path, &code_location); +#else + return Qnil; +#endif +} + int rb_get_node_id_from_frame_info(VALUE obj) { @@ -1530,6 +1684,7 @@ Init_vm_backtrace(void) rb_define_method(rb_cBacktraceLocation, "base_label", location_base_label_m, 0); rb_define_method(rb_cBacktraceLocation, "path", location_path_m, 0); rb_define_method(rb_cBacktraceLocation, "absolute_path", location_absolute_path_m, 0); + rb_define_method(rb_cBacktraceLocation, "source_range", location_source_range_m, 0); rb_define_method(rb_cBacktraceLocation, "to_s", location_to_str_m, 0); rb_define_method(rb_cBacktraceLocation, "inspect", location_inspect_m, 0); From 93638d07e1a4b6c7fe02d1154fd31f201d5faeca Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Sun, 19 Jul 2026 03:42:43 +0900 Subject: [PATCH 23/30] Compute a source hash of the program in parse.y Introduce a streaming source hash API (rb_source_hash_init/update/ finalize) in ruby_parser.c, and use it in the lexer of parse.y to accumulate a hash of the source as each line is read. The hash is stored in the AST, and will be used to check whether a file still contains the same source code when it is re-parsed later. [Feature #21795] The hash algorithm (currently FNV-1a) is hidden behind the API as an implementation detail of the interpreter, so it can be changed freely between releases. Co-Authored-By: Claude Fable 5 --- internal/ruby_parser.h | 4 ++++ parse.y | 7 +++++++ ruby_parser.c | 31 +++++++++++++++++++++++++++++++ rubyparser.h | 13 +++++++++++++ universal_parser.c | 4 ++++ 5 files changed, 59 insertions(+) diff --git a/internal/ruby_parser.h b/internal/ruby_parser.h index 097ad3d291f678..efa2e41e0f70ac 100644 --- a/internal/ruby_parser.h +++ b/internal/ruby_parser.h @@ -40,6 +40,10 @@ VALUE rb_node_integer_literal_val(const NODE *); VALUE rb_node_float_literal_val(const NODE *); VALUE rb_node_rational_literal_val(const NODE *); VALUE rb_node_imaginary_literal_val(const NODE *); + +void rb_source_hash_init(rb_source_hash_state_t *state); +void rb_source_hash_update(rb_source_hash_state_t *state, const uint8_t *ptr, size_t len); +uint64_t rb_source_hash_finalize(const rb_source_hash_state_t *state); RUBY_SYMBOL_EXPORT_END VALUE rb_parser_end_seen_p(VALUE); diff --git a/parse.y b/parse.y index c6973ca6620b0d..2afeb62097471e 100644 --- a/parse.y +++ b/parse.y @@ -579,6 +579,9 @@ struct parser_params { unsigned int error_p: 1; unsigned int cr_seen: 1; + /* Streaming hash state of the source bytes read so far. */ + rb_source_hash_state_t source_hash; + #ifndef RIPPER /* Ruby core only */ @@ -7456,6 +7459,8 @@ yycompile(struct parser_params *p, VALUE fname, int line) p->ast = ast = rb_ast_new(); compile_callback(yycompile0, (VALUE)p); + ast->body.source_hash = rb_source_hash_finalize(&p->source_hash); + ast->body.has_source_hash = 1; p->ast = 0; while (p->lvtbl) { @@ -7482,6 +7487,7 @@ lex_getline(struct parser_params *p) rb_parser_string_t *line = (*p->lex.gets)(p, p->lex.input, p->line_count); if (!line) return 0; p->line_count++; + rb_source_hash_update(&p->source_hash, (const uint8_t *)line->ptr, (size_t)line->len); string_buffer_append(p, line); must_be_ascii_compatible(p, line); return line; @@ -15521,6 +15527,7 @@ parser_initialize(struct parser_params *p) p->node_id = 0; p->delayed.token = NULL; p->frozen_string_literal = -1; /* not specified */ + rb_source_hash_init(&p->source_hash); #ifndef RIPPER p->error_buffer = Qfalse; p->end_expect_token_locations = NULL; diff --git a/ruby_parser.c b/ruby_parser.c index 7f9c04e6b0facf..3f012d3b694a56 100644 --- a/ruby_parser.c +++ b/ruby_parser.c @@ -440,6 +440,11 @@ static const rb_parser_config_t rb_global_parser_config = { /* For Ripper */ .static_id2sym = static_id2sym, .str_coderange_scan_restartable = str_coderange_scan_restartable, + + /* Source hash */ + .source_hash_init = rb_source_hash_init, + .source_hash_update = rb_source_hash_update, + .source_hash_finalize = rb_source_hash_finalize, }; #endif @@ -1091,6 +1096,32 @@ parser_aset_script_lines_for(VALUE path, rb_parser_ary_t *lines) rb_hash_aset(hash, path, script_lines); } +/* The source hash API currently computes FNV-1a, but the algorithm is an + * implementation detail. The hash values are only ever compared against + * hashes computed by the same interpreter, so the algorithm can be changed + * freely between releases. */ +void +rb_source_hash_init(rb_source_hash_state_t *state) +{ + state->hash = 0xcbf29ce484222325; /* FNV-1a offset basis */ +} + +void +rb_source_hash_update(rb_source_hash_state_t *state, const uint8_t *ptr, size_t len) +{ + uint64_t hash = state->hash; + for (size_t i = 0; i < len; i++) { + hash = (hash ^ ptr[i]) * 0x100000001b3; /* FNV-1a prime */ + } + state->hash = hash; +} + +uint64_t +rb_source_hash_finalize(const rb_source_hash_state_t *state) +{ + return state->hash; +} + VALUE rb_ruby_ast_new(const NODE *const root) { diff --git a/rubyparser.h b/rubyparser.h index 2ed93e98948aba..69f9056cf2c540 100644 --- a/rubyparser.h +++ b/rubyparser.h @@ -1175,12 +1175,20 @@ typedef struct node_buffer_struct node_buffer_t; typedef struct rb_parser_config_struct rb_parser_config_t; #endif +/* Streaming state of a source hash. The layout and the hash algorithm are + * implementation details; use rb_source_hash_init/update/finalize. */ +typedef struct rb_source_hash_state { + uint64_t hash; +} rb_source_hash_state_t; + typedef struct rb_ast_body_struct { const NODE *root; rb_parser_ary_t *script_lines; int line_count; signed int frozen_string_literal:2; /* -1: not specified, 0: false, 1: true */ signed int coverage_enabled:2; /* -1: not specified, 0: false, 1: true */ + unsigned int has_source_hash:1; + uint64_t source_hash; } rb_ast_body_t; typedef struct rb_ast_struct { node_buffer_t *node_buffer; @@ -1357,6 +1365,11 @@ typedef struct rb_parser_config_struct { int enc_coderange_unknown; VALUE (*static_id2sym)(ID id); long (*str_coderange_scan_restartable)(const char *s, const char *e, rb_encoding *enc, int *cr); + + /* Source hash */ + void (*source_hash_init)(rb_source_hash_state_t *state); + void (*source_hash_update)(rb_source_hash_state_t *state, const uint8_t *ptr, size_t len); + uint64_t (*source_hash_finalize)(const rb_source_hash_state_t *state); } rb_parser_config_t; #undef rb_encoding diff --git a/universal_parser.c b/universal_parser.c index b9cddd2879d717..6f5826eb9eecdf 100644 --- a/universal_parser.c +++ b/universal_parser.c @@ -209,3 +209,7 @@ #define rb_ast_new() \ rb_ast_new(p->config) + +#define rb_source_hash_init p->config->source_hash_init +#define rb_source_hash_update p->config->source_hash_update +#define rb_source_hash_finalize p->config->source_hash_finalize From 2d6295241f6970d714a35177f0e6d7c0e920b7b7 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Sun, 19 Jul 2026 08:48:09 +0900 Subject: [PATCH 24/30] Compute a source hash for prism as well After each prism parse, compute a source hash of the parsed source and keep it in the scope node. The data section after an __END__ marker is not part of the code, so the hash covers the source only up to the end of the __END__ line, which matches the range that parse.y hashes. Co-Authored-By: Claude Fable 5 --- prism_compile.c | 27 +++++++++++++++++++++++++++ prism_compile.h | 3 +++ 2 files changed, 30 insertions(+) diff --git a/prism_compile.c b/prism_compile.c index 50dadbb80bfb3b..f1b48470d2f223 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -10738,6 +10738,32 @@ pm_warning_emit_callback(const pm_diagnostic_t *diagnostic, void *data) { * It returns an error if one should be raised. It is assumed that the parse * result object is zeroed out. */ +/** + * Compute the hash of the source code that was parsed. The data section after + * an __END__ marker is not part of the code, so the hash covers the source + * only up to the end of the __END__ line, which also matches the range that + * parse.y hashes. + */ +static uint64_t +pm_source_hash(const pm_parser_t *parser) +{ + const uint8_t *start = pm_parser_start(parser); + const uint8_t *end = pm_parser_end(parser); + const pm_location_t *data_loc = pm_parser_data_loc(parser); + + if (data_loc->length != 0) { + const uint8_t *cursor = start + data_loc->start; + while (cursor < end && *cursor != '\n') cursor++; + if (cursor < end) cursor++; + end = cursor; + } + + rb_source_hash_state_t state; + rb_source_hash_init(&state); + rb_source_hash_update(&state, start, (size_t) (end - start)); + return rb_source_hash_finalize(&state); +} + static VALUE pm_parse_process(pm_parse_result_t *result, pm_node_t *node, VALUE *script_lines) { @@ -10793,6 +10819,7 @@ pm_parse_process(pm_parse_result_t *result, pm_node_t *node, VALUE *script_lines // Now set up the constant pool and intern all of the various constants into // their corresponding IDs. scope_node->parser = parser; + scope_node->source_hash = pm_source_hash(parser); scope_node->options = result->options; scope_node->line_offsets = pm_parser_line_offsets(parser); scope_node->start_line = pm_parser_start_line(parser); diff --git a/prism_compile.h b/prism_compile.h index 783a167d798a28..82889d9a5b5316 100644 --- a/prism_compile.h +++ b/prism_compile.h @@ -104,6 +104,9 @@ typedef struct pm_scope_node { pm_constant_id_list_t locals; const pm_parser_t *parser; + + /** The source hash of the parsed source, propagated to every iseq. */ + uint64_t source_hash; const pm_options_t *options; const pm_line_offset_list_t *line_offsets; int32_t start_line; From 25959fe2957f88f5ef59205cb1cde264bedca2c1 Mon Sep 17 00:00:00 2001 From: Yusuke Endoh Date: Sun, 19 Jul 2026 08:36:01 +0900 Subject: [PATCH 25/30] Store a source hash in ISeqs Record the source hash in each ISeq. For prism, the hash computed at parse time is propagated through the scope nodes; for parse.y, it is taken from the AST, and copied to child iseqs whose AST wrappers are created separately. Expose it along with the node id via RubyVM::InstructionSequence#source_hash and #node_id, include it in the misc hash of #to_a, and preserve it in ISeq binaries (bumping IBF_DEVEL_VERSION). Co-Authored-By: Claude Fable 5 --- compile.c | 28 +++++++++++++++++++++++++++- iseq.c | 29 +++++++++++++++++++++++++++++ vm_core.h | 5 +++++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/compile.c b/compile.c index 46d00686b9903b..a6a498e9a1f518 100644 --- a/compile.c +++ b/compile.c @@ -1503,6 +1503,14 @@ new_child_iseq(rb_iseq_t *iseq, const NODE *const node, rb_iseq_t *ret_iseq; VALUE ast_value = rb_ruby_ast_new(node); + // The child AST wrapper does not carry the source hash, so copy it from + // the enclosing iseq before compiling, for grandchildren to inherit it. + if (ISEQ_BODY(iseq)->has_source_hash) { + rb_ast_t *child_ast = rb_ruby_ast_data_get(ast_value); + child_ast->body.source_hash = ISEQ_BODY(iseq)->source_hash; + child_ast->body.has_source_hash = 1; + } + debugs("[new_child_iseq]> ---------------------------------------\n"); int isolated_depth = ISEQ_COMPILE_DATA(iseq)->isolated_depth; ret_iseq = rb_iseq_new_with_opt(ast_value, name, @@ -12476,6 +12484,12 @@ rb_iseq_build_from_ary(rb_iseq_t *iseq, VALUE misc, VALUE locals, VALUE params, #undef INT_PARAM } + VALUE source_hash = rb_hash_aref(misc, ID2SYM(rb_intern("source_hash"))); + if (!NIL_P(source_hash)) { + ISEQ_BODY(iseq)->source_hash = NUM2ULL(source_hash); + ISEQ_BODY(iseq)->has_source_hash = true; + } + VALUE node_ids = Qfalse; #ifdef USE_ISEQ_NODE_ID node_ids = rb_hash_aref(misc, ID2SYM(rb_intern("node_ids"))); @@ -12602,7 +12616,7 @@ typedef uint32_t ibf_offset_t; #define IBF_MAJOR_VERSION ISEQ_MAJOR_VERSION #ifdef RUBY_DEVEL -#define IBF_DEVEL_VERSION 5 +#define IBF_DEVEL_VERSION 6 #define IBF_MINOR_VERSION (ISEQ_MINOR_VERSION * 10000 + IBF_DEVEL_VERSION) #else #define IBF_MINOR_VERSION ISEQ_MINOR_VERSION @@ -13782,6 +13796,12 @@ ibf_dump_iseq_each(struct ibf_dump *dump, const rb_iseq_t *iseq) ibf_dump_write_small_value(dump, location_label_index); ibf_dump_write_small_value(dump, body->location.first_lineno); ibf_dump_write_small_value(dump, body->location.node_id); + /* Dump the source hash in two 32-bit halves, because VALUE may be + * 32 bits wide. */ + uint64_t source_hash = body->has_source_hash ? body->source_hash : 0; + ibf_dump_write_small_value(dump, (VALUE)(uint32_t)(source_hash >> 32)); + ibf_dump_write_small_value(dump, (VALUE)(uint32_t)source_hash); + ibf_dump_write_small_value(dump, body->has_source_hash ? 1 : 0); ibf_dump_write_small_value(dump, body->location.code_location.beg_pos.lineno); ibf_dump_write_small_value(dump, body->location.code_location.beg_pos.column); ibf_dump_write_small_value(dump, body->location.code_location.end_pos.lineno); @@ -13894,6 +13914,10 @@ ibf_load_iseq_each(struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t offset) const VALUE location_label_index = ibf_load_small_value(load, &reading_pos); const int location_first_lineno = (int)ibf_load_small_value(load, &reading_pos); const int location_node_id = (int)ibf_load_small_value(load, &reading_pos); + const uint64_t source_hash_hi = (uint64_t)ibf_load_small_value(load, &reading_pos); + const uint64_t source_hash_lo = (uint64_t)ibf_load_small_value(load, &reading_pos); + const uint64_t source_hash = (source_hash_hi << 32) | (uint32_t)source_hash_lo; + const bool has_source_hash = ibf_load_small_value(load, &reading_pos) != 0; const int location_code_location_beg_pos_lineno = (int)ibf_load_small_value(load, &reading_pos); const int location_code_location_beg_pos_column = (int)ibf_load_small_value(load, &reading_pos); const int location_code_location_end_pos_lineno = (int)ibf_load_small_value(load, &reading_pos); @@ -13994,6 +14018,8 @@ ibf_load_iseq_each(struct ibf_load *load, rb_iseq_t *iseq, ibf_offset_t offset) load_body->location.first_lineno = location_first_lineno; load_body->location.node_id = location_node_id; + load_body->source_hash = source_hash; + load_body->has_source_hash = has_source_hash; load_body->location.code_location.beg_pos.lineno = location_code_location_beg_pos_lineno; load_body->location.code_location.beg_pos.column = location_code_location_beg_pos_column; load_body->location.code_location.end_pos.lineno = location_code_location_end_pos_lineno; diff --git a/iseq.c b/iseq.c index 15a78349caadbb..6f3332b626d3c6 100644 --- a/iseq.c +++ b/iseq.c @@ -1088,6 +1088,11 @@ rb_iseq_new_with_opt(VALUE ast_value, VALUE name, VALUE path, VALUE realpath, prepare_iseq_build(iseq, name, path, realpath, first_lineno, node ? &node->nd_loc : NULL, prepare_node_id(node), parent, isolated_depth, type, script_lines, option); + if (body && body->has_source_hash) { + ISEQ_BODY(iseq)->source_hash = body->source_hash; + ISEQ_BODY(iseq)->has_source_hash = true; + } + rb_iseq_compile_node(iseq, node); finish_iseq_build(iseq); RB_GC_GUARD(ast_value); @@ -1108,6 +1113,9 @@ pm_iseq_build(pm_scope_node_t *node, VALUE name, VALUE path, VALUE realpath, rb_iseq_t *iseq = iseq_alloc(); ISEQ_BODY(iseq)->prism = true; + ISEQ_BODY(iseq)->source_hash = node->source_hash; + ISEQ_BODY(iseq)->has_source_hash = true; + rb_compile_option_t next_option; if (!option) option = &COMPILE_OPTION_DEFAULT; @@ -3706,6 +3714,7 @@ iseq_data_to_ary(const rb_iseq_t *iseq) rb_hash_aset(misc, ID2SYM(rb_intern("local_size")), INT2FIX(iseq_body->local_table_size)); rb_hash_aset(misc, ID2SYM(rb_intern("stack_max")), INT2FIX(iseq_body->stack_max)); rb_hash_aset(misc, ID2SYM(rb_intern("node_id")), INT2FIX(iseq_body->location.node_id)); + rb_hash_aset(misc, ID2SYM(rb_intern("source_hash")), iseq_body->has_source_hash ? ULL2NUM(iseq_body->source_hash) : Qnil); rb_hash_aset(misc, ID2SYM(rb_intern("code_location")), rb_ary_new_from_args(4, INT2FIX(iseq_body->location.code_location.beg_pos.lineno), @@ -4542,6 +4551,24 @@ iseqw_script_lines(VALUE self) return ISEQ_BODY(iseq)->variable.script_lines; } +/* Returns the hash of the source this iseq was compiled from, or nil if it + * is unavailable. */ +static VALUE +iseqw_source_hash(VALUE self) +{ + const rb_iseq_t *iseq = iseqw_check(self); + if (!ISEQ_BODY(iseq)->has_source_hash) return Qnil; + return ULL2NUM(ISEQ_BODY(iseq)->source_hash); +} + +/* Returns the node id of the AST node this iseq corresponds to. */ +static VALUE +iseqw_node_id(VALUE self) +{ + const rb_iseq_t *iseq = iseqw_check(self); + return INT2NUM(ISEQ_BODY(iseq)->location.node_id); +} + /* * Document-class: RubyVM::InstructionSequence * @@ -4613,6 +4640,8 @@ Init_ISeq(void) // script lines rb_define_method(rb_cISeq, "script_lines", iseqw_script_lines, 0); + rb_define_method(rb_cISeq, "source_hash", iseqw_source_hash, 0); + rb_define_method(rb_cISeq, "node_id", iseqw_node_id, 0); rb_undef_method(CLASS_OF(rb_cISeq), "translate"); rb_undef_method(CLASS_OF(rb_cISeq), "load_iseq"); diff --git a/vm_core.h b/vm_core.h index 75907eabd6aa33..ce118d5080ccbf 100644 --- a/vm_core.h +++ b/vm_core.h @@ -574,6 +574,11 @@ struct rb_iseq_constant_body { // ZJIT stores some data on each iseq. void *zjit_payload; #endif + + // Hash of the source this iseq was compiled from. Meaningful only when + // has_source_hash is set. + uint64_t source_hash; + bool has_source_hash; }; /* T_IMEMO/iseq */ From a55e31b769edd3dc142c128e4d15e581fda63cf5 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Wed, 29 Jul 2026 23:46:19 +0200 Subject: [PATCH 26/30] Remove unused ISeq methods --- iseq.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/iseq.c b/iseq.c index 6f3332b626d3c6..23e9abcea7be9b 100644 --- a/iseq.c +++ b/iseq.c @@ -4551,24 +4551,6 @@ iseqw_script_lines(VALUE self) return ISEQ_BODY(iseq)->variable.script_lines; } -/* Returns the hash of the source this iseq was compiled from, or nil if it - * is unavailable. */ -static VALUE -iseqw_source_hash(VALUE self) -{ - const rb_iseq_t *iseq = iseqw_check(self); - if (!ISEQ_BODY(iseq)->has_source_hash) return Qnil; - return ULL2NUM(ISEQ_BODY(iseq)->source_hash); -} - -/* Returns the node id of the AST node this iseq corresponds to. */ -static VALUE -iseqw_node_id(VALUE self) -{ - const rb_iseq_t *iseq = iseqw_check(self); - return INT2NUM(ISEQ_BODY(iseq)->location.node_id); -} - /* * Document-class: RubyVM::InstructionSequence * @@ -4640,8 +4622,6 @@ Init_ISeq(void) // script lines rb_define_method(rb_cISeq, "script_lines", iseqw_script_lines, 0); - rb_define_method(rb_cISeq, "source_hash", iseqw_source_hash, 0); - rb_define_method(rb_cISeq, "node_id", iseqw_node_id, 0); rb_undef_method(CLASS_OF(rb_cISeq), "translate"); rb_undef_method(CLASS_OF(rb_cISeq), "load_iseq"); From eb0254e115ed9639437c8b5c99045262c35e55dc Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Wed, 29 Jul 2026 23:58:06 +0200 Subject: [PATCH 27/30] Check the source hash for Thread::Backtrace::Location#source_range --- ast.c | 22 +--- prism_compile.c | 10 +- .../backtrace/location/source_range_spec.rb | 47 ++++++- vm_backtrace.c | 121 ++++++++++++++---- 4 files changed, 148 insertions(+), 52 deletions(-) diff --git a/ast.c b/ast.c index ed2bcdb2b94888..11cce897274be5 100644 --- a/ast.c +++ b/ast.c @@ -224,24 +224,10 @@ rb_ast_node_source_location(VALUE source, VALUE path, int first_lineno, int node_id, bool block_iseq, int iseq_node_id, rb_code_location_t *location) { - VALUE ast; - - if (NIL_P(source)) { - ast = rb_ast_parse_file(path, Qfalse, Qfalse, Qfalse); - } - else { - VALUE ast_value; - VALUE vparser = setup_vparser(Qfalse, Qfalse, Qfalse); - - if (RB_TYPE_P(source, T_ARRAY)) { - ast_value = rb_parser_compile_array(vparser, path, source, first_lineno); - } - else { - StringValue(source); - ast_value = rb_parser_compile_string_path(vparser, path, source, first_lineno); - } - ast = ast_parse_done(ast_value); - } + StringValue(source); + VALUE vparser = setup_vparser(Qfalse, Qfalse, Qfalse); + VALUE ast_value = rb_parser_compile_string_path(vparser, path, source, first_lineno); + VALUE ast = ast_parse_done(ast_value); struct node_find_result result = { Qnil, Qnil }; if (!node_find_with_parent(ast, Qnil, node_id, &result)) return false; diff --git a/prism_compile.c b/prism_compile.c index f1b48470d2f223..e9c21a9411ee24 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -11103,14 +11103,8 @@ pm_node_source_location(VALUE source, VALUE filepath, int start_line, pm_parse_result_t result; pm_parse_result_init(&result); - VALUE error; - if (NIL_P(source)) { - error = pm_load_parse_file(&result, filepath, NULL); - } - else { - pm_options_line_set(result.options, start_line); - error = pm_parse_string(&result, source, filepath, NULL); - } + pm_options_line_set(result.options, start_line); + VALUE error = pm_parse_string(&result, source, filepath, NULL); if (!NIL_P(error)) { pm_parse_result_free(&result); diff --git a/spec/ruby/core/thread/backtrace/location/source_range_spec.rb b/spec/ruby/core/thread/backtrace/location/source_range_spec.rb index 7246eec99ec91e..1fb4c9f10aa0be 100644 --- a/spec/ruby/core/thread/backtrace/location/source_range_spec.rb +++ b/spec/ruby/core/thread/backtrace/location/source_range_spec.rb @@ -185,6 +185,10 @@ def value.foo=(new_value) TEXT RUBY + "source with a data section" => "$nil.foo$\n__END__\ndata\n", + + "__END__ inside a heredoc" => "value = < <<-RUBY, value = "été" $value.あいうえお$ @@ -272,6 +276,28 @@ def source_range_target(first, second) RUBY end + it "raises for a location without Ruby bytecode" do + report_on_exception = Thread.report_on_exception + Thread.report_on_exception = false + + begin + thread = Thread.new(&method(:throw)) + exception = begin + thread.value + rescue ArgumentError => error + error + end + location = exception.backtrace_locations.first + + location.path.should == nil + -> { + location.source_range + }.should.raise(RuntimeError, "cannot get source range for location without Ruby bytecode") + ensure + Thread.report_on_exception = report_on_exception + end + end + it "propagates an error when the absolute source file no longer exists" do keep_source(false) do location, path = capture_backtrace_location_from_source("nil.foo\n") @@ -285,27 +311,40 @@ def source_range_target(first, second) end end - it "propagates a syntax error from changed source" do + it "raises when changed source has invalid syntax" do keep_source(false) do location, path = capture_backtrace_location_from_source("nil.foo\n") File.binwrite(path, "(\n") -> { location.source_range - }.should.raise(SyntaxError) + }.should.raise(RuntimeError, "source has been modified") ensure rm_r path if path end end - it "raises when changed source no longer contains the node ID" do + it "validates changed source before looking up the node ID" do keep_source(false) do location, path = capture_backtrace_location_from_source("first = 1\nsecond = 2\nnil.foo\n") File.binwrite(path, "nil\n") -> { location.source_range - }.should.raise(RuntimeError, /cannot find node ID \d+ in parsed source/) + }.should.raise(RuntimeError, "source has been modified") + ensure + rm_r path if path + end + end + + it "raises when changed source has the same node ID layout" do + keep_source(false) do + location, path = capture_backtrace_location_from_source("nil.foo\n") + File.binwrite(path, "nil.longer_method_name\n") + + -> { + location.source_range + }.should.raise(RuntimeError, "source has been modified") ensure rm_r path if path end diff --git a/vm_backtrace.c b/vm_backtrace.c index c987b44f1f07b3..b84ed105a7cc74 100644 --- a/vm_backtrace.c +++ b/vm_backtrace.c @@ -20,6 +20,7 @@ #include "iseq.h" #include "ruby/debug.h" #include "ruby/encoding.h" +#include "ruby/internal/intern/io.h" #include "vm_core.h" #include "zjit.h" @@ -412,6 +413,67 @@ location_node_id(rb_backtrace_location_t *loc) extern VALUE rb_e_script; +static bool +location_source_end_marker_p(const uint8_t *line, size_t length) +{ + return (length == 7 && memcmp(line, "__END__", 7) == 0) || + (length == 8 && memcmp(line, "__END__\n", 8) == 0) || + (length == 9 && memcmp(line, "__END__\r\n", 9) == 0); +} + +static bool +location_source_hash_matches(VALUE source, uint64_t source_hash) +{ + StringValue(source); + const uint8_t *bytes = (const uint8_t *)RSTRING_PTR(source); + size_t length = (size_t)RSTRING_LEN(source); + size_t line_start = 0; + rb_source_hash_state_t state; + rb_source_hash_init(&state); + + for (size_t index = 0; index < length; index++) { + if (bytes[index] != '\n') continue; + + size_t line_length = index + 1 - line_start; + rb_source_hash_update(&state, bytes + line_start, line_length); + if (location_source_end_marker_p(bytes + line_start, line_length) && + rb_source_hash_finalize(&state) == source_hash) { + return true; + } + line_start = index + 1; + } + + if (line_start < length) { + size_t line_length = length - line_start; + rb_source_hash_update(&state, bytes + line_start, line_length); + if (location_source_end_marker_p(bytes + line_start, line_length) && + rb_source_hash_finalize(&state) == source_hash) { + return true; + } + } + + return rb_source_hash_finalize(&state) == source_hash; +} + +static VALUE +location_source_read(VALUE io) +{ + VALUE source = rb_str_buf_new(0); + VALUE line; + + while (!NIL_P(line = rb_io_gets(io))) { + rb_str_buf_append(source, line); + } + return source; +} + +static VALUE +location_source_read_file(VALUE path) +{ + VALUE file = rb_file_open_str(path, "rb"); + return rb_ensure(location_source_read, file, rb_io_close, file); +} + static bool location_code_location_equal(const rb_code_location_t *left, const rb_code_location_t *right) { @@ -422,7 +484,7 @@ location_code_location_equal(const rb_code_location_t *left, const rb_code_locat } static bool -iseq_from_e_script_p(const rb_iseq_t *iseq, VALUE path) +iseq_from_e_script_p(const rb_iseq_t *iseq, VALUE path, uint64_t source_hash) { if (!RB_TYPE_P(path, T_STRING) || RSTRING_LEN(path) != 2 || @@ -430,6 +492,7 @@ iseq_from_e_script_p(const rb_iseq_t *iseq, VALUE path) !RTEST(rb_e_script)) { return false; } + if (!location_source_hash_matches(rb_e_script, source_hash)) return false; const rb_iseq_t *source_iseq = iseq; for (; source_iseq; source_iseq = ISEQ_BODY(source_iseq)->parent_iseq) { @@ -478,22 +541,21 @@ location_source_first_lineno(const rb_iseq_t *iseq, VALUE script_lines) /* * call-seq: - * location.source_range -> Ruby::SourceRange or nil + * location.source_range -> Ruby::SourceRange * * Returns the Ruby::SourceRange for the Ruby expression associated with this - * backtrace location, or +nil+ when the location is not available - * (e.g., the source is not Ruby code). + * backtrace location. * - * This method requires re-reading the source file from the filesystem - * (since this information is not kept in the bytecode to avoid memory overhead). - * Errno::ENOENT if the source file no longer exists. - * RuntimeError is raised if the file has been modified. + * On CRuby, this method re-reads and re-parses the source file to determine + * the range. File errors encountered while reading the source are propagated. + * RuntimeError is raised if required source location information is + * unavailable, or if the source has changed. * - * On CRuby, `RubyVM.keep_script_lines = true` can be used to avoid to re-read - * source files from the filesystem, however this will increase memory usage, - * by keeping all source files in memory. + * RubyVM.keep_script_lines = true can be used to retain source files in + * memory and avoid re-reading them from the filesystem. * - * Locations from eval'd code are only available with `RubyVM.keep_script_lines = true`. + * Locations from eval'd code are only available with + * RubyVM.keep_script_lines = true. */ static VALUE location_source_range_m(VALUE self) @@ -501,27 +563,36 @@ location_source_range_m(VALUE self) #ifdef USE_ISEQ_NODE_ID rb_backtrace_location_t *backtrace_location = location_ptr(self); const rb_iseq_t *iseq = location_iseq(backtrace_location); - if (!iseq) return Qnil; + if (!iseq) { + rb_raise(rb_eRuntimeError, "cannot get source range for location without Ruby bytecode"); + } rb_iseq_check(iseq); int node_id = location_node_id(backtrace_location); - if (node_id == -1) return Qnil; + if (node_id == -1) { + rb_raise(rb_eRuntimeError, "cannot get source range for location without a node ID"); + } + if (!ISEQ_BODY(iseq)->has_source_hash) { + rb_raise(rb_eRuntimeError, "cannot get source range because the source hash is unavailable"); + } + uint64_t source_hash = ISEQ_BODY(iseq)->source_hash; VALUE path = rb_iseq_path(iseq); VALUE absolute_path = rb_iseq_realpath(iseq); VALUE script_lines = ISEQ_BODY(iseq)->variable.script_lines; - VALUE source = script_lines; + VALUE source; VALUE parser_path = path; int first_lineno = 1; if (!NIL_P(script_lines)) { + source = rb_ary_join(script_lines, Qnil); first_lineno = location_source_first_lineno(iseq, script_lines); } - else if (iseq_from_e_script_p(iseq, path)) { + else if (iseq_from_e_script_p(iseq, path, source_hash)) { source = rb_e_script; } else if (!NIL_P(absolute_path)) { - source = Qnil; + source = location_source_read_file(absolute_path); parser_path = absolute_path; } else { @@ -531,15 +602,21 @@ location_source_range_m(VALUE self) if (NIL_P(parser_path)) { parser_path = rb_str_new_cstr("(eval)"); } + if (!location_source_hash_matches(source, source_hash)) { + rb_raise(rb_eRuntimeError, "source has been modified"); + } rb_code_location_t code_location; bool found; if (ISEQ_BODY(iseq)->prism) { - if (RB_TYPE_P(source, T_ARRAY)) { - source = rb_ary_join(source, Qnil); - } - found = pm_node_source_location(source, parser_path, first_lineno, node_id, &code_location); + found = pm_node_source_location( + source, + parser_path, + first_lineno, + node_id, + &code_location + ); } else { found = rb_ast_node_source_location( @@ -559,7 +636,7 @@ location_source_range_m(VALUE self) return rb_source_range_new(path, absolute_path, &code_location); #else - return Qnil; + rb_raise(rb_eRuntimeError, "cannot get source range because node IDs are disabled"); #endif } From a89b3a6fd5cb5f382adfef7d28dfc73e330f614d Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Fri, 24 Jul 2026 14:57:48 +0200 Subject: [PATCH 28/30] [Bug #22197] Show the original definition module in backtrace labels An alias or a method installed via define_method(UnboundMethod) shares the original method definition, but the CME's owner and defined_class point at the site where the copy was installed. Combined with the method name (taken from the original definition), backtraces reported a "Class#method" pair that never existed: an alias in a subclass was shown as Child#original instead of Parent#original, and define_method(Original.instance_method(:m)) was shown as A#m instead of Original#m. Recover the defining module only for shared definitions (def->aliased), leaving plain, singleton and class methods untouched. Apply it to both Location#label and the backtrace string built by location_to_str. Distinguish the singleton-class cases via the singleton's attached object: define_method(SomeModule.instance_method(:m)) installed on a singleton class must report SomeModule#m, while an aliased class method (def self.m), whose iseq cref holds the lexical class rather than the singleton it lives on, must keep its owner. Only keep the owner when it is the singleton class of the cref's class; otherwise the cref names the genuine definition site, so use it. Co-Authored-By: Claude Opus 4.8 --- .../backtrace/location/fixtures/classes.rb | 24 ++++++ .../thread/backtrace/location/label_spec.rb | 18 +++++ test/ruby/test_backtrace.rb | 65 ++++++++++++++++ vm_backtrace.c | 77 +++++++++++++++++-- 4 files changed, 176 insertions(+), 8 deletions(-) diff --git a/spec/ruby/core/thread/backtrace/location/fixtures/classes.rb b/spec/ruby/core/thread/backtrace/location/fixtures/classes.rb index 103c36b3a0ab04..b23794e54f1cc0 100644 --- a/spec/ruby/core/thread/backtrace/location/fixtures/classes.rb +++ b/spec/ruby/core/thread/backtrace/location/fixtures/classes.rb @@ -68,6 +68,30 @@ def instance_locations_inside_nested_block def original_method = LABEL.call alias_method :aliased_method, :original_method + # [Bug #22197]: an alias in a subclass, and define_method with an UnboundMethod + # from another module, should report the module where the body was originally + # defined -- not the subclass/class where the copy was installed. + class AliasParent + def alias_original = LABEL.call + end + class AliasChild < AliasParent + alias_method :alias_in_subclass, :alias_original + end + + module DefineMethodSource + def define_method_original = LABEL.call + end + class DefineMethodTarget + define_method(:defined_from_other_module, DefineMethodSource.instance_method(:define_method_original)) + end + class DefineMethodSingletonTarget; end + class << DefineMethodSingletonTarget + define_method(:defined_on_singleton, DefineMethodSource.instance_method(:define_method_original)) + end + class DefineMethodSameNameTarget + define_method(:define_method_original, DefineMethodSource.instance_method(:define_method_original)) + end + module M class C def regular_instance_method = LABEL.call diff --git a/spec/ruby/core/thread/backtrace/location/label_spec.rb b/spec/ruby/core/thread/backtrace/location/label_spec.rb index 5f6a7b73dfed1f..bc3a385c21fe34 100644 --- a/spec/ruby/core/thread/backtrace/location/label_spec.rb +++ b/spec/ruby/core/thread/backtrace/location/label_spec.rb @@ -124,6 +124,24 @@ def ThreadBacktraceLocationSpecs.def_singleton ThreadBacktraceLocationSpecs::INSTANCE.aliased_method.should == "ThreadBacktraceLocationSpecs#original_method" end + ruby_version_is "4.1" do # [Bug #22197] + it "shows the defining class for a method aliased in a subclass" do + ThreadBacktraceLocationSpecs::AliasChild.new.alias_in_subclass.should == "ThreadBacktraceLocationSpecs::AliasParent#alias_original" + end + + it "shows the source module for a method defined via define_method with an UnboundMethod from another module" do + ThreadBacktraceLocationSpecs::DefineMethodTarget.new.defined_from_other_module.should == "ThreadBacktraceLocationSpecs::DefineMethodSource#define_method_original" + end + + it "shows the source module for define_method with an UnboundMethod installed on a singleton class" do + ThreadBacktraceLocationSpecs::DefineMethodSingletonTarget.defined_on_singleton.should == "ThreadBacktraceLocationSpecs::DefineMethodSource#define_method_original" + end + + it "shows the source module for define_method with an UnboundMethod installed under its original name" do + ThreadBacktraceLocationSpecs::DefineMethodSameNameTarget.new.define_method_original.should == "ThreadBacktraceLocationSpecs::DefineMethodSource#define_method_original" + end + end + # A wide variety of cases. # These show interesting cases when trying to determine the name statically/at parse time describe "is correct for" do diff --git a/test/ruby/test_backtrace.rb b/test/ruby/test_backtrace.rb index 332d76c58e1596..3691eb8ec54529 100644 --- a/test/ruby/test_backtrace.rb +++ b/test/ruby/test_backtrace.rb @@ -2,6 +2,39 @@ require 'test/unit' require 'tempfile' +module Bug22197 + class Parent + def original + caller_locations(0, 1).first + end + end + + class Child < Parent + alias_method :aliased, :original + end + + module Original + def original + caller_locations(0, 1).first + end + end + + class A + define_method(:a, Original.instance_method(:original)) + end + + class WithClassMethod + def self.cm + caller_locations(0, 1).first + end + end + + class SingletonTarget; end + class << SingletonTarget + define_method(:on_singleton, Original.instance_method(:original)) + end +end + class TestBacktrace < Test::Unit::TestCase def test_exception bt = Fiber.new{ @@ -217,6 +250,38 @@ def self.label_caller end end + def test_original_definition_module # [Bug #22197] + # An alias in a subclass reports the module where the body was defined, + # not the subclass where the alias was installed. + loc = Bug22197::Child.new.aliased + assert_equal 'Bug22197::Parent#original', loc.label + assert_match(/:in 'Bug22197::Parent#original'\z/, loc.to_s) + + # define_method(UnboundMethod) reports the source module, not the target class. + loc = Bug22197::A.new.a + assert_equal 'Bug22197::Original#original', loc.label + assert_match(/:in 'Bug22197::Original#original'\z/, loc.to_s) + + # ... including when installed on a singleton class, where the target owner + # would otherwise render as a phantom "SingletonTarget.original". + loc = Bug22197::SingletonTarget.on_singleton + assert_equal 'Bug22197::Original#original', loc.label + assert_match(/:in 'Bug22197::Original#original'\z/, loc.to_s) + + # Regression guard: a plain class method keeps its own "Class.method" label + # rather than borrowing the lexical nesting from the iseq cref. + loc = Bug22197::WithClassMethod.cm + assert_equal 'Bug22197::WithClassMethod.cm', loc.label + assert_match(/:in 'Bug22197::WithClassMethod.cm'\z/, loc.to_s) + + # Regression guard: a plain singleton method keeps its bare label. + obj = Object.new + def obj.singleton_m + caller_locations(0, 1).first + end + assert_equal 'singleton_m', obj.singleton_m.label + end + def test_caller_limit_cfunc_iseq_no_pc def self.a; [1].group_by { b } end def self.b diff --git a/vm_backtrace.c b/vm_backtrace.c index b84ed105a7cc74..3744f096e8e59a 100644 --- a/vm_backtrace.c +++ b/vm_backtrace.c @@ -292,18 +292,79 @@ location_cfunc_p(rb_backtrace_location_t *loc) } } +/* Return the module where the running method body was actually defined. + * + * For an alias or a method installed via define_method(UnboundMethod), the CME's + * owner and defined_class point at the site where the alias/copy was installed, + * not where the body was originally defined. Combined with the method name (taken + * from the original definition) that yields a "Class#method" pair which never + * existed -- e.g. an alias in a subclass reported as Child#original instead of + * Parent#original, or define_method(Original.instance_method(:m)) reported as + * A#m instead of Original#m ([Bug #22197]). + * + * Only a shared definition (def->aliased) can carry a misleading owner, so plain + * defs -- including singleton (def obj.m) and class (def self.m) methods -- keep + * their owner untouched. For a shared def, rb_alias() copies the real module into + * defined_class (owner != defined_class), while define_method() overwrites both + * owner and defined_class with the install class, leaving the original module + * recoverable only from the iseq's cref. */ +static VALUE +location_original_defined_class(const rb_callable_method_entry_t *cme) +{ + if (!cme) return Qnil; + + const rb_method_entry_t *me = (const rb_method_entry_t *)cme; + + /* Unwrap the explicit alias/refined wrappers (the defined_class == 0 form). */ + while (me->def) { + if (me->def->type == VM_METHOD_TYPE_ALIAS) { + me = me->def->body.alias.original_me; + } + else if (me->def->type == VM_METHOD_TYPE_REFINED && me->def->body.refined.orig_me) { + me = me->def->body.refined.orig_me; + } + else { + break; + } + } + + VALUE owner = me->owner; + + if (me->def && me->def->aliased) { + VALUE defined_class = me->defined_class; + if (RB_TYPE_P(defined_class, T_ICLASS)) { + defined_class = RBASIC_CLASS(defined_class); + } + /* rb_alias() copied the real module here. */ + if (defined_class && !NIL_P(defined_class) && defined_class != owner) { + return defined_class; + } + /* define_method(UnboundMethod) overwrote owner and defined_class alike; + * the original module survives only on the iseq's cref. A body written + * as `def self.m` / `def obj.m` records its *lexical* class in cref, not + * the singleton class it lives on, so when owner is exactly the singleton + * of cref's class (e.g. an aliased class method) keep owner instead. */ + if (me->def->type == VM_METHOD_TYPE_ISEQ && me->def->body.iseq.cref) { + VALUE cref_class = CREF_CLASS(me->def->body.iseq.cref); + if (RB_TYPE_P(owner, T_CLASS) && RCLASS_SINGLETON_P(owner) && + RCLASS_ATTACHED_OBJECT(owner) == cref_class) { + return owner; + } + return cref_class; + } + } + + return owner; +} + static VALUE location_label(rb_backtrace_location_t *loc) { if (location_cfunc_p(loc)) { - return rb_gen_method_name(loc->cme->owner, rb_id2str(loc->cme->def->original_id)); + return rb_gen_method_name(location_original_defined_class(loc->cme), rb_id2str(loc->cme->def->original_id)); } else { - VALUE owner = Qnil; - if (loc->cme) { - owner = loc->cme->owner; - } - return calculate_iseq_label(owner, loc->iseq); + return calculate_iseq_label(location_original_defined_class(loc->cme), loc->iseq); } } /* @@ -713,13 +774,13 @@ location_to_str(rb_backtrace_location_t *loc) file = GET_VM()->progname; lineno = 0; } - name = rb_gen_method_name(loc->cme->owner, rb_id2str(loc->cme->def->original_id)); + name = rb_gen_method_name(location_original_defined_class(loc->cme), rb_id2str(loc->cme->def->original_id)); } else { file = rb_iseq_path(loc->iseq); lineno = calc_lineno(loc->iseq, loc->pc); if (loc->cme) { - owner = loc->cme->owner; + owner = location_original_defined_class(loc->cme); } name = calculate_iseq_label(owner, loc->iseq); } From c0ed960df16629602a5106d5b537799c9d7d3a46 Mon Sep 17 00:00:00 2001 From: Benoit Daloze Date: Thu, 16 Jul 2026 16:40:12 +0200 Subject: [PATCH 29/30] [Bug #22197] Record the definition module on the method definition Backtrace labels need the module where a method body was originally defined, but alias_method and define_method(UnboundMethod) overwrite the CME owner with the install site, producing "Class#method" pairs that never existed. The label code reconstructed the origin from a mix of owner, defined_class and the iseq cref with several heuristics. Store it directly instead: a new original_module field on rb_method_definition_t, set once when a body is created. Because the definition is reference-counted and shared, every alias/define_method copy inherits it, so the backtrace reader collapses to a field read. module_function is the one case that also installs an instance method's shared definition onto the module's singleton class; that copy is still labeled by its owner (M.f), detected as the copy whose owner is the singleton class of the definition module. Co-Authored-By: Claude Opus 4.8 --- imemo.c | 2 ++ method.h | 1 + vm_backtrace.c | 88 ++++++++++++++++---------------------------------- vm_method.c | 16 +++++++-- 4 files changed, 44 insertions(+), 63 deletions(-) diff --git a/imemo.c b/imemo.c index 4818876a3eae0e..62f9e5768dd28d 100644 --- a/imemo.c +++ b/imemo.c @@ -325,6 +325,8 @@ mark_and_move_method_entry(rb_method_entry_t *ment, bool reference_updating) rb_gc_mark_and_move(&ment->defined_class); if (def) { + rb_gc_mark_and_move(&def->original_module); + switch (def->type) { case VM_METHOD_TYPE_ISEQ: if (def->body.iseq.iseqptr) { diff --git a/method.h b/method.h index 660961a26d6fab..0c08f8d9529b02 100644 --- a/method.h +++ b/method.h @@ -203,6 +203,7 @@ struct rb_method_definition_struct { } body; ID original_id; + VALUE original_module; /* module in which the method definition is; see location_original_module() */ uintptr_t method_serial; const rb_box_t *box; }; diff --git a/vm_backtrace.c b/vm_backtrace.c index 3744f096e8e59a..91ca7f3f0188f7 100644 --- a/vm_backtrace.c +++ b/vm_backtrace.c @@ -295,76 +295,44 @@ location_cfunc_p(rb_backtrace_location_t *loc) /* Return the module where the running method body was actually defined. * * For an alias or a method installed via define_method(UnboundMethod), the CME's - * owner and defined_class point at the site where the alias/copy was installed, - * not where the body was originally defined. Combined with the method name (taken - * from the original definition) that yields a "Class#method" pair which never - * existed -- e.g. an alias in a subclass reported as Child#original instead of - * Parent#original, or define_method(Original.instance_method(:m)) reported as - * A#m instead of Original#m ([Bug #22197]). + * owner points at the site where the alias/copy was installed, not where the body + * was originally defined. Combined with the method name (taken from the original + * definition) that yields a "Class#method" pair which never existed -- e.g. an + * alias in a subclass reported as Child#original instead of Parent#original, or + * define_method(Original.instance_method(:m)) reported as A#m instead of + * Original#m ([Bug #22197]). * - * Only a shared definition (def->aliased) can carry a misleading owner, so plain - * defs -- including singleton (def obj.m) and class (def self.m) methods -- keep - * their owner untouched. For a shared def, rb_alias() copies the real module into - * defined_class (owner != defined_class), while define_method() overwrites both - * owner and defined_class with the install class, leaving the original module - * recoverable only from the iseq's cref. */ + * The definition module is recorded once on the (reference-counted, shared) + * method definition when the body is first created, so every alias/define_method + * copy keeps pointing at the original module. + * + * The exception is module_function, which installs the instance method's *shared* + * def onto the module's singleton class as well: that copy must be labeled as a + * class method of the module (M.f), i.e. by its owner. Such a copy is exactly the + * one whose owner is the singleton class of the definition module. */ static VALUE -location_original_defined_class(const rb_callable_method_entry_t *cme) +location_original_module(const rb_callable_method_entry_t *cme) { - if (!cme) return Qnil; - - const rb_method_entry_t *me = (const rb_method_entry_t *)cme; - - /* Unwrap the explicit alias/refined wrappers (the defined_class == 0 form). */ - while (me->def) { - if (me->def->type == VM_METHOD_TYPE_ALIAS) { - me = me->def->body.alias.original_me; - } - else if (me->def->type == VM_METHOD_TYPE_REFINED && me->def->body.refined.orig_me) { - me = me->def->body.refined.orig_me; - } - else { - break; - } + if (!cme || !cme->def) return Qnil; + VALUE owner = cme->owner; + VALUE defined_in = cme->def->original_module; + if (!defined_in) return owner; + if (defined_in != owner && + RB_TYPE_P(owner, T_CLASS) && RCLASS_SINGLETON_P(owner) && + RCLASS_ATTACHED_OBJECT(owner) == defined_in) { + return owner; } - - VALUE owner = me->owner; - - if (me->def && me->def->aliased) { - VALUE defined_class = me->defined_class; - if (RB_TYPE_P(defined_class, T_ICLASS)) { - defined_class = RBASIC_CLASS(defined_class); - } - /* rb_alias() copied the real module here. */ - if (defined_class && !NIL_P(defined_class) && defined_class != owner) { - return defined_class; - } - /* define_method(UnboundMethod) overwrote owner and defined_class alike; - * the original module survives only on the iseq's cref. A body written - * as `def self.m` / `def obj.m` records its *lexical* class in cref, not - * the singleton class it lives on, so when owner is exactly the singleton - * of cref's class (e.g. an aliased class method) keep owner instead. */ - if (me->def->type == VM_METHOD_TYPE_ISEQ && me->def->body.iseq.cref) { - VALUE cref_class = CREF_CLASS(me->def->body.iseq.cref); - if (RB_TYPE_P(owner, T_CLASS) && RCLASS_SINGLETON_P(owner) && - RCLASS_ATTACHED_OBJECT(owner) == cref_class) { - return owner; - } - return cref_class; - } - } - - return owner; + return defined_in; } static VALUE location_label(rb_backtrace_location_t *loc) { if (location_cfunc_p(loc)) { - return rb_gen_method_name(location_original_defined_class(loc->cme), rb_id2str(loc->cme->def->original_id)); + return rb_gen_method_name(location_original_module(loc->cme), rb_id2str(loc->cme->def->original_id)); } else { - return calculate_iseq_label(location_original_defined_class(loc->cme), loc->iseq); + return calculate_iseq_label(location_original_module(loc->cme), loc->iseq); } } /* @@ -774,13 +742,13 @@ location_to_str(rb_backtrace_location_t *loc) file = GET_VM()->progname; lineno = 0; } - name = rb_gen_method_name(location_original_defined_class(loc->cme), rb_id2str(loc->cme->def->original_id)); + name = rb_gen_method_name(location_original_module(loc->cme), rb_id2str(loc->cme->def->original_id)); } else { file = rb_iseq_path(loc->iseq); lineno = calc_lineno(loc->iseq, loc->pc); if (loc->cme) { - owner = location_original_defined_class(loc->cme); + owner = location_original_module(loc->cme); } name = calculate_iseq_label(owner, loc->iseq); } diff --git a/vm_method.c b/vm_method.c index cf4998f477c331..ac992db8909802 100644 --- a/vm_method.c +++ b/vm_method.c @@ -1153,12 +1153,18 @@ rb_method_definition_set(const rb_method_entry_t *me, rb_method_definition_t *de return; case VM_METHOD_TYPE_REFINED: { - RB_OBJ_WRITE(me, &def->body.refined.orig_me, (rb_method_entry_t *)opts); + const rb_method_entry_t *orig_me = (const rb_method_entry_t *)opts; + RB_OBJ_WRITE(me, &def->body.refined.orig_me, orig_me); + RB_OBJ_WRITE(me, &def->original_module, orig_me->def->original_module); return; } case VM_METHOD_TYPE_ALIAS: - RB_OBJ_WRITE(me, &def->body.alias.original_me, (rb_method_entry_t *)opts); - return; + { + const rb_method_entry_t *orig_me = (const rb_method_entry_t *)opts; + RB_OBJ_WRITE(me, &def->body.alias.original_me, orig_me); + RB_OBJ_WRITE(me, &def->original_module, orig_me->def->original_module); + return; + } case VM_METHOD_TYPE_ZSUPER: case VM_METHOD_TYPE_UNDEF: case VM_METHOD_TYPE_MISSING: @@ -1172,6 +1178,8 @@ method_definition_reset(const rb_method_entry_t *me) { rb_method_definition_t *def = me->def; + RB_OBJ_WRITTEN(me, Qundef, def->original_module); + switch (def->type) { case VM_METHOD_TYPE_ISEQ: RB_OBJ_WRITTEN(me, Qundef, def->body.iseq.iseqptr); @@ -1536,6 +1544,7 @@ rb_method_entry_make(VALUE klass, ID mid, VALUE defined_class, rb_method_visibil def->body.cfunc.invoker = ractor_safe_call_cfunc_m1; def->body.cfunc.argc = -1; } + RB_OBJ_WRITE(me, &def->original_module, me->owner); } rb_method_definition_set(me, def, opts); @@ -1663,6 +1672,7 @@ get_overloaded_cme(const rb_callable_method_entry_t *cme) RB_OBJ_WRITE(me, &def->body.iseq.cref, cme->def->body.iseq.cref); RB_OBJ_WRITE(me, &def->body.iseq.iseqptr, ISEQ_BODY(cme->def->body.iseq.iseqptr)->mandatory_only_iseq); + RB_OBJ_WRITE(me, &def->original_module, cme->def->original_module); ASSERT_vm_locking(); st_insert(overloaded_cme_table(), (st_data_t)cme, (st_data_t)me); From 2461caebba1d47ca79cfd4e0a825e6df40f375bd Mon Sep 17 00:00:00 2001 From: HASUMI Hitoshi Date: Thu, 6 Aug 2026 21:40:37 +0900 Subject: [PATCH 30/30] [Feature #22118] Introduce Basic Bit Operations into String (#17353) This patch adds the following methods to String class: * String#bit_get(offset, lsb_first: true) -> 1 | 0 | nil * String#bit_set?(offset, lsb_first: true) -> true | false | nil * String#bit_set(offset, lsb_first: true) -> self * String#bit_clear(offset, lsb_first: true) -> self * String#bit_flip(offset, lsb_first: true) -> self * String#bit_count -> Integer * String#bitwise_not -> String * String#bitwise_not! -> self * String#bitwise_and(other) -> String * String#bitwise_and!(other) -> self * String#bitwise_or(other) -> String * String#bitwise_or!(other) -> self * String#bitwise_xor(other) -> String * String#bitwise_xor!(other) -> self Other than implementation, tests, specs, and docs are added. Link: [Feature #22118] ## Note In `string.c`, I wrote some big macro that create method functions and helper functions: * STR_DEFINE_BINARY_BITWISE_METHOD * STR_DEFINE_UNARY_BITWISE_KERNEL * STR_DEFINE_BINARY_BITWISE_KERNEL While using macros like this reduces maintainability, I believe it's acceptable because there are no plans to extend the `bitwise_*` methods beyond this proposal, and the logic is stable. On the other hand, other methods such as `bit_get` and `bit_count` are planned to have argument extensions in the future. --- doc/string/bit_clear.rdoc | 16 + doc/string/bit_count.rdoc | 8 + doc/string/bit_flip.rdoc | 16 + doc/string/bit_get.rdoc | 20 + doc/string/bit_set.rdoc | 16 + doc/string/bit_set_p.rdoc | 20 + doc/string/bitwise_and.rdoc | 7 + doc/string/bitwise_and_bang.rdoc | 10 + doc/string/bitwise_not.rdoc | 5 + doc/string/bitwise_not_bang.rdoc | 7 + doc/string/bitwise_or.rdoc | 7 + doc/string/bitwise_or_bang.rdoc | 10 + doc/string/bitwise_xor.rdoc | 7 + doc/string/bitwise_xor_bang.rdoc | 10 + spec/ruby/core/string/bit_clear_spec.rb | 33 ++ spec/ruby/core/string/bit_count_spec.rb | 18 + spec/ruby/core/string/bit_flip_spec.rb | 35 ++ spec/ruby/core/string/bit_get_spec.rb | 38 ++ spec/ruby/core/string/bit_set_p_spec.rb | 38 ++ spec/ruby/core/string/bit_set_spec.rb | 33 ++ spec/ruby/core/string/bitwise_and_spec.rb | 41 ++ spec/ruby/core/string/bitwise_not_spec.rb | 31 ++ spec/ruby/core/string/bitwise_or_spec.rb | 41 ++ spec/ruby/core/string/bitwise_xor_spec.rb | 41 ++ string.c | 498 ++++++++++++++++++++++ test/ruby/test_string.rb | 113 +++++ 26 files changed, 1119 insertions(+) create mode 100644 doc/string/bit_clear.rdoc create mode 100644 doc/string/bit_count.rdoc create mode 100644 doc/string/bit_flip.rdoc create mode 100644 doc/string/bit_get.rdoc create mode 100644 doc/string/bit_set.rdoc create mode 100644 doc/string/bit_set_p.rdoc create mode 100644 doc/string/bitwise_and.rdoc create mode 100644 doc/string/bitwise_and_bang.rdoc create mode 100644 doc/string/bitwise_not.rdoc create mode 100644 doc/string/bitwise_not_bang.rdoc create mode 100644 doc/string/bitwise_or.rdoc create mode 100644 doc/string/bitwise_or_bang.rdoc create mode 100644 doc/string/bitwise_xor.rdoc create mode 100644 doc/string/bitwise_xor_bang.rdoc create mode 100644 spec/ruby/core/string/bit_clear_spec.rb create mode 100644 spec/ruby/core/string/bit_count_spec.rb create mode 100644 spec/ruby/core/string/bit_flip_spec.rb create mode 100644 spec/ruby/core/string/bit_get_spec.rb create mode 100644 spec/ruby/core/string/bit_set_p_spec.rb create mode 100644 spec/ruby/core/string/bit_set_spec.rb create mode 100644 spec/ruby/core/string/bitwise_and_spec.rb create mode 100644 spec/ruby/core/string/bitwise_not_spec.rb create mode 100644 spec/ruby/core/string/bitwise_or_spec.rb create mode 100644 spec/ruby/core/string/bitwise_xor_spec.rb diff --git a/doc/string/bit_clear.rdoc b/doc/string/bit_clear.rdoc new file mode 100644 index 00000000000000..737577258c2121 --- /dev/null +++ b/doc/string/bit_clear.rdoc @@ -0,0 +1,16 @@ +Sets the bit at zero-based bit +offset+ to 0; returns +self+: + + s = "\xFF" + s.bit_clear(1) # => "\xFD" + s # => "\xFD" + +By default, bits within each byte are numbered from least-significant to +most-significant. If +lsb_first+ is +false+, byte order is unchanged but bits +within each byte are numbered from most-significant to least-significant: + + s = "\xFF" + s.bit_clear(1, lsb_first: false) # => "\xBF" + +Raises +IndexError+ if +offset+ is out of range. +Raises +ArgumentError+ if +offset+ is too large to be represented. +Raises +ArgumentError+ if +lsb_first+ is neither +true+ nor +false+. diff --git a/doc/string/bit_count.rdoc b/doc/string/bit_count.rdoc new file mode 100644 index 00000000000000..021cf37101e6ae --- /dev/null +++ b/doc/string/bit_count.rdoc @@ -0,0 +1,8 @@ +Returns the number of set bits in +self+: + + "\x00".bit_count # => 0 + "\xFF".bit_count # => 8 + "\xAA".bit_count # => 4 + +The count is over the bytes of +self+ and is independent of string encoding. +Raises +ArgumentError+ if any argument is given. diff --git a/doc/string/bit_flip.rdoc b/doc/string/bit_flip.rdoc new file mode 100644 index 00000000000000..7a480b03d6f532 --- /dev/null +++ b/doc/string/bit_flip.rdoc @@ -0,0 +1,16 @@ +Flips the bit at zero-based bit +offset+; returns +self+: + + s = "\x00" + s.bit_flip(1) # => "\x02" + s.bit_flip(1) # => "\x00" + +By default, bits within each byte are numbered from least-significant to +most-significant. If +lsb_first+ is +false+, byte order is unchanged but bits +within each byte are numbered from most-significant to least-significant: + + s = "\x00" + s.bit_flip(1, lsb_first: false) # => "\x40" + +Raises +IndexError+ if +offset+ is out of range. +Raises +ArgumentError+ if +offset+ is too large to be represented. +Raises +ArgumentError+ if +lsb_first+ is neither +true+ nor +false+. diff --git a/doc/string/bit_get.rdoc b/doc/string/bit_get.rdoc new file mode 100644 index 00000000000000..fb8da5644cdc78 --- /dev/null +++ b/doc/string/bit_get.rdoc @@ -0,0 +1,20 @@ +Returns +0+ or +1+ for the bit at zero-based bit +offset+: + + s = "\xAA" # 0b10101010 + s.bit_get(0) # => 0 + s.bit_get(1) # => 1 + +Returns +nil+ if +offset+ is beyond the end of +self+: + + s.bit_get(8) # => nil + +By default, bits within each byte are numbered from least-significant to +most-significant. If +lsb_first+ is +false+, byte order is unchanged but bits +within each byte are numbered from most-significant to least-significant: + + s.bit_get(0, lsb_first: false) # => 1 + s.bit_get(1, lsb_first: false) # => 0 + +Raises +IndexError+ if +offset+ is negative. +Raises +ArgumentError+ if +offset+ is too large to be represented. +Raises +ArgumentError+ if +lsb_first+ is neither +true+ nor +false+. diff --git a/doc/string/bit_set.rdoc b/doc/string/bit_set.rdoc new file mode 100644 index 00000000000000..82c4cb25e4ca0b --- /dev/null +++ b/doc/string/bit_set.rdoc @@ -0,0 +1,16 @@ +Sets the bit at zero-based bit +offset+ to 1; returns +self+: + + s = "\x00" + s.bit_set(1) # => "\x02" + s # => "\x02" + +By default, bits within each byte are numbered from least-significant to +most-significant. If +lsb_first+ is +false+, byte order is unchanged but bits +within each byte are numbered from most-significant to least-significant: + + s = "\x00" + s.bit_set(1, lsb_first: false) # => "\x40" + +Raises +IndexError+ if +offset+ is out of range. +Raises +ArgumentError+ if +offset+ is too large to be represented. +Raises +ArgumentError+ if +lsb_first+ is neither +true+ nor +false+. diff --git a/doc/string/bit_set_p.rdoc b/doc/string/bit_set_p.rdoc new file mode 100644 index 00000000000000..2d70feeaf38b5e --- /dev/null +++ b/doc/string/bit_set_p.rdoc @@ -0,0 +1,20 @@ +Returns +true+ or +false+ for whether the bit at zero-based bit +offset+ is set: + + s = "\xAA" # 0b10101010 + s.bit_set?(0) # => false + s.bit_set?(1) # => true + +Returns +nil+ if +offset+ is beyond the end of +self+: + + s.bit_set?(8) # => nil + +By default, bits within each byte are numbered from least-significant to +most-significant. If +lsb_first+ is +false+, byte order is unchanged but bits +within each byte are numbered from most-significant to least-significant: + + s.bit_set?(0, lsb_first: false) # => true + s.bit_set?(1, lsb_first: false) # => false + +Raises +IndexError+ if +offset+ is negative. +Raises +ArgumentError+ if +offset+ is too large to be represented. +Raises +ArgumentError+ if +lsb_first+ is neither +true+ nor +false+. diff --git a/doc/string/bitwise_and.rdoc b/doc/string/bitwise_and.rdoc new file mode 100644 index 00000000000000..535f0fef872c94 --- /dev/null +++ b/doc/string/bitwise_and.rdoc @@ -0,0 +1,7 @@ +Returns a new string whose bytes are the bitwise AND of +self+ and +other+: + + "\xF0".bitwise_and("\xCC") # => "\xC0" + ++other+ is converted to a string using +to_str+. +Raises +ArgumentError+ if the two strings have different byte sizes. +The returned string has BINARY encoding. diff --git a/doc/string/bitwise_and_bang.rdoc b/doc/string/bitwise_and_bang.rdoc new file mode 100644 index 00000000000000..913ca467fd0d28 --- /dev/null +++ b/doc/string/bitwise_and_bang.rdoc @@ -0,0 +1,10 @@ +Replaces each byte in +self+ with the bitwise AND of that byte and the +corresponding byte in +other+; returns +self+: + + s = "\xF0" + s.bitwise_and!("\xCC") # => "\xC0" + s # => "\xC0" + ++other+ is converted to a string using +to_str+. +Raises +ArgumentError+ if the two strings have different byte sizes. +The encoding of +self+ is not changed. diff --git a/doc/string/bitwise_not.rdoc b/doc/string/bitwise_not.rdoc new file mode 100644 index 00000000000000..b13b1f69824d7d --- /dev/null +++ b/doc/string/bitwise_not.rdoc @@ -0,0 +1,5 @@ +Returns a new string whose bytes are the bitwise complement of +self+: + + "\x00\xAA".bitwise_not # => "\xFF\x55" + +The returned string has BINARY encoding. diff --git a/doc/string/bitwise_not_bang.rdoc b/doc/string/bitwise_not_bang.rdoc new file mode 100644 index 00000000000000..336faeb3a4d10f --- /dev/null +++ b/doc/string/bitwise_not_bang.rdoc @@ -0,0 +1,7 @@ +Replaces each byte in +self+ with its bitwise complement; returns +self+: + + s = "\x00\xAA" + s.bitwise_not! # => "\xFF\x55" + s # => "\xFF\x55" + +The encoding of +self+ is not changed. diff --git a/doc/string/bitwise_or.rdoc b/doc/string/bitwise_or.rdoc new file mode 100644 index 00000000000000..58349de44a217e --- /dev/null +++ b/doc/string/bitwise_or.rdoc @@ -0,0 +1,7 @@ +Returns a new string whose bytes are the bitwise OR of +self+ and +other+: + + "\xF0".bitwise_or("\x0C") # => "\xFC" + ++other+ is converted to a string using +to_str+. +Raises +ArgumentError+ if the two strings have different byte sizes. +The returned string has BINARY encoding. diff --git a/doc/string/bitwise_or_bang.rdoc b/doc/string/bitwise_or_bang.rdoc new file mode 100644 index 00000000000000..1fa52da3d2cdb0 --- /dev/null +++ b/doc/string/bitwise_or_bang.rdoc @@ -0,0 +1,10 @@ +Replaces each byte in +self+ with the bitwise OR of that byte and the +corresponding byte in +other+; returns +self+: + + s = "\xF0" + s.bitwise_or!("\x0C") # => "\xFC" + s # => "\xFC" + ++other+ is converted to a string using +to_str+. +Raises +ArgumentError+ if the two strings have different byte sizes. +The encoding of +self+ is not changed. diff --git a/doc/string/bitwise_xor.rdoc b/doc/string/bitwise_xor.rdoc new file mode 100644 index 00000000000000..44dc61186ba8a9 --- /dev/null +++ b/doc/string/bitwise_xor.rdoc @@ -0,0 +1,7 @@ +Returns a new string whose bytes are the bitwise XOR of +self+ and +other+: + + "\xF0".bitwise_xor("\xCC") # => "\x3C" + ++other+ is converted to a string using +to_str+. +Raises +ArgumentError+ if the two strings have different byte sizes. +The returned string has BINARY encoding. diff --git a/doc/string/bitwise_xor_bang.rdoc b/doc/string/bitwise_xor_bang.rdoc new file mode 100644 index 00000000000000..f0fbdcdd9a2a99 --- /dev/null +++ b/doc/string/bitwise_xor_bang.rdoc @@ -0,0 +1,10 @@ +Replaces each byte in +self+ with the bitwise XOR of that byte and the +corresponding byte in +other+; returns +self+: + + s = "\xF0" + s.bitwise_xor!("\xCC") # => "\x3C" + s # => "\x3C" + ++other+ is converted to a string using +to_str+. +Raises +ArgumentError+ if the two strings have different byte sizes. +The encoding of +self+ is not changed. diff --git a/spec/ruby/core/string/bit_clear_spec.rb b/spec/ruby/core/string/bit_clear_spec.rb new file mode 100644 index 00000000000000..1e8851d93231ec --- /dev/null +++ b/spec/ruby/core/string/bit_clear_spec.rb @@ -0,0 +1,33 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bit_clear" do + it "clears a bit in LSB-first order by default and returns self" do + str = +"\xFF" + str.bit_clear(1).should.equal?(str) + str.should == "\xFD" + end + + it "clears a bit in MSB-first order" do + str = +"\xFF" + str.bit_clear(1, lsb_first: false) + str.should == "\xBF" + end + + it "preserves byte order when using MSB-first order" do + str = +"\xFF\xFF" + str.bit_clear(8, lsb_first: false) + str.should == "\xFF\x7F" + end + + it "raises an IndexError for an out of range bit offset" do + -> { "\x00".bit_clear(8) }.should.raise(IndexError) + -> { "\x00".bit_clear(-1) }.should.raise(IndexError) + end + + it "raises a FrozenError if self is frozen" do + -> { "\x00".freeze.bit_clear(0) }.should.raise(FrozenError) + end + end +end diff --git a/spec/ruby/core/string/bit_count_spec.rb b/spec/ruby/core/string/bit_count_spec.rb new file mode 100644 index 00000000000000..3c77d6f7f20cfb --- /dev/null +++ b/spec/ruby/core/string/bit_count_spec.rb @@ -0,0 +1,18 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bit_count" do + it "returns the number of set bits in the string" do + "".bit_count.should == 0 + "\x00".bit_count.should == 0 + "\xFF".bit_count.should == 8 + "\xAA\xF0".bit_count.should == 8 + end + + it "raises an ArgumentError when given an argument" do + -> { "\x00".bit_count(0) }.should.raise(ArgumentError) + -> { "\x00".bit_count(lsb_first: false) }.should.raise(ArgumentError) + end + end +end diff --git a/spec/ruby/core/string/bit_flip_spec.rb b/spec/ruby/core/string/bit_flip_spec.rb new file mode 100644 index 00000000000000..7f443d064575ff --- /dev/null +++ b/spec/ruby/core/string/bit_flip_spec.rb @@ -0,0 +1,35 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bit_flip" do + it "flips a bit in LSB-first order by default and returns self" do + str = +"\x00" + str.bit_flip(1).should.equal?(str) + str.should == "\x02" + str.bit_flip(1) + str.should == "\x00" + end + + it "flips a bit in MSB-first order" do + str = +"\x00" + str.bit_flip(1, lsb_first: false) + str.should == "\x40" + end + + it "preserves byte order when using MSB-first order" do + str = +"\x00\x00" + str.bit_flip(8, lsb_first: false) + str.should == "\x00\x80" + end + + it "raises an IndexError for an out of range bit offset" do + -> { "\x00".bit_flip(8) }.should.raise(IndexError) + -> { "\x00".bit_flip(-1) }.should.raise(IndexError) + end + + it "raises a FrozenError if self is frozen" do + -> { "\x00".freeze.bit_flip(0) }.should.raise(FrozenError) + end + end +end diff --git a/spec/ruby/core/string/bit_get_spec.rb b/spec/ruby/core/string/bit_get_spec.rb new file mode 100644 index 00000000000000..e008171e00a852 --- /dev/null +++ b/spec/ruby/core/string/bit_get_spec.rb @@ -0,0 +1,38 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bit_get" do + it "returns 0 or 1 for a bit offset in LSB-first order by default" do + str = "\xAA" + str.bit_get(0).should == 0 + str.bit_get(1).should == 1 + str.bit_get(7).should == 1 + end + + it "returns 0 or 1 for a bit offset in MSB-first order" do + str = "\xAA" + str.bit_get(0, lsb_first: false).should == 1 + str.bit_get(1, lsb_first: false).should == 0 + str.bit_get(7, lsb_first: false).should == 0 + end + + it "preserves byte order when using MSB-first order" do + str = "\x00\x80" + str.bit_get(8, lsb_first: false).should == 1 + end + + it "returns nil for a bit offset beyond the string" do + "\x00".bit_get(8).should == nil + "".bit_get(0).should == nil + end + + it "raises an IndexError for a negative bit offset" do + -> { "\x00".bit_get(-1) }.should.raise(IndexError) + end + + it "raises an ArgumentError for an invalid lsb_first value" do + -> { "\x00".bit_get(0, lsb_first: nil) }.should.raise(ArgumentError) + end + end +end diff --git a/spec/ruby/core/string/bit_set_p_spec.rb b/spec/ruby/core/string/bit_set_p_spec.rb new file mode 100644 index 00000000000000..5235dff6116555 --- /dev/null +++ b/spec/ruby/core/string/bit_set_p_spec.rb @@ -0,0 +1,38 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bit_set?" do + it "returns true or false for a bit offset in LSB-first order by default" do + str = "\xAA" + str.bit_set?(0).should == false + str.bit_set?(1).should == true + str.bit_set?(7).should == true + end + + it "returns true or false for a bit offset in MSB-first order" do + str = "\xAA" + str.bit_set?(0, lsb_first: false).should == true + str.bit_set?(1, lsb_first: false).should == false + str.bit_set?(7, lsb_first: false).should == false + end + + it "preserves byte order when using MSB-first order" do + str = "\x00\x80" + str.bit_set?(8, lsb_first: false).should == true + end + + it "returns nil for a bit offset beyond the string" do + "\x00".bit_set?(8).should == nil + "".bit_set?(0).should == nil + end + + it "raises an IndexError for a negative bit offset" do + -> { "\x00".bit_set?(-1) }.should.raise(IndexError) + end + + it "raises an ArgumentError for an invalid lsb_first value" do + -> { "\x00".bit_set?(0, lsb_first: nil) }.should.raise(ArgumentError) + end + end +end diff --git a/spec/ruby/core/string/bit_set_spec.rb b/spec/ruby/core/string/bit_set_spec.rb new file mode 100644 index 00000000000000..1f44dc7f801a5c --- /dev/null +++ b/spec/ruby/core/string/bit_set_spec.rb @@ -0,0 +1,33 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bit_set" do + it "sets a bit in LSB-first order by default and returns self" do + str = +"\x00" + str.bit_set(1).should.equal?(str) + str.should == "\x02" + end + + it "sets a bit in MSB-first order" do + str = +"\x00" + str.bit_set(1, lsb_first: false) + str.should == "\x40" + end + + it "preserves byte order when using MSB-first order" do + str = +"\x00\x00" + str.bit_set(8, lsb_first: false) + str.should == "\x00\x80" + end + + it "raises an IndexError for an out of range bit offset" do + -> { "\x00".bit_set(8) }.should.raise(IndexError) + -> { "\x00".bit_set(-1) }.should.raise(IndexError) + end + + it "raises a FrozenError if self is frozen" do + -> { "\x00".freeze.bit_set(0) }.should.raise(FrozenError) + end + end +end diff --git a/spec/ruby/core/string/bitwise_and_spec.rb b/spec/ruby/core/string/bitwise_and_spec.rb new file mode 100644 index 00000000000000..c6c70479eaf5d9 --- /dev/null +++ b/spec/ruby/core/string/bitwise_and_spec.rb @@ -0,0 +1,41 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bitwise_and" do + it "returns a new string containing the byte-wise AND with another string" do + str = "\xF0" + result = str.bitwise_and("\xCC") + result.should == "\xC0".b + result.should_not.equal?(str) + str.should == "\xF0" + end + + it "converts the argument with to_str" do + other = mock("string") + other.should_receive(:to_str).and_return("\xCC") + "\xF0".bitwise_and(other).should == "\xC0".b + end + + it "raises an ArgumentError if byte sizes differ" do + -> { "\xF0".bitwise_and("") }.should.raise(ArgumentError) + -> { "\xF0".bitwise_and("\x00\x00") }.should.raise(ArgumentError) + end + + it "returns a BINARY string" do + (+"\xF0").force_encoding("UTF-8").bitwise_and("\xCC").encoding.should == Encoding::BINARY + end + end + + describe "String#bitwise_and!" do + it "replaces self with the byte-wise AND and returns self" do + str = +"\xF0" + str.bitwise_and!("\xCC").should.equal?(str) + str.should == "\xC0" + end + + it "raises a FrozenError if self is frozen" do + -> { "\x00".freeze.bitwise_and!("\x00") }.should.raise(FrozenError) + end + end +end diff --git a/spec/ruby/core/string/bitwise_not_spec.rb b/spec/ruby/core/string/bitwise_not_spec.rb new file mode 100644 index 00000000000000..b584ec0c33d011 --- /dev/null +++ b/spec/ruby/core/string/bitwise_not_spec.rb @@ -0,0 +1,31 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bitwise_not" do + it "returns a new string with every bit inverted" do + str = "\x00\xAA" + result = str.bitwise_not + result.should == "\xFF\x55".b + result.should_not.equal?(str) + str.should == "\x00\xAA" + end + + it "returns a BINARY string" do + str = (+"\x00").force_encoding("US-ASCII") + str.bitwise_not.encoding.should == Encoding::BINARY + end + end + + describe "String#bitwise_not!" do + it "inverts every bit in self and returns self" do + str = +"\x00\xAA" + str.bitwise_not!.should.equal?(str) + str.should == "\xFF\x55" + end + + it "raises a FrozenError if self is frozen" do + -> { "\x00".freeze.bitwise_not! }.should.raise(FrozenError) + end + end +end diff --git a/spec/ruby/core/string/bitwise_or_spec.rb b/spec/ruby/core/string/bitwise_or_spec.rb new file mode 100644 index 00000000000000..eda73b1bccb937 --- /dev/null +++ b/spec/ruby/core/string/bitwise_or_spec.rb @@ -0,0 +1,41 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bitwise_or" do + it "returns a new string containing the byte-wise OR with another string" do + str = "\xF0" + result = str.bitwise_or("\x0C") + result.should == "\xFC".b + result.should_not.equal?(str) + str.should == "\xF0" + end + + it "converts the argument with to_str" do + other = mock("string") + other.should_receive(:to_str).and_return("\x0C") + "\xF0".bitwise_or(other).should == "\xFC".b + end + + it "raises an ArgumentError if byte sizes differ" do + -> { "\xF0".bitwise_or("") }.should.raise(ArgumentError) + -> { "\xF0".bitwise_or("\x00\x00") }.should.raise(ArgumentError) + end + + it "returns a BINARY string" do + (+"\xF0").force_encoding("UTF-8").bitwise_or("\x0C").encoding.should == Encoding::BINARY + end + end + + describe "String#bitwise_or!" do + it "replaces self with the byte-wise OR and returns self" do + str = +"\xF0" + str.bitwise_or!("\x0C").should.equal?(str) + str.should == "\xFC" + end + + it "raises a FrozenError if self is frozen" do + -> { "\x00".freeze.bitwise_or!("\x00") }.should.raise(FrozenError) + end + end +end diff --git a/spec/ruby/core/string/bitwise_xor_spec.rb b/spec/ruby/core/string/bitwise_xor_spec.rb new file mode 100644 index 00000000000000..3b58513c3f9b54 --- /dev/null +++ b/spec/ruby/core/string/bitwise_xor_spec.rb @@ -0,0 +1,41 @@ +# encoding: binary +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "String#bitwise_xor" do + it "returns a new string containing the byte-wise XOR with another string" do + str = "\xF0" + result = str.bitwise_xor("\xCC") + result.should == "\x3C".b + result.should_not.equal?(str) + str.should == "\xF0" + end + + it "converts the argument with to_str" do + other = mock("string") + other.should_receive(:to_str).and_return("\xCC") + "\xF0".bitwise_xor(other).should == "\x3C".b + end + + it "raises an ArgumentError if byte sizes differ" do + -> { "\xF0".bitwise_xor("") }.should.raise(ArgumentError) + -> { "\xF0".bitwise_xor("\x00\x00") }.should.raise(ArgumentError) + end + + it "returns a BINARY string" do + (+"\xF0").force_encoding("UTF-8").bitwise_xor("\xCC").encoding.should == Encoding::BINARY + end + end + + describe "String#bitwise_xor!" do + it "replaces self with the byte-wise XOR and returns self" do + str = +"\xF0" + str.bitwise_xor!("\xCC").should.equal?(str) + str.should == "\x3C" + end + + it "raises a FrozenError if self is frozen" do + -> { "\x00".freeze.bitwise_xor!("\x00") }.should.raise(FrozenError) + end + end +end diff --git a/string.c b/string.c index 9488baeda468c4..18c31812c71623 100644 --- a/string.c +++ b/string.c @@ -26,6 +26,7 @@ #include "id.h" #include "internal.h" #include "internal/array.h" +#include "internal/bits.h" #include "internal/compar.h" #include "internal/compilers.h" #include "internal/concurrent_set.h" @@ -6758,6 +6759,489 @@ rb_str_setbyte(VALUE str, VALUE index, VALUE value) return value; } +static inline bool +str_bit_offset_out_of_range(long byte_len, uint64_t bit_offset) +{ + /* Compare byte indexes to avoid overflowing byte_len * CHAR_BIT. */ + return bit_offset / CHAR_BIT >= (uint64_t)byte_len; +} + +/* + * Keep both the full bit offset and its long representation. Most calls use a + * Fixnum-sized offset and can stay on the original long fast path; only large + * Bignum offsets need the uint64_t path below. This matters on platforms + * where long is narrower than the address space, such as 32-bit and LLP64. + */ +struct str_bit_offset { + uint64_t value; + long long_value; + bool fits_long; +}; + +static inline struct str_bit_offset +str_bit_offset_from_index(VALUE index) +{ + VALUE integer = rb_to_int(index); + struct str_bit_offset offset; + + /* + * FIXNUM_P only decides whether the common long path is immediately usable. + * This covers practically all offsets on LP64 platforms; Bignum offsets + * are still accepted below when they fit in uint64_t, mainly for platforms + * with 32-bit long where large strings can have Bignum bit offsets. + */ + if (FIXNUM_P(integer)) { + offset.long_value = FIX2LONG(integer); + if (offset.long_value < 0) { + rb_raise(rb_eIndexError, "bit index out of range"); + } + offset.value = (uint64_t)offset.long_value; + offset.fits_long = true; + return offset; + } + + RUBY_ASSERT(RB_TYPE_P(integer, T_BIGNUM)); + if (rb_int_negative_p(integer)) { + rb_raise(rb_eIndexError, "bit index out of range"); + } + if (rb_cmpint(rb_int_cmp(integer, ULL2NUM(UINT64_MAX)), integer, ULL2NUM(UINT64_MAX)) > 0) { + rb_raise(rb_eArgError, "bit index out of representable range"); + } + + offset.value = (uint64_t)NUM2ULL(integer); + if (offset.value <= (uint64_t)LONG_MAX) { + offset.long_value = (long)offset.value; + offset.fits_long = true; + } + else { + offset.long_value = 0; + offset.fits_long = false; + } + return offset; +} + +static bool +str_lsb_first(int argc, VALUE *argv, VALUE *index) +{ + static ID keywords[1]; + VALUE opts, vlsb_first; + + if (!keywords[0]) { + keywords[0] = rb_intern_const("lsb_first"); + } + + rb_scan_args(argc, argv, "1:", index, &opts); + rb_get_kwargs(opts, keywords, 0, 1, &vlsb_first); + if (vlsb_first == Qundef || vlsb_first == Qtrue) { + return true; + } + if (vlsb_first == Qfalse) { + return false; + } + rb_raise(rb_eArgError, "lsb_first must be true or false"); + UNREACHABLE_RETURN(false); +} + +static inline uint64_t +str_logical_to_physical_bit64(uint64_t logical, bool lsb_first) +{ + return lsb_first ? logical : ((logical & ~(uint64_t)7) | (7 - (logical & 7))); +} + +static inline long +str_logical_to_physical_bit(long logical, bool lsb_first) +{ + return lsb_first ? logical : ((logical & ~7L) | (7 - (logical & 7L))); +} + +struct str_bit_location { + long byte_index; + unsigned int bit_offset; +}; + +static inline struct str_bit_location +str_bit_location_from_offset(uint64_t logical, bool lsb_first) +{ + /* + * When long is 32-bit, a bit offset for a large string can be a Bignum + * while the byte index still fits in long, which is RSTRING_LEN's type. + */ + uint64_t physical = str_logical_to_physical_bit64(logical, lsb_first); + struct str_bit_location location; + location.byte_index = (long)(physical / CHAR_BIT); + location.bit_offset = (unsigned int)(physical % CHAR_BIT); + return location; +} + +static inline int +str_get_bit(const char *ptr, long bit_index) +{ + return (((unsigned char)ptr[bit_index / CHAR_BIT]) >> (bit_index % CHAR_BIT)) & 1; +} + +static inline int +str_get_bit_location(const char *ptr, struct str_bit_location location) +{ + return (((unsigned char)ptr[location.byte_index]) >> location.bit_offset) & 1; +} + +static int +str_bit_get(int argc, VALUE *argv, VALUE str) +{ + VALUE index; + bool lsb_first = str_lsb_first(argc, argv, &index); + struct str_bit_offset offset = str_bit_offset_from_index(index); + + if (str_bit_offset_out_of_range(RSTRING_LEN(str), offset.value)) { + return -1; + } + + if (offset.fits_long) { + return str_get_bit(RSTRING_PTR(str), str_logical_to_physical_bit(offset.long_value, lsb_first)); + } + else { + return str_get_bit_location(RSTRING_PTR(str), str_bit_location_from_offset(offset.value, lsb_first)); + } +} + +/* + * call-seq: + * bit_get(offset, lsb_first: true) -> 0, 1, or nil + * + * :include: doc/string/bit_get.rdoc + * + */ +static VALUE +rb_str_bit_get(int argc, VALUE *argv, VALUE str) +{ + int bit = str_bit_get(argc, argv, str); + return bit < 0 ? Qnil : INT2FIX(bit); +} + +/* + * call-seq: + * bit_set?(offset, lsb_first: true) -> true, false, or nil + * + * :include: doc/string/bit_set_p.rdoc + * + */ +static VALUE +rb_str_bit_set_p(int argc, VALUE *argv, VALUE str) +{ + int bit = str_bit_get(argc, argv, str); + return bit < 0 ? Qnil : RBOOL(bit); +} + +enum str_bit_mutation { + STR_BIT_SET, + STR_BIT_CLEAR, + STR_BIT_FLIP +}; + +static VALUE +str_mutate_bit(int argc, VALUE *argv, VALUE str, enum str_bit_mutation mutation) +{ + VALUE index; + bool lsb_first = str_lsb_first(argc, argv, &index); + struct str_bit_offset offset = str_bit_offset_from_index(index); + struct str_bit_location location; + long bit_index; + unsigned char *ptr; + unsigned char mask; + + if (str_bit_offset_out_of_range(RSTRING_LEN(str), offset.value)) { + rb_raise(rb_eIndexError, "bit index out of range"); + } + + rb_str_modify(str); + ptr = (unsigned char *)RSTRING_PTR(str); + if (offset.fits_long) { + bit_index = str_logical_to_physical_bit(offset.long_value, lsb_first); + mask = (unsigned char)(1u << (bit_index % CHAR_BIT)); + location.byte_index = bit_index / CHAR_BIT; + } + else { + location = str_bit_location_from_offset(offset.value, lsb_first); + mask = (unsigned char)(1u << location.bit_offset); + } + + switch (mutation) { + case STR_BIT_SET: + ptr[location.byte_index] |= mask; + break; + case STR_BIT_CLEAR: + ptr[location.byte_index] &= (unsigned char)~mask; + break; + case STR_BIT_FLIP: + ptr[location.byte_index] ^= mask; + break; + } + + return str; +} + +/* + * call-seq: + * bit_set(offset, lsb_first: true) -> self + * + * :include: doc/string/bit_set.rdoc + * + */ +static VALUE +rb_str_bit_set(int argc, VALUE *argv, VALUE str) +{ + return str_mutate_bit(argc, argv, str, STR_BIT_SET); +} + +/* + * call-seq: + * bit_clear(offset, lsb_first: true) -> self + * + * :include: doc/string/bit_clear.rdoc + * + */ +static VALUE +rb_str_bit_clear(int argc, VALUE *argv, VALUE str) +{ + return str_mutate_bit(argc, argv, str, STR_BIT_CLEAR); +} + +/* + * call-seq: + * bit_flip(offset, lsb_first: true) -> self + * + * :include: doc/string/bit_flip.rdoc + * + */ +static VALUE +rb_str_bit_flip(int argc, VALUE *argv, VALUE str) +{ + return str_mutate_bit(argc, argv, str, STR_BIT_FLIP); +} + +static uint64_t +str_count_bits(const unsigned char *ptr, long len) +{ + uint64_t count = 0; + long off = 0; + long unrolled_end = len & ~31L; + long aligned_end = len & ~7L; + + // 32 bytes (256 bits) at a time + for (; off < unrolled_end; off += 32) { + uint64_t w0, w1, w2, w3; + memcpy(&w0, ptr + off, 8); + memcpy(&w1, ptr + off + 8, 8); + memcpy(&w2, ptr + off + 16, 8); + memcpy(&w3, ptr + off + 24, 8); + count += rb_popcount64(w0); + count += rb_popcount64(w1); + count += rb_popcount64(w2); + count += rb_popcount64(w3); + } + + // 8 bytes (64 bits) at a time + for (; off < aligned_end; off += 8) { + uint64_t word; + memcpy(&word, ptr + off, 8); + count += rb_popcount64(word); + } + + // remaining bytes + if (off < len) { + uint64_t word = 0; + int shift = 0; + for (; off < len; off++, shift += CHAR_BIT) { + word |= (uint64_t)ptr[off] << shift; + } + count += rb_popcount64(word); + } + + return count; +} + +/* + * call-seq: + * bit_count -> integer + * + * :include: doc/string/bit_count.rdoc + * + */ +static VALUE +rb_str_bit_count(VALUE str) +{ + return ULL2NUM(str_count_bits((const unsigned char *)RSTRING_PTR(str), RSTRING_LEN(str))); +} + +static void +str_check_bitwise_length(VALUE str, VALUE other) +{ + if (RSTRING_LEN(str) != RSTRING_LEN(other)) { + rb_raise(rb_eArgError, "operands must have the same length (%ld vs %ld)", + RSTRING_LEN(str), RSTRING_LEN(other)); + } +} + +static VALUE +str_bitwise_result(VALUE str) +{ + long len = RSTRING_LEN(str); + VALUE result = rb_str_buf_new(len); + rb_str_resize(result, len); + rb_enc_associate(result, rb_ascii8bit_encoding()); + ENC_CODERANGE_CLEAR(result); + return result; +} + +#define STR_DEFINE_UNARY_BITWISE_KERNEL(name, expr_word, expr_byte) \ + static void \ + name(unsigned char *dst, const unsigned char *src, long len) \ + { \ + long off = 0; \ + long unrolled_end = len & ~31L; \ + long aligned_end = len & ~7L; \ + for (; off < unrolled_end; off += 32) { \ + uint64_t s0, s1, s2, s3; \ + memcpy(&s0, src + off, 8); \ + memcpy(&s1, src + off + 8, 8); \ + memcpy(&s2, src + off + 16, 8); \ + memcpy(&s3, src + off + 24, 8); \ + s0 = (expr_word(s0)); \ + s1 = (expr_word(s1)); \ + s2 = (expr_word(s2)); \ + s3 = (expr_word(s3)); \ + memcpy(dst + off, &s0, 8); \ + memcpy(dst + off + 8, &s1, 8); \ + memcpy(dst + off + 16, &s2, 8); \ + memcpy(dst + off + 24, &s3, 8); \ + } \ + for (; off < aligned_end; off += 8) { \ + uint64_t word; \ + memcpy(&word, src + off, 8); \ + word = (expr_word(word)); \ + memcpy(dst + off, &word, 8); \ + } \ + for (; off < len; off++) dst[off] = (expr_byte(src[off])); \ + } + +#define STR_DEFINE_BINARY_BITWISE_KERNEL(name, expr_word, expr_byte) \ + static void \ + name(unsigned char *dst, const unsigned char *lhs, \ + const unsigned char *rhs, long len) \ + { \ + long off = 0; \ + long unrolled_end = len & ~31L; \ + long aligned_end = len & ~7L; \ + for (; off < unrolled_end; off += 32) { \ + uint64_t l0, l1, l2, l3, r0, r1, r2, r3; \ + memcpy(&l0, lhs + off, 8); memcpy(&r0, rhs + off, 8); \ + memcpy(&l1, lhs + off + 8, 8); memcpy(&r1, rhs + off + 8, 8); \ + memcpy(&l2, lhs + off + 16, 8); memcpy(&r2, rhs + off + 16, 8); \ + memcpy(&l3, lhs + off + 24, 8); memcpy(&r3, rhs + off + 24, 8); \ + l0 = expr_word(l0, r0); \ + l1 = expr_word(l1, r1); \ + l2 = expr_word(l2, r2); \ + l3 = expr_word(l3, r3); \ + memcpy(dst + off, &l0, 8); \ + memcpy(dst + off + 8, &l1, 8); \ + memcpy(dst + off + 16, &l2, 8); \ + memcpy(dst + off + 24, &l3, 8); \ + } \ + for (; off < aligned_end; off += 8) { \ + uint64_t lhs_word, rhs_word; \ + memcpy(&lhs_word, lhs + off, 8); \ + memcpy(&rhs_word, rhs + off, 8); \ + lhs_word = expr_word(lhs_word, rhs_word); \ + memcpy(dst + off, &lhs_word, 8); \ + } \ + for (; off < len; off++) dst[off] = expr_byte(lhs[off], rhs[off]); \ + } + +#define STR_BITWISE_NOT_WORD(x) (~(x)) +#define STR_BITWISE_NOT_BYTE(x) ((unsigned char)~(x)) +#define STR_BITWISE_AND_WORD(x, y) ((x) & (y)) +#define STR_BITWISE_AND_BYTE(x, y) ((unsigned char)((x) & (y))) +#define STR_BITWISE_OR_WORD(x, y) ((x) | (y)) +#define STR_BITWISE_OR_BYTE(x, y) ((unsigned char)((x) | (y))) +#define STR_BITWISE_XOR_WORD(x, y) ((x) ^ (y)) +#define STR_BITWISE_XOR_BYTE(x, y) ((unsigned char)((x) ^ (y))) + +STR_DEFINE_UNARY_BITWISE_KERNEL(str_bitwise_not, STR_BITWISE_NOT_WORD, STR_BITWISE_NOT_BYTE) +STR_DEFINE_BINARY_BITWISE_KERNEL(str_bitwise_and, STR_BITWISE_AND_WORD, STR_BITWISE_AND_BYTE) +STR_DEFINE_BINARY_BITWISE_KERNEL(str_bitwise_or, STR_BITWISE_OR_WORD, STR_BITWISE_OR_BYTE) +STR_DEFINE_BINARY_BITWISE_KERNEL(str_bitwise_xor, STR_BITWISE_XOR_WORD, STR_BITWISE_XOR_BYTE) + +/* + * call-seq: + * bitwise_not -> string + * + * :include: doc/string/bitwise_not.rdoc + * + */ +static VALUE +rb_str_bitwise_not(VALUE str) +{ + long len = RSTRING_LEN(str); + VALUE result = str_bitwise_result(str); + str_bitwise_not((unsigned char *)RSTRING_PTR(result), + (const unsigned char *)RSTRING_PTR(str), len); + return result; +} + +/* + * call-seq: + * bitwise_not! -> self + * + * :include: doc/string/bitwise_not_bang.rdoc + * + */ +static VALUE +rb_str_bitwise_not_bang(VALUE str) +{ + long len; + unsigned char *ptr; + + rb_str_modify(str); + len = RSTRING_LEN(str); + ptr = (unsigned char *)RSTRING_PTR(str); + str_bitwise_not(ptr, ptr, len); + return str; +} + +#define STR_DEFINE_BINARY_BITWISE_METHOD(name) \ + static VALUE \ + rb_str_bitwise_##name(VALUE str, VALUE other) \ + { \ + long len; \ + VALUE result; \ + StringValue(other); \ + str_check_bitwise_length(str, other); \ + len = RSTRING_LEN(str); \ + result = str_bitwise_result(str); \ + str_bitwise_##name((unsigned char *)RSTRING_PTR(result), \ + (const unsigned char *)RSTRING_PTR(str), \ + (const unsigned char *)RSTRING_PTR(other), len); \ + return result; \ + } \ + static VALUE \ + rb_str_bitwise_##name##_bang(VALUE str, VALUE other) \ + { \ + long len; \ + unsigned char *ptr; \ + StringValue(other); \ + str_check_bitwise_length(str, other); \ + rb_str_modify(str); \ + len = RSTRING_LEN(str); \ + ptr = (unsigned char *)RSTRING_PTR(str); \ + str_bitwise_##name(ptr, ptr, \ + (const unsigned char *)RSTRING_PTR(other), len); \ + return str; \ + } + +STR_DEFINE_BINARY_BITWISE_METHOD(and) +STR_DEFINE_BINARY_BITWISE_METHOD(or) +STR_DEFINE_BINARY_BITWISE_METHOD(xor) + static VALUE str_byte_substr(VALUE str, long beg, long len, int empty) { @@ -12932,6 +13416,20 @@ Init_String(void) rb_define_method(rb_cString, "chr", rb_str_chr, 0); rb_define_method(rb_cString, "getbyte", rb_str_getbyte, 1); rb_define_method(rb_cString, "setbyte", rb_str_setbyte, 2); + rb_define_method(rb_cString, "bit_get", rb_str_bit_get, -1); + rb_define_method(rb_cString, "bit_set?", rb_str_bit_set_p, -1); + rb_define_method(rb_cString, "bit_set", rb_str_bit_set, -1); + rb_define_method(rb_cString, "bit_clear", rb_str_bit_clear, -1); + rb_define_method(rb_cString, "bit_flip", rb_str_bit_flip, -1); + rb_define_method(rb_cString, "bit_count", rb_str_bit_count, 0); + rb_define_method(rb_cString, "bitwise_not", rb_str_bitwise_not, 0); + rb_define_method(rb_cString, "bitwise_not!", rb_str_bitwise_not_bang, 0); + rb_define_method(rb_cString, "bitwise_and", rb_str_bitwise_and, 1); + rb_define_method(rb_cString, "bitwise_and!", rb_str_bitwise_and_bang, 1); + rb_define_method(rb_cString, "bitwise_or", rb_str_bitwise_or, 1); + rb_define_method(rb_cString, "bitwise_or!", rb_str_bitwise_or_bang, 1); + rb_define_method(rb_cString, "bitwise_xor", rb_str_bitwise_xor, 1); + rb_define_method(rb_cString, "bitwise_xor!", rb_str_bitwise_xor_bang, 1); rb_define_method(rb_cString, "byteslice", rb_str_byteslice, -1); rb_define_method(rb_cString, "bytesplice", rb_str_bytesplice, -1); rb_define_method(rb_cString, "scrub", str_scrub, -1); diff --git a/test/ruby/test_string.rb b/test/ruby/test_string.rb index a7affd46cdf27b..d16ffcba2e19ee 100644 --- a/test/ruby/test_string.rb +++ b/test/ruby/test_string.rb @@ -1026,6 +1026,119 @@ def test_setbyte assert_raise(FrozenError) { S('foo').freeze.setbyte(0, 0x61) } end + def test_bit_get + s = S("\xAA\x80") + assert_equal(0, s.bit_get(0)) + assert_equal(1, s.bit_get(1)) + assert_equal(1, s.bit_get(7)) + assert_equal(1, s.bit_get(0, lsb_first: false)) + assert_equal(0, s.bit_get(1, lsb_first: false)) + assert_equal(1, s.bit_get(8, lsb_first: false)) + assert_nil(s.bit_get(16)) + assert_raise(IndexError) { s.bit_get(-1) } + assert_raise(ArgumentError) { s.bit_get(2**100) } + assert_raise(ArgumentError) { s.bit_get(0, lsb_first: nil) } + end + + def test_bit_set_p + s = S("\xAA\x80") + assert_equal(false, s.bit_set?(0)) + assert_equal(true, s.bit_set?(1)) + assert_equal(true, s.bit_set?(7)) + assert_equal(true, s.bit_set?(0, lsb_first: false)) + assert_equal(false, s.bit_set?(1, lsb_first: false)) + assert_equal(true, s.bit_set?(8, lsb_first: false)) + assert_nil(s.bit_set?(16)) + assert_raise(IndexError) { s.bit_set?(-1) } + assert_raise(ArgumentError) { s.bit_set?(2**100) } + assert_raise(ArgumentError) { s.bit_set?(0, lsb_first: nil) } + end + + def test_bit_set_clear_flip + s = S("\x00") + assert_same(s, s.bit_set(1)) + assert_equal(S("\x02"), s) + assert_same(s, s.bit_clear(1)) + assert_equal(S("\x00"), s) + assert_same(s, s.bit_flip(1)) + assert_equal(S("\x02"), s) + assert_same(s, s.bit_flip(1)) + assert_equal(S("\x00"), s) + + s.bit_set(1, lsb_first: false) + assert_equal(S("\x40"), s) + s.bit_clear(1, lsb_first: false) + assert_equal(S("\x00"), s) + + s = S("\x00\x00") + s.bit_set(8, lsb_first: false) + assert_equal(S("\x00\x80"), s) + s.bit_clear(8, lsb_first: false) + assert_equal(S("\x00\x00"), s) + s.bit_flip(8, lsb_first: false) + assert_equal(S("\x00\x80"), s) + + assert_raise(IndexError) { S("\x00").bit_set(8) } + assert_raise(IndexError) { S("\x00").bit_set(-1) } + assert_raise(IndexError) { S("\x00").bit_clear(8) } + assert_raise(IndexError) { S("\x00").bit_clear(-1) } + assert_raise(IndexError) { S("\x00").bit_flip(8) } + assert_raise(IndexError) { S("\x00").bit_flip(-1) } + assert_raise(ArgumentError) { S("\x00").bit_set(0, lsb_first: nil) } + assert_raise(FrozenError) { S("\x00").freeze.bit_set(0) } + + shared = S("fooXbar").split(S("X")).last + shared.bit_set(0) + assert_equal(S("car"), shared) + end + + def test_bit_count + assert_equal(0, S("").bit_count) + assert_equal(0, S("\x00").bit_count) + assert_equal(8, S("\xFF").bit_count) + assert_equal(8, S("\xAA\xF0").bit_count) + assert_raise(ArgumentError) { S("\x00").bit_count(0) } + assert_raise(ArgumentError) { S("\x00").bit_count(lsb_first: false) } + end + + def test_bitwise + s = S("\x00\xAA") + result = s.bitwise_not + assert_equal(S("\xFF\x55").b, result) + assert_not_same(s, result) + assert_equal(S("\x00\xAA"), s) + assert_equal(Encoding::BINARY, result.encoding) + + assert_same(s, s.bitwise_not!) + assert_equal(S("\xFF\x55"), s) + + assert_equal(S("\xC0").b, S("\xF0").bitwise_and(S("\xCC"))) + assert_equal(S("\xFC").b, S("\xF0").bitwise_or(S("\x0C"))) + assert_equal(S("\x3C").b, S("\xF0").bitwise_xor(S("\xCC"))) + assert_equal(Encoding::BINARY, S("\xF0").force_encoding("UTF-8").bitwise_and(S("\xCC")).encoding) + assert_equal(Encoding::BINARY, S("\xF0").force_encoding("UTF-8").bitwise_or(S("\x0C")).encoding) + assert_equal(Encoding::BINARY, S("\xF0").force_encoding("UTF-8").bitwise_xor(S("\xCC")).encoding) + + s = S("\xF0") + assert_same(s, s.bitwise_and!(S("\xCC"))) + assert_equal(S("\xC0"), s) + assert_same(s, s.bitwise_or!(S("\x0C"))) + assert_equal(S("\xCC"), s) + assert_same(s, s.bitwise_xor!(S("\xFF"))) + assert_equal(S("\x33"), s) + + other = Object.new + def other.to_str + "\xCC" + end + assert_equal(S("\xC0").b, S("\xF0").bitwise_and(other)) + + assert_raise(ArgumentError) { S("\x00").bitwise_and(S("\x00\x00")) } + assert_raise(TypeError) { S("\x00").bitwise_or(Object.new) } + assert_raise(FrozenError) { S("\x00").freeze.bitwise_not! } + assert_raise(FrozenError) { S("\x00").freeze.bitwise_xor!(S("\x00")) } + end + def test_each_codepoint # Single byte optimization assert_equal 65, S("ABC").each_codepoint.next