Skip to content

Commit

Permalink
[GR-18163] Repair serialising Data instances into Marshal format
Browse files Browse the repository at this point in the history
PullRequest: truffleruby/4411
  • Loading branch information
andrykonchin committed Nov 27, 2024
2 parents 47f7d0e + 69f9d3c commit 7b2cc51
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 1 deletion.
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.

0 comments on commit 7b2cc51

Please sign in to comment.