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

Array#fill double error? #2652

Closed
djberg96 opened this issue May 3, 2022 · 4 comments
Closed

Array#fill double error? #2652

djberg96 opened this issue May 3, 2022 · 4 comments

Comments

@djberg96
Copy link
Contributor

djberg96 commented May 3, 2022

Not sure what's happening here, but it seems to raise two errors, an ArgumentError and a TypeError. It should just be a TypeError:

TruffleRuby 22.1.0:

> a = %w[a b c d]
=> ["a", "b", "c", "d"]
irb(main):002:0> a.fill('x', 1, 'x')
<internal:core> core/array.rb:461:in `fill_internal': second argument must be an Integer (ArgumentError)
	from (irb):2:in `fill'
	from (irb):2:in `<top (required)>'
	from <internal:core> core/kernel.rb:407:in `loop'
	from <internal:core> core/throw_catch.rb:36:in `catch'
	from <internal:core> core/throw_catch.rb:36:in `catch'
	from /home/dberger/.rbenv/versions/truffleruby-22.1.0/lib/gems/gems/irb-1.3.5/exe/irb:11:in `<top (required)>'
	from <internal:core> core/kernel.rb:376:in `load'
	from <internal:core> core/kernel.rb:376:in `load'
	from /home/dberger/.rbenv/versions/truffleruby-22.1.0/bin/irb:42:in `<main>'
<internal:core> core/type.rb:276:in `convert_type': no implicit conversion of String into Integer (TypeError)
	from <internal:core> core/type.rb:178:in `rb_to_int_fallback'
	from <internal:core> core/array.rb:459:in `fill_internal'
	from (irb):2:in `fill'
	from (irb):2:in `<top (required)>'
	from <internal:core> core/kernel.rb:407:in `loop'
	from <internal:core> core/throw_catch.rb:36:in `catch'
	from <internal:core> core/throw_catch.rb:36:in `catch'
	from /home/dberger/.rbenv/versions/truffleruby-22.1.0/lib/gems/gems/irb-1.3.5/exe/irb:11:in `<top (required)>'
	from <internal:core> core/kernel.rb:376:in `load'
	from <internal:core> core/kernel.rb:376:in `load'
	from /home/dberger/.rbenv/versions/truffleruby-22.1.0/bin/irb:42:in `<main>'

Ruby 3.0.x:

> a = %w[a b c d]
=> ["a", "b", "c", "d"]
irb(main):002:0> a.fill('x', 1, 'x')
(irb):2:in `fill': no implicit conversion of String into Integer (TypeError)
	from (irb):2:in `<main>'
	from /home/dberger/.rbenv/versions/3.0.4/lib/ruby/gems/3.0.0/gems/irb-1.3.5/exe/irb:11:in `<top (required)>'
	from /home/dberger/.rbenv/versions/3.0.4/bin/irb:23:in `load'
	from /home/dberger/.rbenv/versions/3.0.4/bin/irb:23:in `<main>'
@eregon
Copy link
Member

eregon commented May 4, 2022

It's best to reproduce this outside of IRB for simplicity and smaller stacktraces:

$ ruby -e '%w[a b c d].fill("x", 1, "x")'
-e:1:in `fill': no implicit conversion of String into Integer (TypeError)
	from -e:1:in `<main>'


$ truffleruby -e '%w[a b c d].fill("x", 1, "x")'
<internal:core> core/array.rb:461:in `fill_internal': second argument must be an Integer (ArgumentError)
	from -e:1:in `fill'
	from -e:1:in `<main>'
<internal:core> core/type.rb:276:in `convert_type': no implicit conversion of String into Integer (TypeError)
	from <internal:core> core/type.rb:178:in `rb_to_int_fallback'
	from <internal:core> core/array.rb:459:in `fill_internal'
	from -e:1:in `fill'
	from -e:1:in `<main>'

It's simply an exception which has a cause.
Which is not surprising given the code is this:

begin
right = Primitive.rb_num2int length
rescue TypeError
raise ArgumentError, 'second argument must be an Integer'
end

So I guess we should change that to be a TypeError and not ArgumentError, but otherwise I think we should leave the cause as it is to reflect the wrapped and the original exception.

BTW the error message from CRuby is very confusing, 'x' is what fails to be converted and that's the 3rd argument not the 2nd. Maybe we should not try to match CRuby's message since it's not that helpful, and just let the original TypeError propagate, then we would also not need another exception.

@djberg96
Copy link
Contributor Author

djberg96 commented May 4, 2022

BTW the error message from CRuby is very confusing, 'x' is what fails to be converted and that's the 3rd argument not the 2nd.

I'm not following. The CRuby error message looks correct to me. It's only the Truffleruby error message that makes reference to the 2nd argument.

@eregon
Copy link
Member

eregon commented May 4, 2022

Oh you are right, I confused the output somehow.
Then we we just remove this rescue TypeError + raise ArgumentError (which I guess comes from Rubinius) and we should match CRuby.

@andrykonchin
Copy link
Member

Fixed in 4693f7b, thank you for the report.

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