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.

Sign up for GitHub

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

Jump to bottom

Interpreter vs JIT difference in access levels for nested method definitions at the toplevel #2205

Closed
subbuss opened this issue Nov 16, 2014 · 5 comments
Closed

Interpreter vs JIT difference in access levels for nested method definitions at the toplevel #2205

subbuss opened this issue Nov 16, 2014 · 5 comments
Labels
ir
Milestone
JRuby 9.0.0.0-pre1

Comments

@subbuss
Copy link
Contributor

subbuss commented Nov 16, 2014

See the interp vs JIT difference in behavior in method definitions at the toplevel. JIT behavior seems consistent with MRI 2.2 behavior. This difference seems to show up only for nested method definitions at the top level.

[subbu@earth lib] cat /tmp/priv.rb 
def toplevel_m1
  def toplevel_m2
    def toplevel_m3
    end
  end
end

toplevel_m1
toplevel_m2

p "public : #{Object.public_instance_methods.grep /top/}"
p "private: #{Object.private_instance_methods.grep /top/}"
[subbu@earth ir.docs] jruby /tmp/priv.rb 
"public : [:toplevel_m2, :toplevel_m3]"
"private: [:toplevel_m1]"
[subbu@earth ir.docs] jruby -X-C !$
jruby -X-C /tmp/priv.rb
"public : []"
"private: [:toplevel_m2, :toplevel_m3, :toplevel_m1]"
subbu@earth:~/jruby/ir.docs$ ruby /tmp/priv.rb 
"public : [:toplevel_m2, :toplevel_m3]"
"private: [:toplevel_m1]"
@subbuss subbuss added the ir label Nov 16, 2014
@subbuss subbuss added this to the JRuby 9.0.0.0 milestone Nov 16, 2014
@subbuss
Copy link
Contributor Author

subbuss commented Nov 17, 2014

So, as far as the difference between interpreter and JIT goes, I traced it line 1342 in JVMVisitor.java which pushes PUBLIC visibility always for a PushFrameInstr (jvmAdapter().ldc(Visibility.PUBLIC.ordinal());)

However, the interpreter pushes the visibility that comes from the method being interpreted which seems correct. However, if I force interpreter to always push PUBLIC visibility for PushFrame, interpreter and JIT agree with MRI.

I am missing something here @headius @enebo

@subbuss subbuss mentioned this issue Nov 17, 2014
Closed
@headius
Copy link
Member

headius commented Nov 17, 2014

Do you need more information than what we discussed on IRC? (only toplevel is private by default, all others public...if I still know the rules correctly)

Does fixing this make 2205 obsolete, or was that a valid difference from MRI?

@subbuss
Copy link
Contributor Author

subbuss commented Nov 17, 2014

That was a valid difference between the interpreter and MRI which I fixed with that commit.

@headius
Copy link
Member

headius commented Nov 17, 2014

Sorry, I meant "does fixing this make #2201 obsolete", since that is another related visibility issue?

@subbuss
Copy link
Contributor Author

subbuss commented Nov 17, 2014

Ah, no. I don't think so. This fixed visibility issues in the interpreter whereas #2201 is JIT-related. I verified with the test case in #2201 and it is not fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
ir
Projects
None yet
Milestone
JRuby 9.0.0.0-pre1
Development

No branches or pull requests
2 participants
@headius @subbuss