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

Block Parameter Scoping (vs Ruby) #2960

Closed
bppr opened this Issue May 19, 2015 · 2 comments

Comments

Projects
None yet
3 participants
@bppr

bppr commented May 19, 2015

I found that if I use parens around a set of block parameters, JRuby seems to be re-assigning the existing variable in the containing scope. MRI keeps the block parameter local to the block. This only has been an issue in practice if there is a defined variable with the block param's name, but thought it was worth bringing up.

Here's some minimal example code to show the differences in behavior between JRuby and MRI.

def foo(key)  # no parens around block params
  { a: "b", c: "d" }.map { |key, value| [key, value] }
  return key
end

def bar(key)  # note the presence of parens around block params
  { a: "b", c: "d" }.map { |(key, value)| [key, value] }
  return key
end

# Ruby (tested with 1.9.3-p484 and 2.2.0)
puts foo(:hello)  # => :hello
puts bar(:hello)  # => :hello

# JRuby (1.7.20)
puts foo(:hello)  # => :hello
puts bar(:hello)  # => :c

@enebo enebo added this to the JRuby 1.7.21 milestone May 19, 2015

@enebo

This comment has been minimized.

Show comment
Hide comment
@enebo

enebo May 19, 2015

Member

Just verified this does not affect jruby 9k which has same behavior as MRI

Member

enebo commented May 19, 2015

Just verified this does not affect jruby 9k which has same behavior as MRI

@enebo enebo modified the milestones: JRuby 1.7.21, JRuby 1.7.22 Jul 7, 2015

@enebo enebo modified the milestones: JRuby 1.7.22, JRuby 1.7.23 Aug 20, 2015

@enebo enebo modified the milestones: JRuby 1.7.23, JRuby 1.7.24 Nov 24, 2015

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Jan 15, 2016

Member

I've committed a fix for 1.7 that we may want to mimic in 9k.

Currently 9k works around bad parser output in IRBuilder by forcing all variables for a multiple assignment to be block-local if they're arguments to the block. The changes I made to 1.7 fixes (or at least improves) the parser output to force the proper scoping of these variables at parse time. I have not ported it forward because of the other workarounds done in IR that would need to be removed or modified.

Member

headius commented Jan 15, 2016

I've committed a fix for 1.7 that we may want to mimic in 9k.

Currently 9k works around bad parser output in IRBuilder by forcing all variables for a multiple assignment to be block-local if they're arguments to the block. The changes I made to 1.7 fixes (or at least improves) the parser output to force the proper scoping of these variables at parse time. I have not ported it forward because of the other workarounds done in IR that would need to be removed or modified.

@kares kares closed this in e179f01 Jan 20, 2016

enebo added a commit that referenced this issue Jan 21, 2016

Parser changes for #2960: Block Parameter Scoping (vs Ruby).
A followup commit will try and address the depth change hacks in IR
but we still need to create some variables for magic vars in evals
and for flip flop.  So unfornately just deleting that fix code in
IR breaks those things.  Leaving the IR code does not break our
compat so it more of a maintenance issue...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment