Skip to content

Fix usage of rb_struct_initialize() to pass an Array of members values and not a Hash and add TruffleRuby in CI #749

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Fix usage of rb_struct_initialize() to pass an Array of members values and not a Hash and add TruffleRuby in CI #749

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3550148
Fix usage of rb_struct_initialize() to pass an Array of members value…
eregon Nov 5, 2025
d7110e3
Pin power_assert to v2 for Ruby 2.7 (v3 requires 3.1+)
eregon Nov 5, 2025
dbabe7a
Properly set the message of Exceptions on TruffleRuby
eregon Nov 5, 2025
c16b4e6
Add TruffleRuby in CI
eregon Nov 5, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,14 @@ jobs:
ruby: ${{ fromJson(needs.ruby-versions.outputs.versions) }}
os: [ ubuntu-latest, macos-latest, windows-latest ]
include:
# jruby is broken with "undefined method 'init_struct'"
# jruby is broken with "undefined method 'init_data'"
# https://github.com/ruby/psych/actions/runs/15434465445/job/43438083198?pr=734
- { os: windows-latest, ruby: jruby-head }
- { os: macos-latest, ruby: jruby-head }
- { os: ubuntu-latest, ruby: jruby-head }
# Needs truffleruby-head for rb_struct_initialize() (which truffleruby 25.0 does not have)
- { os: ubuntu-latest, ruby: truffleruby-head }
- { os: macos-latest, ruby: truffleruby-head }
- { os: windows-latest, ruby: ucrt }
- { os: windows-latest, ruby: mingw }
- { os: windows-latest, ruby: mswin }
Expand Down
1 change: 1 addition & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@ group :development do
gem 'ruby-maven', :platforms => :jruby
gem 'test-unit'
gem 'test-unit-ruby-core', ">= 1.0.7"
gem 'power_assert', '~> 2.0' if RUBY_VERSION < '3.0' # https://github.com/ruby/power_assert/pull/61
end
13 changes: 7 additions & 6 deletions ext/psych/psych_to_ruby.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,11 @@ static VALUE build_exception(VALUE self, VALUE klass, VALUE mesg)
{
VALUE e = rb_obj_alloc(klass);

#ifdef TRUFFLERUBY
rb_exc_set_message(e, mesg);
#else
rb_iv_set(e, "mesg", mesg);
#endif

return e;
}
Expand All @@ -24,12 +28,9 @@ static VALUE path2class(VALUE self, VALUE path)
return rb_path_to_class(path);
}

static VALUE init_struct(VALUE self, VALUE data, VALUE attrs)
static VALUE init_data(VALUE self, VALUE data, VALUE values)
{
VALUE args = rb_ary_new2(1);
rb_ary_push(args, attrs);
rb_struct_initialize(data, args);

rb_struct_initialize(data, values);
return data;
}

Expand All @@ -42,7 +43,7 @@ void Init_psych_to_ruby(void)
VALUE visitor = rb_define_class_under(visitors, "Visitor", rb_cObject);
cPsychVisitorsToRuby = rb_define_class_under(visitors, "ToRuby", visitor);

rb_define_private_method(cPsychVisitorsToRuby, "init_struct", init_struct, 2);
rb_define_private_method(cPsychVisitorsToRuby, "init_data", init_data, 2);
rb_define_private_method(cPsychVisitorsToRuby, "build_exception", build_exception, 2);
rb_define_private_method(class_loader, "path2class", path2class, 1);
}
3 changes: 2 additions & 1 deletion lib/psych/visitors/to_ruby.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,8 @@ def visit_Psych_Nodes_Mapping o
revive_data_members(members, o)
end
data ||= allocate_anon_data(o, members)
init_struct(data, **members)
values = data.members.map { |m| members[m] }
Copy link
Member Author

@eregon eregon Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also use members.values but then we should check if the data_class.members == members.keys, and raise an exception if not. If so, which exception class should I use then, do we have one for "incompatible change in loaded classes"?
Otherwise if e.g. one changes the order of fields from Foo = Data.define(:a, :b) to Data.define(:b, :a) then it would load the instance incorrectly in a rather surprising manner.

If there are extra members in Data.define, the current approach sets them to nil, like before:

irb(main):002> Psych::VERSION
=> "5.2.6"
irb(main):011> Foo = Data.define(:a,:b)
=> Foo
irb(main):012> d=Psych.dump Foo.new(1,2)
=> "--- !ruby/data:Foo\na: 1\nb: 2\n"
irb(main):013> puts d
--- !ruby/data:Foo
a: 1
b: 2
=> nil
irb(main):014> Foo = Data.define(:a,:b, :c)
(irb):14: warning: already initialized constant Foo
(irb):11: warning: previous definition of Foo was here
=> Foo
irb(main):015> Psych.unsafe_load d
=> #<data Foo a=1, b=2, c=nil>

This is rb_struct_initialize()-specific behavior BTW, Data#initialize doesn't allow that:

> Data.define(:a,:b).new(a: 1)
(irb):3:in 'Data#initialize': missing keyword: :b (ArgumentError)

Checking members would raise, unless we get fancy and only compare the first N members where N is members.size.

Basically this seems a good approach and it's compatible with what was done before.

init_data(data, values)
data.freeze
data

Expand Down
Loading