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

Proc source location gives wrong result #5262

Closed
wants to merge 1 commit into from
Closed

Proc source location gives wrong result #5262

wants to merge 1 commit into from
+10 −14

Conversation

jmalves
Copy link
Contributor

@jmalves jmalves commented Jul 26, 2018 •
edited
Loading

Environment

  • Ruby version: jruby 9.2.0.0 (2.5.0) 2018-05-24 81156a8 Java HotSpot(TM) 64-Bit Server VM 25.152-b16 on 1.8.0_152-b16 +jit [darwin-x86_64]
  • OS/platform: macOS 10.13.5

Expected Behavior

On ruby 2.5.0 and jruby-9.1.17.0 the following script:

expected = __LINE__ + 1
proc = Proc.new do


end

p "#{expected} == #{proc.source_location[1]}"

Prints: 2 == 2

Actual Behavior

On jruby-9.2.0.0 it prints 2 == 5

Investigation

I found this bug by noticing that rspec with line filters (e.g. rspec test_file.rb:20) was not working.

I traced this back to ca25471

I am doubtful that the fix I proposed in this PR is adequate but anyway it was fun to look around this.

Tested with fast specs, mri's test_lambda and test_proc. No spec failures were added but also none were fixed :X

@ahorek
Copy link
Contributor

ahorek commented Jul 26, 2018

there're some failures

 1) Error:
     [exec] TestSyntax#test_brace_block_after_blockcall_colon_no_arg:
     [exec] Java::JavaLang::NullPointerException: 
     [exec]     org.jruby.parser.RubyParser$408.execute(RubyParser.java:4472)
     [exec]     org.jruby.parser.RubyParser.yyparse(RubyParser.java:1695)
...

@enebo enebo mentioned this pull request Aug 21, 2018
Closed
@enebo enebo added this to the Invalid or Duplicate milestone Aug 21, 2018
@enebo
Copy link
Member

enebo commented Aug 21, 2018

I opened up #5286 to capture issue and did a different fix. This fix did not really work because $1 was a Java long to save cmd argument stack. If you look at my commit you can see I added another block making a different $1. yacc is icky and tough to grok. kudos for the attempt though!

@enebo enebo closed this Aug 21, 2018
@jmalves
Copy link
Contributor Author

jmalves commented Aug 21, 2018

The idea behind the PR was to use $-1 which I guessed it should be either { or do but it seems it does not work. Your solution of adding another block makes sense and is also clearer since you do not have to know what $-1 would be.

Thx for taking a look!

@enebo
Copy link
Member

enebo commented Aug 21, 2018

@jmalves yeah I apologize for not noticing the '-'. I think this is a difference of jay perhaps and a more modern LALR-yacc like bison. I believe your PR would work if it wasn't jay.

@kyrylo kyrylo mentioned this pull request Oct 15, 2018
Closed
kyrylo added a commit to kyrylo/method_source that referenced this pull request Nov 1, 2018
@kyrylo
method_source: fix broken Procs on JRuby 9.2.0.0
09cb079 
Fixes pry/pry#1804
(JRuby 9.2.0.0 breaks `source_location` and therefore our test suite)

JRuby 9.2.0.0 fails to fetch source code for procs because of the bug:
jruby/jruby#5262

The problem is that source_location is reported at the end of the proc, instead
of the beginning.

The way I fix it is rather dumb (rewinding back and checking if it's complete
expression) but it's isolated only to 9.2.0.0 and likely won't be needed when
another JRuby is released. However, so far it's the latest release.
@kyrylo kyrylo mentioned this pull request Nov 1, 2018
Merged
kyrylo added a commit to kyrylo/method_source that referenced this pull request Nov 1, 2018
@kyrylo
method_source: fix broken Procs on JRuby 9.2.0.0
53a4442 
Fixes pry/pry#1804
(JRuby 9.2.0.0 breaks `source_location` and therefore our test suite)

JRuby 9.2.0.0 fails to fetch source code for procs because of the bug:
jruby/jruby#5262

The problem is that source_location is reported at the end of the proc, instead
of the beginning.

The way I fix it is rather dumb (rewinding back and checking if it's complete
expression) but it's isolated only to 9.2.0.0 and likely won't be needed when
another JRuby is released. However, so far it's the latest release.
kyrylo added a commit to kyrylo/method_source that referenced this pull request Nov 1, 2018
@kyrylo
method_source: fix broken Procs on JRuby 9.2.0.0
3463d44 
Fixes pry/pry#1804
(JRuby 9.2.0.0 breaks `source_location` and therefore our test suite)

JRuby 9.2.0.0 fails to fetch source code for procs because of the bug:
jruby/jruby#5262

The problem is that source_location is reported at the end of the proc, instead
of the beginning.

The way I fix it is rather dumb (rewinding back and checking if it's complete
expression) but it's isolated only to 9.2.0.0 and likely won't be needed when
another JRuby is released. However, so far it's the latest release.
@kyrylo kyrylo mentioned this pull request Nov 2, 2018
Closed
kyrylo added a commit to kyrylo/method_source that referenced this pull request Nov 2, 2018
@kyrylo
method_source: fix broken Procs on JRuby 9.2.0.0
20fb1ef 
Fixes pry/pry#1804
(JRuby 9.2.0.0 breaks `source_location` and therefore our test suite)

JRuby 9.2.0.0 fails to fetch source code for procs because of the bug:
jruby/jruby#5262

The problem is that source_location is reported at the end of the proc, instead
of the beginning.

The way I fix it is rather dumb (rewinding back and checking if it's complete
expression) but it's isolated only to 9.2.0.0 and likely won't be needed when
another JRuby is released. However, so far it's the latest release.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Invalid or Duplicate
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jmalves @ahorek @enebo