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

Wrong number of arguments for Array#[]= subclass redefinition #2772

Closed
djberg96 opened this issue Nov 7, 2022 · 8 comments
Closed

Wrong number of arguments for Array#[]= subclass redefinition #2772

djberg96 opened this issue Nov 7, 2022 · 8 comments

Comments

@djberg96
Copy link
Contributor

djberg96 commented Nov 7, 2022

truffleruby 22.3.0, like ruby 3.0.3, GraalVM CE Native [x86_64-darwin]

Given the following code:

module Foo
  class Bar < Array
    def []=(index, obj)
      puts "ASET"
      super
    end

    def unshift(obj)
      puts "UNSHIFT"
      super
    end
  end
end

a = Foo::Bar.new
a[0] = 1
a.unshift(2)
p a

With Ruby I get the expected results:

ASET
UNSHIFT
[2, 1]

With TruffleRuby I get:

ASET
UNSHIFT
subclass_test.rb:3:in `[]=': wrong number of arguments (given 3, expected 2) (ArgumentError)
	from <internal:core> core/array.rb:1271:in `unshift'
	from subclass_test.rb:10:in `unshift'
	from subclass_test.rb:17:in `<main>'
@eregon
Copy link
Member

eregon commented Nov 8, 2022

Thanks for the report, we'll take a look.
Note that that Array subclass is incompatible, Array#[]= supports 3 arguments, e.g. ary[0, 1] = value.
In this case it's due to unshift being implemented as self[0, 0] = values.

Is there real code depending on this?

@eregon
Copy link
Member

eregon commented Nov 8, 2022

Is there real code depending on this?

Ah it's probably from https://github.com/djberg96/berger_spec I guess.
I think we should fix this, but the priority is low if it doesn't happen in gems/apps, also due to only happening with a slightly-broken Array subclass.

@djberg96
Copy link
Contributor Author

djberg96 commented Nov 8, 2022

Is there real code depending on this?

In this case it was actually https://github.com/djberg96/html-table :)

@djberg96
Copy link
Contributor Author

djberg96 commented Nov 8, 2022

Note that that Array subclass is incompatible, Array#[]= supports 3 arguments, e.g. ary[0, 1] = value.
In this case it's due to unshift being implemented as self[0, 0] = values.

Ah, that might actually be a bug in my code then that happened to smoke out something.

@eregon
Copy link
Member

eregon commented Nov 8, 2022

I see, https://github.com/djberg96/html-table/blob/902b12f72a3f7130331356cbd4b4de9bd7840e15/lib/html/table.rb#L207
In general it's best to not subclass core classes but for compatibility TruffleRuby should support this case.

It's a bit unfortunate because to fix that we probably have to make the core library code less clear & efficient (e.g., using bind_call to call the original []=), or increase maintenance by having unshift written in Java and duplicating logic.

@djberg96
Copy link
Contributor Author

djberg96 commented Nov 8, 2022

Is it something that could be deferred for subclasses?

@eregon
Copy link
Member

eregon commented Nov 9, 2022

Not really, that would probably be more complicated. Anyway I think we need to take that cost here and choose the better alternative of the two.

@andrykonchin
Copy link
Member

Fixed in 534659f.

@andrykonchin andrykonchin added this to the 23.0.0 Release milestone Jan 4, 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

3 participants