Skip to content
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.

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

[GR-18163] Repair serialising Data instances into Marshal format #3729

Merged
merged 1 commit into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Compatibility:
* Add `IO#{pread, pwrite}` methods (#3718, @andrykonchin).
* Add `rb_io_closed_p()` (#3681, @andrykonchin).
* Add `rb_io_open_descriptor()` (#3681, @andrykonchin).
* Support serializing of `Data` instances into Marshal format (#3726, @andrykonchin).

Performance:

Expand Down
22 changes: 22 additions & 0 deletions spec/ruby/core/marshal/dump_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,28 @@ def _dump(level)
end
end

ruby_version_is "3.2" do
describe "with a Data" do
it "dumps a Data" do
Marshal.dump(MarshalSpec::DataSpec::Measure.new(100, 'km')).should == "\x04\bS:#MarshalSpec::DataSpec::Measure\a:\vamountii:\tunit\"\akm"
end

it "dumps an extended Data" do
obj = MarshalSpec::DataSpec::MeasureExtended.new(100, "km")
Marshal.dump(obj).should == "\x04\bS:+MarshalSpec::DataSpec::MeasureExtended\a:\vamountii:\tunit\"\akm"
end

it "ignores overridden name method" do
obj = MarshalSpec::DataSpec::MeasureWithOverriddenName.new(100, "km")
Marshal.dump(obj).should == "\x04\bS:5MarshalSpec::DataSpec::MeasureWithOverriddenName\a:\vamountii:\tunit\"\akm"
end

it "raises TypeError with an anonymous Struct" do
-> { Marshal.dump(Data.define(:a).new(1)) }.should raise_error(TypeError, /can't dump anonymous class/)
end
end
end

describe "with a String" do
it "dumps a blank String" do
Marshal.dump("".dup.force_encoding("binary")).should == "\004\b\"\000"
Expand Down
16 changes: 16 additions & 0 deletions spec/ruby/core/marshal/fixtures/marshal_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,22 @@ class ObjectWithoutFreeze < Object
"\004\bS:\024Struct::Pyramid\000"],
"Random" => random_data,
}

if defined? Data # TODO: remove the condition when minimal supported version is 3.2
module DataSpec
Measure = Data.define(:amount, :unit)
Empty = Data.define

MeasureExtended = Class.new(Measure)
MeasureExtended.extend(Enumerable)

class MeasureWithOverriddenName < Measure
def self.name
"Foo"
end
end
end
end
end

class ArraySub < Array
Expand Down
28 changes: 28 additions & 0 deletions spec/ruby/core/marshal/shared/load.rb
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,34 @@ def io.binmode; raise "binmode"; end
end
end

ruby_version_is "3.2" do
describe "for a Data" do
it "loads a Data" do
obj = MarshalSpec::DataSpec::Measure.new(100, 'km')
dumped = "\x04\bS:#MarshalSpec::DataSpec::Measure\a:\vamountii:\tunit\"\akm"
Marshal.dump(obj).should == dumped

Marshal.send(@method, dumped).should == obj
end

it "loads an extended Data" do
obj = MarshalSpec::DataSpec::MeasureExtended.new(100, "km")
dumped = "\x04\bS:+MarshalSpec::DataSpec::MeasureExtended\a:\vamountii:\tunit\"\akm"
Marshal.dump(obj).should == dumped

Marshal.send(@method, dumped).should == obj
end

it "returns a frozen object" do
obj = MarshalSpec::DataSpec::Measure.new(100, 'km')
dumped = "\x04\bS:#MarshalSpec::DataSpec::Measure\a:\vamountii:\tunit\"\akm"
Marshal.dump(obj).should == dumped

Marshal.send(@method, dumped).should.frozen?
end
end
end

describe "for an Exception" do
it "loads a marshalled exception with no message" do
obj = Exception.new
Expand Down
36 changes: 36 additions & 0 deletions src/main/ruby/truffleruby/core/marshal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,39 @@ class Struct
end
end

class Data
# Data is serialized like Struct with the same type 'S'
private def __marshal__(ms)
# Data instances are frozen so no instance variables could be set (with #instance_variable_set).
# But when Data class has no members - then its instances are allowed to be assigned instance variables.
# Foo = Data.define
# f = Foo.new
# f.instance_variable_set(:@a, 42) # => 42
# So keep the logic of serializing instance variables for now.
out = ms.serialize_instance_variables_prefix(self)
Primitive.string_binary_append out, ms.serialize_extended_object(self)

Primitive.string_binary_append out, 'S'

cls = Primitive.class self
if Primitive.module_anonymous?(cls)
raise TypeError, "can't dump anonymous class #{cls}"
end
class_name = Primitive.module_name cls
Primitive.string_binary_append out, ms.serialize(class_name.to_sym)
Primitive.string_binary_append out, ms.serialize_integer(self.members.length)

self.to_h.each_pair do |name, value|
Primitive.string_binary_append out, ms.serialize(name)
Primitive.string_binary_append out, ms.serialize(value)
end

Primitive.string_binary_append out, ms.serialize_instance_variables_suffix(self)

out
end
end

class Array
private def __marshal__(ms)
out = ms.serialize_instance_variables_prefix(self)
Expand Down Expand Up @@ -952,6 +985,7 @@ def construct_regexp(ivar_index)
store_unique_object obj
end

# handle both Struct and Data classes
def construct_struct
name = get_symbol

Expand All @@ -970,6 +1004,8 @@ def construct_struct
Primitive.object_hidden_var_set obj, slot, construct
end

obj.freeze if Primitive.is_a?(obj, Data)

obj
end

Expand Down
1 change: 0 additions & 1 deletion test/mri/excludes/TestData.rb

This file was deleted.

Loading