oracle / truffleruby

A high performance implementation of the Ruby programming language, built on GraalVM.
https://www.graalvm.org/ruby/
Other
3.02k stars 185 forks source link

Enumerable#inject without block causes error #2626

Closed djberg96 closed 2 years ago

djberg96 commented 2 years ago

Ruby 3.0.2:

[1,2,3].inject # => LocalJumpError

Truffleruby 22.0.0.2:

[1,2,3].inject # =>

<internal:core> core/enumerable.rb:488:in `inject': TruffleRuby doesn't have a case for the org.truffleruby.core.array.ArrayNodesFactory$InjectNodeFactory$InjectNodeGen node with values of type Array(org.truffleruby.core.array.RubyArray)[[I,3] org.truffleruby.language.NotProvided org.truffleruby.language.NotProvided org.truffleruby.language.Nil (TypeError)
    from org.truffleruby.core.array.ArrayNodesFactory$InjectNodeFactory$InjectNodeGen.executeAndSpecialize(ArrayNodesFactory.java:6945)
    from org.truffleruby.core.array.ArrayNodesFactory$InjectNodeFactory$InjectNodeGen.execute(ArrayNodesFactory.java:6595)
    from org.truffleruby.language.control.IfElseNode.execute(IfElseNode.java:43)
    from org.truffleruby.language.control.SequenceNode.execute(SequenceNode.java:36)
    from org.truffleruby.language.RubyMethodRootNode.execute(RubyMethodRootNode.java:58)
    from org.graalvm.compiler.truffle.runtime.OptimizedCallTarget.executeRootNode(OptimizedCallTarget.java:655)
    from (irb):1: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 /Users/dberger/.rbenv/versions/truffleruby-22.0.0.2/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 /Users/dberger/.rbenv/versions/truffleruby-22.0.0.2/bin/irb:42:in `<main>'
bjfish commented 2 years ago

@eregon This is the spec so far:

  it "raises a LocalJumpError when no parameters or block is given and self has more than 1 items" do
    {}.send(@method).should be_nil
    {one: 1}.send(@method).should == [:one, 1]
    [].send(@method).should be_nil
    [1].send(@method).should == 1
    -> { [1,2].send(@method) }.should raise_error(LocalJumpError)
    -> { {one: 1, two: 2}.send(@method) }.should raise_error(LocalJumpError)
  end
eregon commented 2 years ago

Indeed, that CRuby behavior is weird, looks like a bug/unintended. I think inject should always require either a Symbol argument or a block, but all those cases in that specs do something different.

It should be ArgumentError or LocalJumpError in all cases.

The docs say this:

------------------------------------------------------------------------
  enum.inject(initial, sym) -> obj
  enum.inject(sym)          -> obj
  enum.inject(initial) { |memo, obj| block }  -> obj
  enum.inject          { |memo, obj| block }  -> obj
  enum.reduce(initial, sym) -> obj
  enum.reduce(sym)          -> obj
  enum.reduce(initial) { |memo, obj| block }  -> obj
  enum.reduce          { |memo, obj| block }  -> obj

------------------------------------------------------------------------

Combines all elements of enum by applying a binary operation,
specified by a block or a symbol that names a method or operator.

The inject and reduce methods are aliases. There
is no performance benefit to either.

If you specify a block, then for each element in enum the block
is passed an accumulator value (memo) and the element. If you
specify a symbol instead, then each element in the collection will be
passed to the named method of memo. In either case, the result
becomes the new value for memo. At the end of the iteration, the
final value of memo is the return value for the method.

If you do not explicitly specify an initial value for
memo, then the first element of collection is used as the
initial value of memo.

So enum.inject() is not documented. Maybe enum.inject() uses either the first element (or nil if there isn't), and raises LocalJumpError if 2+ elements? (because then it would need the symbol or block to combine them). The vast majority of Ruby methods check arguments early though, not that late, so it's inconsistent at least.

I would say let's try to fix the reported issue (i.e. LocalJumpError if no argument & no block) and file a ticket on the CRuby tracker for clarification.

bjfish commented 2 years ago

@eregon I've reported the issue here: https://bugs.ruby-lang.org/issues/18635

bjfish commented 2 years ago

CRuby has updated this to be anArgumentError and TruffleRuby has implemented the same at: https://github.com/oracle/truffleruby/commit/ae7e102d13d74c5bc3319c0cc7a852da880ab665.