Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

JRuby fails to parse a block under certain circumstances #305

Closed
alex opened this Issue · 20 comments

7 participants

@alex

Code to reproduce is

return [].inject 0 do
end

Note that this also fails under Rubiniux (FWIW):

alex@alex-gaynor-laptop:/tmp$ ruby-1.9.3-p125 t.rb 
t.rb:1:in `<main>': unexpected return (LocalJumpError)
alex@alex-gaynor-laptop:/tmp$ jruby-1.6.7.2 --1.9 t.rb 
SyntaxError: t.rb:1: syntax error, unexpected kDO_BLOCK

return [].inject 0 do
^
alex@alex-gaynor-laptop:/tmp$ rbx-head -X19 t.rb 
An exception occurred running t.rb
    Error trying to compile /tmp/t.rb (Rubinius::CompileError)

Backtrace:
  Rubinius::Compiler.compiler_error at /home/alex/.rvm/rubies/rbx-head/runtime/19/compiler/compiler.rbc:13
         Rubinius::Compiler.compile at /home/alex/.rvm/rubies/rbx-head/runtime/19/compiler/compiler.rbc:92
  Rubinius::CodeLoader#compile_file at kernel/delta/codeloader.rb:166
     Rubinius::CodeLoader#load_file at kernel/delta/codeloader.rb:138
   Rubinius::CodeLoader#load_script at kernel/delta/codeloader.rb:63
   Rubinius::CodeLoader.load_script at kernel/delta/codeloader.rb:109
            Rubinius::Loader#script at kernel/loader.rb:640
              Rubinius::Loader#main at kernel/loader.rb:844

Caused by: undefined method `block=' on an instance of Rubinius::AST::Return. (NoMethodError)

Backtrace:
   Kernel(Rubinius::AST::Return)#block= (method_missing) at kernel/delta/kernel.rb:81
                     Rubinius::Melbourne19#process_iter at /home/alex/.rvm/rubies/rbx-head/runtime/19/melbourne
                                                           /processor.rbc:550
 Rubinius::Melbourne(Rubinius::Melbourne19)#file_to_ast_19 at /home/alex/.rvm/src/rbx-head/lib/ext/melbourne
                                                              /melbourne.cpp
   Rubinius::Melbourne(Rubinius::Melbourne19)#parse_file at /home/alex/.rvm/rubies/rbx-head/runtime/19/melbourne.rbc:84
                   Rubinius::Compiler::FileParser#parse at /home/alex/.rvm/rubies/rbx-head/runtime/19/compiler
                                                           /stages.rbc:235
 Rubinius::Compiler::Parser(Rubinius::Compiler::FileParser)#run at /home/alex/.rvm/rubies/rbx-head/runtime/19/compiler
                                                                   /stages.rbc:217
                                 Rubinius::Compiler#run at /home/alex/.rvm/rubies/rbx-head/runtime/19/compiler
                                                           /compiler.rbc:374
                             Rubinius::Compiler.compile at /home/alex/.rvm/rubies/rbx-head/runtime/19/compiler
                                                           /compiler.rbc:88
                       Rubinius::CodeLoader#compile_file at kernel/delta/codeloader.rb:166
                          Rubinius::CodeLoader#load_file at kernel/delta/codeloader.rb:138
                        Rubinius::CodeLoader#load_script at kernel/delta/codeloader.rb:63
                        Rubinius::CodeLoader.load_script at kernel/delta/codeloader.rb:109
                                 Rubinius::Loader#script at kernel/loader.rb:640
                                   Rubinius::Loader#main at kernel/loader.rb:844

@alex

Managed to simplify the test case slightly

return f 0 do
end
@headius
Owner

Yeah, I think this has been reported before. The precedence of "do" is going all the way to return.

@headius
Owner

Appears to be working in all modes on master.

The missing LocalJumpError is a known issue.

system ~/projects/jruby $ jruby -e "return [1].map do; puts 'here'; end"
here

system ~/projects/jruby $ jruby --1.8 -e "return [1].map do; puts 'here'; end"
here

system ~/projects/jruby $ jruby -X-C -e "return [1].map do; puts 'here'; end"
here

system ~/projects/jruby $ jruby -X-C --1.8 -e "return [1].map do; puts 'here'; end"
here

system ~/projects/jruby $ jruby -X-CIR -e "return [1].map do; puts 'here'; end"
here

system ~/projects/jruby $ jruby -X-CIR --1.8 -e "return [1].map do; puts 'here'; end"
here
@headius headius closed this
@headius headius reopened this
@headius
Owner

Oops, my case wasn't the same as yours. Yours still fails.

@headius
Owner

Ruby 1.8 has the same issue:

system ~/projects/jruby $ ruby-1.8.7-p358 -e "return f 0 do; puts 'here'; end"
-e:1: syntax error, unexpected kDO_BLOCK, expecting $end
return f 0 do; puts 'here'; end
             ^
@alex

For reference, rubinius issue is #1917.

@urbanautomaton

I'm trying to port a relatively large rails app to jruby and am running into this issue (at least, the error is identical). Is there any way to get the parser to show what line of code is causing the error?

(edit: sorry, should've said: jruby 1.7.3)

@badosu

@urbanautomaton maybe you could run:

grep "return .* do\($\| |\)" -R *

@urbanautomaton

Hi @badosu - thanks, in the end I found it in a similar manner. Lurking in an obscure gem dependency, which is why it didn't crop up the first time I looked.

@enebo enebo closed this in 53710d8
@enebo
Owner

Wow. I still don't know why we go down the wrong productions but I did fix it in spite of that. My work-around in the grammar is a hack but a pretty harmless one since the production should never receive nodes like return. If this gets fixed for real then at worst it will unreachable dead code.

@whitequark

@enebo For the record, you don't go down wrong productions. MRI constructs a similarly weird AST: https://gist.github.com/whitequark/8b526b84b288022e9d46 and invokes all the same productions, as evident by ruby -y.

@enebo
Owner

OH!?!?! Well that is a lead worth pursuing. Thanks. I guess I did work around the issue in the end but I would like to make a sane grammar change to make this proper. Especially for jruby-parser, but also for JRuby if for no other reason than not being weird.

@whitequark whitequark referenced this issue in seattlerb/ruby_parser
Closed

Return takes a block #140

@whitequark

@enebo I'm not sure honestly, this restructuring could easily prove very nontrivial (due to cmdarg stack state insanity and general complexity of parser) and make maintenance harder because merging any changes to grammar would require re-examining whether they break this patch.

As much as I love clean grammars, in Parser I decided to just work around this in a reduction rule (similarly to yours).

@enebo
Owner
@chrisseaton
Collaborator

Why did this issue get closed - it's still there isn't it? That's why this spec is tagged https://github.com/jruby/jruby/blob/master/spec/tags/ruby/language/return_tags.txt#L2.

@chrisseaton chrisseaton reopened this
@enebo
Owner

I had thought I fixed the case in question with the commit. That fix did feel like a hack to me so perhaps some case is still wrong?

@chrisseaton
Collaborator

The spec was still tagged, and try this test case (just the spec minimised) before and after my commit above:

class MethodWithBlock
  def method1
    return [2, 3].inject 0 do |a, b|
      a + b
    end
    nil
  end

  def get_ary(count, &blk)
    count.times.to_a do |i|
      blk.call(i) if blk
    end
  end

  def method2
    return get_ary 3 do |i|
    end
    nil
  end
end

p MethodWithBlock.new.method1#.should == 5
p MethodWithBlock.new.method2#.should == [0, 1, 2]

My fix actually looks very similar to yours - I'm not sure what the essential difference is that makes it work.

@enebo
Owner

Heh. I am happy you fixed it :)

@chrisseaton
Collaborator

Yeah, only took an entire day to get one extra passing spec!

@headius
Owner

:+1:

@enebo enebo referenced this issue from a commit
@enebo enebo Consolidate fixes for #305 6d43d9f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.