diff --git a/cont.c b/cont.c index 252e2ec199036d..1145a9f64a0620 100644 --- a/cont.c +++ b/cont.c @@ -2869,6 +2869,13 @@ fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int kw_splat, rb_fi VM_ASSERT(FIBER_RUNNABLE_P(fiber)); + /* + * Keep the target fiber object alive across fiber_store. The raw + * rb_fiber_t pointer is used after the coroutine switch, and GC may run + * while this C frame is suspended. + */ + VALUE fiber_value = fiber->cont.self; + rb_fiber_t *current_fiber = fiber_current(); VM_ASSERT(!current_fiber->resuming_fiber); @@ -2902,6 +2909,7 @@ fiber_switch(rb_fiber_t *fiber, int argc, const VALUE *argv, int kw_splat, rb_fi } } #endif + RB_GC_GUARD(fiber_value); if (fiber_current()->blocking) { th->blocking += 1; diff --git a/lib/prism.rb b/lib/prism.rb index 8f0342724a30d3..9189c449d7bd05 100644 --- a/lib/prism.rb +++ b/lib/prism.rb @@ -136,7 +136,30 @@ def self.find(callable, rubyvm: !!defined?(RubyVM)) # The C extension is the default backend on CRuby. Prism::BACKEND = :CEXT - require "prism/prism" + begin + # The precompiled native libraries are in /lib/prism/ + require_relative "prism/#{RUBY_VERSION[/\d+\.\d+/]}/prism" + rescue LoadError => e + if e.message.include?("GLIBC") + warn(<<~EOM) + + ERROR: It looks like you're trying to use Prism as a precompiled native gem on a system + with an unsupported version of glibc. + + #{e.message} + + If that's the case, then please install Prism via the `ruby` platform gem: + gem install prism --platform=ruby + or, in your Gemfile: + gem "prism", force_ruby_platform: true + + EOM + raise e + end + + # Precompiled library isn't available, fall back to the library compiled at installation time. + require "prism/prism" + end else # The FFI backend is used on other Ruby implementations. Prism::BACKEND = :FFI diff --git a/prism/prism.c b/prism/prism.c index 19af4f4a259805..311291aff5b43a 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -12710,14 +12710,6 @@ match6(const pm_parser_t *parser, pm_token_type_t type1, pm_token_type_t type2, return match1(parser, type1) || match1(parser, type2) || match1(parser, type3) || match1(parser, type4) || match1(parser, type5) || match1(parser, type6); } -/** - * Returns true if the current token is any of the seven given types. - */ -static PRISM_INLINE bool -match7(const pm_parser_t *parser, pm_token_type_t type1, pm_token_type_t type2, pm_token_type_t type3, pm_token_type_t type4, pm_token_type_t type5, pm_token_type_t type6, pm_token_type_t type7) { - return match1(parser, type1) || match1(parser, type2) || match1(parser, type3) || match1(parser, type4) || match1(parser, type5) || match1(parser, type6) || match1(parser, type7); -} - /** * Returns true if the current token is any of the eight given types. */ @@ -12930,6 +12922,31 @@ token_begins_expression_p(pm_token_type_t type) { } } +/** + * Returns true if the given token can begin a pattern element. This is the + * set of tokens that can begin an expression plus the tokens that begin + * pattern-only constructs — `*` (rest patterns), `**` (keyword rest + * patterns), and `^` (pin patterns) — which are binary operator tokens in + * expression contexts and therefore excluded from token_begins_expression_p. + * + * When a token fails this predicate at a decision point, the pattern ends + * there and the token is left for the enclosing context to accept or reject. + * This mirrors the grammar, whose pattern reductions (`p_top_expr_body: + * p_expr ','`, `p_kw: p_kw_label`, `p_kwargs: p_kwarg ','`) fire by default + * on any token that cannot start a pattern. Tokens that pass this predicate + * but are invalid in the specific context (e.g. `**` in an array pattern) + * are rejected by the pattern parser itself with a more targeted diagnostic. + */ +static PRISM_INLINE bool +token_begins_pattern_p(pm_token_type_t type) { + return ( + token_begins_expression_p(type) || + type == PM_TOKEN_USTAR || + type == PM_TOKEN_USTAR_STAR || + type == PM_TOKEN_CARET + ); +} + /** * Parse an expression with the given binding power that may be optionally * prefixed by the * operator. @@ -17097,7 +17114,12 @@ parse_pattern_hash(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_node pm_node_t *value; - if (match8(parser, PM_TOKEN_COMMA, PM_TOKEN_KEYWORD_THEN, PM_TOKEN_BRACE_RIGHT, PM_TOKEN_BRACKET_RIGHT, PM_TOKEN_PARENTHESIS_RIGHT, PM_TOKEN_NEWLINE, PM_TOKEN_SEMICOLON, PM_TOKEN_EOF)) { + /* + * The label has an implicit value when the next token cannot + * begin a pattern, mirroring the grammar's `p_kw: p_kw_label` + * reduction. + */ + if (!token_begins_pattern_p(parser->current.type)) { if (PM_NODE_TYPE_P(first_node, PM_SYMBOL_NODE)) { value = parse_pattern_hash_implicit_value(parser, captures, (pm_symbol_node_t *) first_node); } else { @@ -17132,8 +17154,12 @@ parse_pattern_hash(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_node // If there are any other assocs, then we'll parse them now. while (accept1(parser, PM_TOKEN_COMMA)) { - // Here we need to break to support trailing commas. - if (match7(parser, PM_TOKEN_KEYWORD_THEN, PM_TOKEN_BRACE_RIGHT, PM_TOKEN_BRACKET_RIGHT, PM_TOKEN_PARENTHESIS_RIGHT, PM_TOKEN_NEWLINE, PM_TOKEN_SEMICOLON, PM_TOKEN_EOF)) { + /* + * A trailing comma ends the pattern when the next token cannot begin + * another element, mirroring the grammar's `p_kwargs: p_kwarg ','` + * reduction. + */ + if (!token_begins_pattern_p(parser->current.type)) { // Trailing commas are not allowed to follow a rest pattern. if (rest != NULL) { pm_parser_err_token(parser, &parser->current, PM_ERR_PATTERN_EXPRESSION_AFTER_REST); @@ -17174,7 +17200,12 @@ parse_pattern_hash(pm_parser_t *parser, pm_constant_id_list_t *captures, pm_node parse_pattern_hash_key(parser, &keys, key); pm_node_t *value = NULL; - if (match8(parser, PM_TOKEN_COMMA, PM_TOKEN_KEYWORD_THEN, PM_TOKEN_BRACE_RIGHT, PM_TOKEN_BRACKET_RIGHT, PM_TOKEN_PARENTHESIS_RIGHT, PM_TOKEN_NEWLINE, PM_TOKEN_SEMICOLON, PM_TOKEN_EOF)) { + /* + * The label has an implicit value when the next token cannot + * begin a pattern, mirroring the grammar's `p_kw: p_kw_label` + * reduction. + */ + if (!token_begins_pattern_p(parser->current.type)) { if (PM_NODE_TYPE_P(key, PM_SYMBOL_NODE)) { value = parse_pattern_hash_implicit_value(parser, captures, (pm_symbol_node_t *) key); } else { @@ -17674,15 +17705,12 @@ parse_pattern(pm_parser_t *parser, pm_constant_id_list_t *captures, uint8_t flag // Gather up all of the patterns into the list. while (accept1(parser, PM_TOKEN_COMMA)) { - // Break early here in case we have a trailing comma. The newline and - // EOF terminators cover a one-line match (`x => a,`) or a `case`/`in` - // clause (`in a,\n ...`); a newline is only lexed as a token here - // when `pattern_matching_newlines` is set, so this does not affect - // patterns nested in brackets or parentheses. - if ( - match7(parser, PM_TOKEN_KEYWORD_THEN, PM_TOKEN_BRACE_RIGHT, PM_TOKEN_BRACKET_RIGHT, PM_TOKEN_PARENTHESIS_RIGHT, PM_TOKEN_SEMICOLON, PM_TOKEN_KEYWORD_AND, PM_TOKEN_KEYWORD_OR) || - match2(parser, PM_TOKEN_NEWLINE, PM_TOKEN_EOF) - ) { + /* + * A trailing comma ends the pattern when the next token cannot + * begin another pattern element, leaving the token for the + * enclosing context to accept or reject. + */ + if (!token_begins_pattern_p(parser->current.type)) { // A trailing comma forms an implicit rest pattern (`[a,]` is // `[a, *]`). If a rest pattern has already been parsed, then // this is a second rest, which is not allowed (e.g. `[a, *b,]` diff --git a/test/prism/errors/dynamic_label_pattern.txt b/test/prism/errors/dynamic_label_pattern.txt index b8d1012e45bc55..2e0fb058793f77 100644 --- a/test/prism/errors/dynamic_label_pattern.txt +++ b/test/prism/errors/dynamic_label_pattern.txt @@ -1,3 +1,4 @@ :a => 'a': | 1 - ^ expected a pattern expression after the key + ^ unexpected '|', expecting end-of-input + ^ unexpected '|', ignoring it diff --git a/test/prism/fixtures/pattern_terminators.txt b/test/prism/fixtures/pattern_terminators.txt new file mode 100644 index 00000000000000..6224622208fbb8 --- /dev/null +++ b/test/prism/fixtures/pattern_terminators.txt @@ -0,0 +1,21 @@ +while a in b, do 1 end + +until a in b, do 1 end + +for x in a in b, do 1 end + +loop do a in b, end + +"#{a in b,}" + +while a in x: do 1 end + +while a in x: 1, do 1 end + +while a in x:, y: do 1 end + +loop do a in x: end + +"#{a in x:}" + +a in x: and 1 diff --git a/test/prism/packaging/test-gem-file-contents b/test/prism/packaging/test-gem-file-contents new file mode 100755 index 00000000000000..5ec89d6fcc0e9d --- /dev/null +++ b/test/prism/packaging/test-gem-file-contents @@ -0,0 +1,215 @@ +#! /usr/bin/env ruby +# +# This is a standalone script intended to run as part of the CI test suite. +# +# It inspects the contents of a gem file -- both the files and the gemspec -- to ensure we're +# packaging what we expect, and that we're not packaging anything we don't expect. +# +require "bundler/inline" + +gemfile do + source "https://rubygems.org" + gem "minitest" + gem "minitest-reporters" +end + +require "yaml" + +def usage_and_exit(message = nil) + puts "ERROR: #{message}" if message + puts "USAGE: #{File.basename(__FILE__)} [options]" + exit(1) +end + +usage_and_exit if ARGV.include?("-h") +usage_and_exit unless (gemfile = ARGV[0]) +usage_and_exit("#{gemfile} does not exist") unless File.file?(gemfile) +usage_and_exit("#{gemfile} is not a gem") unless /\.gem$/.match?(gemfile) +gemfile = File.expand_path(gemfile) + +gemfile_contents = Dir.mktmpdir do |dir| + Dir.chdir(dir) do + unless system("tar", "-xf", gemfile, "data.tar.gz") + raise "could not unpack gem #{gemfile}" + end + + `tar -ztf data.tar.gz`.split("\n") + end +end + +gemspec = Dir.mktmpdir do |dir| + Dir.chdir(dir) do + unless system("tar", "-xf", gemfile, "metadata.gz") + raise "could not unpack gem #{gemfile}" + end + + YAML.unsafe_load(`gunzip -c metadata.gz`) + end +end + +if ARGV.include?("-v") + puts "---------- gemfile contents ----------" + puts gemfile_contents + puts + puts "---------- gemspec ----------" + puts gemspec.to_ruby + puts +end + +require "minitest/autorun" +require "minitest/reporters" +Minitest::Reporters.use!([Minitest::Reporters::SpecReporter.new]) + +puts "Testing '#{gemfile}' (#{gemspec.platform})" +describe File.basename(gemfile) do + let(:native_ruby_versions) { ["3.3", "3.4", "4.0"] } + + describe "setup" do + it "gemfile contains some files" do + actual = gemfile_contents.length + assert_operator(actual, :>, 10, "expected gemfile to contain more than #{actual} files") + end + + it "gemspec is a Gem::Specification" do + assert_equal(Gem::Specification, gemspec.class) + end + end + + describe "all platforms" do + ["lib"].each do |dir| + it "contains every ruby file in #{dir}/" do + committed_files = `git ls-files #{dir}`.split("\n").grep(/\.rb$/) + generated_files = `git ls-files templates/lib/prism/*.rb.erb`.split("\n").map { |f| f.delete_prefix("templates/").delete_suffix(".erb") } + expected_files = Set.new(committed_files + generated_files) + + skip "looks like this isn't a git repository" if expected_files.empty? + + actual_files = Set.new(gemfile_contents.select { |f| f.start_with?("#{dir}/") }.grep(/\.rb$/)) + assert_equal(expected_files, actual_files) + end + end + + ["test"].each do |dir| + it "does not contain files from #{dir}/" do + actual = gemfile_contents.select { |f| f.start_with?("#{dir}/") }.grep(/\.rb$/) + assert_empty(actual) + end + end + + it "does not contain the Gemfile" do + refute_includes(gemfile_contents, "Gemfile") + end + end + + describe "ruby platform" do + before { skip unless gemspec.platform == Gem::Platform::RUBY } + + it "contains C source files in ext/" do + assert_operator(gemfile_contents.count { |f| File.fnmatch?("ext/**/*.c", f) }, :>=, 2) + end + + it "contains header files" do + assert_operator(gemfile_contents.count { |f| File.fnmatch?("ext/**/*.h", f) }, :>=, 1) + assert_operator(gemfile_contents.count { |f| File.fnmatch?("include/**/*.h", f) }, :>=, 20) + end + + it "contains C source files in src/" do + assert_operator(gemfile_contents.count { |f| File.fnmatch?("src/*.c", f) }, :>=, 10) + end + + it "contains extconf.rb" do + assert_includes(gemfile_contents, "ext/prism/extconf.rb") + end + + it "has extensions set" do + assert_includes(gemspec.extensions, "ext/prism/extconf.rb") + end + + it "sets required_ruby_version appropriately" do + assert( + gemspec.required_ruby_version.satisfied_by?(Gem::Version.new("2.7")), + "required_ruby_version='#{gemspec.required_ruby_version}' should support ruby 2.7" + ) + native_ruby_versions.each do |v| + assert( + gemspec.required_ruby_version.satisfied_by?(Gem::Version.new(v)), + "required_ruby_version='#{gemspec.required_ruby_version}' should support ruby #{v}" + ) + end + end + end + + describe "native platform" do + before { skip unless gemspec.platform.is_a?(Gem::Platform) && gemspec.platform.cpu } + + # aarch64-mingw-ucrt only has cross-rubies for 3.4+, all other platforms have all versions + let(:expected_ruby_versions) do + if gemspec.platform.to_s == "aarch64-mingw-ucrt" + native_ruby_versions.reject { |v| v == "3.3" } + else + native_ruby_versions + end + end + + it "contains expected shared library files" do + expected_ruby_versions.each do |version| + actual = gemfile_contents.find do |p| + File.fnmatch?("lib/prism/#{version}/prism.{so,bundle}", p, File::FNM_EXTGLOB) + end + assert(actual, "expected to find shared library file for ruby #{version}") + end + + actual = gemfile_contents.find do |p| + File.fnmatch?("lib/prism/prism.{so,bundle}", p, File::FNM_EXTGLOB) + end + refute(actual, "did not expect to find shared library file in lib/prism") + + actual = gemfile_contents.find_all do |p| + File.fnmatch?("lib/prism/**/*.{so,bundle}", p, File::FNM_EXTGLOB) + end + assert_equal( + expected_ruby_versions.length, + actual.length, + "expected exactly #{expected_ruby_versions.length} shared library files, found: #{actual.inspect}" + ) + end + + it "has extensions cleared" do + assert_empty(gemspec.extensions) + end + + it "sets required_ruby_version appropriately" do + expected_ruby_versions.each do |v| + assert( + gemspec.required_ruby_version.satisfied_by?(Gem::Version.new(v)), + "required_ruby_version='#{gemspec.required_ruby_version}' should support ruby #{v}" + ) + end + + # verify unsupported versions are excluded + unsupported_ruby_versions = native_ruby_versions - expected_ruby_versions + unsupported_ruby_versions.each do |v| + refute( + gemspec.required_ruby_version.satisfied_by?(Gem::Version.new(v)), + "required_ruby_version='#{gemspec.required_ruby_version}' should NOT support ruby #{v}" + ) + end + + refute( + gemspec.required_ruby_version.satisfied_by?(Gem::Version.new("2.7")), + "required_ruby_version='#{gemspec.required_ruby_version}' should NOT support ruby 2.7" + ) + + refute( + gemspec.required_ruby_version.satisfied_by?(Gem::Version.new("3.2")), + "required_ruby_version='#{gemspec.required_ruby_version}' should NOT support ruby 3.2" + ) + + # verify the upper bound is set (required_ruby_version should not be open-ended) + refute( + gemspec.required_ruby_version.satisfied_by?(Gem::Version.new("9.9")), + "required_ruby_version='#{gemspec.required_ruby_version}' should have an upper bound" + ) + end + end +end diff --git a/test/ruby/test_fiber.rb b/test/ruby/test_fiber.rb index 6976bd97423bd9..ec82a3a3473efc 100644 --- a/test/ruby/test_fiber.rb +++ b/test/ruby/test_fiber.rb @@ -497,6 +497,40 @@ def test_create_fiber_in_new_thread assert_equal :ok, ret, '[Bug #14642]' end + def test_gc_during_nested_resume_yield + assert_normal_exit <<-'RUBY', '[Bug #22196]', timeout: 10 + parent = child1 = child2 = nil + + parent = Fiber.new do + child1 = Fiber.new do + parent.resume + end + + child2 = Fiber.new do + GC.start + parent.resume + end + + Fiber.yield + + child1 = nil + + Fiber.yield + end + + parent.resume + + child = child1 + child1 = nil + child.resume + child = nil + + child = child2 + child2 = nil + child.resume + RUBY + end + def test_machine_stack_gc assert_normal_exit <<-RUBY, '[Bug #14561]', timeout: 60 enum = Enumerator.new { |y| y << 1 }