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

do/end and break precedence issue with omitted parens #4841

Closed
headius opened this Issue Nov 6, 2017 · 3 comments

Comments

Projects
None yet
1 participant
@headius
Member

headius commented Nov 6, 2017

Environment

JRuby 9.1 HEAD

Expected Behavior

Code looking like below exists in Bundler. The break here should work fine since it's within the block passed to foo.

def foo; yield; end
def bar(*); end
bar foo do
  break
end

Actual Behavior

The above code and the similar code in Bundler both raise LocalJumpError. It seems like the break is getting marked as invalid even though it appears inside a block.

To trigger it, run bundle exec from a dir that has no Gemfile, but has a parent dir with a Gemfile.

@headius headius added ir parser labels Nov 6, 2017

@headius headius added this to the JRuby 9.1.14.0 milestone Nov 6, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 6, 2017

Member

My repro is faulty...it was failing for a different reason.

The code from RubyGems (not Bundler) is here:

    Gem::Util.traverse_parents Dir.pwd do |directory|
      next unless gemfile = Gem::GEM_DEP_FILES.find { |f| File.file?(f.untaint) }

      gemfile = File.join directory, gemfile
      break
    end unless gemfile
Member

headius commented Nov 6, 2017

My repro is faulty...it was failing for a different reason.

The code from RubyGems (not Bundler) is here:

    Gem::Util.traverse_parents Dir.pwd do |directory|
      next unless gemfile = Gem::GEM_DEP_FILES.find { |f| File.file?(f.untaint) }

      gemfile = File.join directory, gemfile
      break
    end unless gemfile
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 6, 2017

Member

JRuby 9.1.12 does not appear to have the problem. After installing and updating RubyGems, it's able to bundle exec just fine.

Member

headius commented Nov 6, 2017

JRuby 9.1.12 does not appear to have the problem. After installing and updating RubyGems, it's able to bundle exec just fine.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Nov 8, 2017

Member

Ok I've managed to come up with a reduced repro. It only fails in the interpreter.

def bar(&block)
  2.times do |i|
    p i
    foo(i, &block)
  end
end

def foo(i)
  yield i
end

bar {|i| break if i == 1 }
Member

headius commented Nov 8, 2017

Ok I've managed to come up with a reduced repro. It only fails in the interpreter.

def bar(&block)
  2.times do |i|
    p i
    foo(i, &block)
  end
end

def foo(i)
  yield i
end

bar {|i| break if i == 1 }

headius added a commit that referenced this issue Nov 8, 2017

Add spec for looped delegation of a block containing a break.
In jruby/jruby#4841 we found that JRuby was incorrectly marking
a delegated block (passed via &block) as having "escaped". We use
this escape bit to know whether non-local flow control like
break has a valid target, and so prematurely marking a delegated
block as escaped caused the second iteration's break to raise an
error. The test here loops twice, which is sufficient to test
that the block's break continues to work on the n+1 iteration.

@headius headius added core and removed parser labels Nov 8, 2017

@headius headius closed this in c7ce51d Nov 8, 2017

eregon added a commit to ruby/spec that referenced this issue Dec 1, 2017

Add spec for looped delegation of a block containing a break.
In jruby/jruby#4841 we found that JRuby was incorrectly marking
a delegated block (passed via &block) as having "escaped". We use
this escape bit to know whether non-local flow control like
break has a valid target, and so prematurely marking a delegated
block as escaped caused the second iteration's break to raise an
error. The test here loops twice, which is sufficient to test
that the block's break continues to work on the n+1 iteration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment