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

Marshal.dump of Rational sets ivars on array & can't be loaded in MRI #3228

Closed
segiddins opened this issue Aug 24, 2023 · 1 comment
Closed

Comments

@segiddins
Copy link

❯ chruby-exec truffleruby -- ruby --debug -vwe 'p Marshal.dump Rational(1, 3)'
truffleruby 23.0.0, like ruby 3.1.3, Oracle GraalVM Native [aarch64-darwin]
"\x04\bU:\rRationalI[\ai\x06i\b\a:\x0F@numeratori\x06:\x11@denominatori\b"
❯ chruby-exec ruby-3.2.2 -- ruby --debug -vwe 'p Marshal.dump Rational(1, 3)'
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22]
"\x04\bU:\rRational[\ai\x06i\b"
❯ chruby-exec ruby-3.2.2 -- ruby -vwe 'p Marshal.load("\x04\bU:\rRationalI[\ai\x06i\b\a:\x0F@numeratori\x06:\x11@denominatori\b")'
ruby 3.2.2 (2023-03-30 revision e51014f9c0) [arm64-darwin22]
<internal:marshal>:34:in `load': can't modify frozen Rational: (1/3) (FrozenError)
        from -e:1:in `<main>'

I think that omitting the ivars entirely from the dumped array (and thus not nesting under an I element) would work perfectly fine since truffleruby is able to load the more compact dumped output from MRI:

❯ chruby-exec truffleruby -- ruby -vwe 'p Marshal.load("\x04\bU:\rRational[\ai\x06i\b")'
truffleruby 23.0.0, like ruby 3.1.3, Oracle GraalVM Native [aarch64-darwin]
(1/3)
@eregon
Copy link
Member

eregon commented Aug 25, 2023

Thanks for the report.
Rational uses 2 ivars on TruffleRuby internally, but we should be able to match MRI's format for Marshal of Rational.
Maybe the current logic (from Rubinius) dealt with extra ivars on Rational but since Rational is frozen that shouldn't be a concern anymore.

@eregon eregon self-assigned this Aug 25, 2023
eregon added a commit to ruby/spec that referenced this issue Sep 4, 2023
mtortonesi pushed a commit to mtortonesi/truffleruby that referenced this issue Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants