Project

General

Profile

Actions

Bug #5930

closed

source_location of blocks incorrect

Bug #5930: source_location of blocks incorrect

Added by charlton (Charlton Wang) over 13 years ago. Updated over 13 years ago.

Status:
Closed
Target version:
-
ruby -v:
1.9.2-p136 and newer
Backport:
[ruby-core:42232]

Description

Similar to bug [ruby-core:27014]) (Closed)" href="/issues/2427">#2427 but maybe the opposite problem. Newer versions of ruby appear to store the source locations of blocks incorrectly when passed as an argument to a multi-line invocation of a function. Snippet of code below reveals the problem:


def foo(*args, &block)
p block.source_location
end

foo(1,
2,
3) do
end

Under ruby 1.9.1-p243: ["-", 8]

Under ruby 1.9.2-p136 and newer: ["-", 5]

It seems with newer versions of ruby, it reports it based on the line on which the function is called instead of where the actual block is. I believe the old behaviour is actually the correct behaviour.

Charlton

Updated by charlton (Charlton Wang) over 13 years ago Actions #1 [ruby-core:42233]

I've verified this behaviour under the following versions:

works: ruby 1.9.1p243 (2009-07-16 revision 24175) [i386-darwin10.4.0]
fails: ruby 1.9.2p136 (2010-12-25 revision 30365) [x86_64-darwin10.4.0]
fails: ruby 2.0.0dev (2012-01-26 trunk 34377) [x86_64-darwin10.8.0]
works: ruby 1.9.1p243 (2009-07-16 revision 24175) [i686-linux]
works: ruby 1.9.1p243 (2009-07-16 revision 24175) [x86_64-linux]
fails: ruby 1.9.2p136 (2010-12-25) [x86_64-linux]

Updated by mame (Yusuke Endoh) over 13 years ago Actions #2 [ruby-core:42314]

Hello,

2012/1/26 Charlton Wang :


def foo(*args, &block)
   p block.source_location
end

foo(1,
   2,
   3) do
end

Under ruby 1.9.1-p243: ["-", 8]

Under ruby 1.9.2-p136 and newer: ["-", 5]

It seems with newer versions of ruby, it reports it based on the line on which the function is called instead of where the actual block is. I believe the old behaviour is actually the correct behaviour.

Do you really like the old behavior? Line 8 points
the end of the block. I prefer the beginning (Line 7).
IOW, a result I expect is: ["-", 7]

The following patch works for me, and passes test-all,
though I'm not sure if the fix is right. I'll commit
it if there is no objection.

diff --git a/parse.y b/parse.y
index 1ad9d62..428b942 100644
--- a/parse.y
+++ b/parse.y
@@ -2798,7 +2798,6 @@ primary : literal
/%%%/
$2->nd_iter = NEW_FCALL($1, 0);
$$ = $2;

  •  	fixpos($2->nd_iter, $2);
         /*%
     	$$ = method_arg(dispatch1(fcall, $1), arg_new());
     	$$ = method_add_block($$, $2);
    

@@ -2811,7 +2810,6 @@ primary : literal
block_dup_check($1->nd_args, $2);
$2->nd_iter = $1;
$$ = $2;

  •  	fixpos($$, $1);
         /*%
     	$$ = method_add_block($1, $2);
         %*/
    

diff --git a/test/ruby/test_proc.rb b/test/ruby/test_proc.rb
index 686db84..3deb5c0 100644
--- a/test/ruby/test_proc.rb
+++ b/test/ruby/test_proc.rb
@@ -1059,6 +1059,20 @@ class TestProc < Test::Unit::TestCase
assert_equal(@@line_of_attr_accessor_source_location_test, lineno)
end

  • def block_source_location_test(*args, &block)
  • block.source_location
  • end
  • def test_block_source_location
  • exp_lineno = LINE + 3
  • file, lineno = block_source_location_test(1,
  •                                          2,
    
  •                                          3) do
    
  •                                          end
    
  • assert_match(/^#{ Regexp.quote(FILE) }$/, file)
  • assert_equal(exp_lineno, lineno)
  • end
  • def test_splat_without_respond_to
    def (obj = Object.new).respond_to?(m,*); false end
    [obj].each do |a, b|

--
Yusuke Endoh

Updated by nobu (Nobuyoshi Nakada) over 13 years ago Actions #3 [ruby-core:42319]

Yusuke Endoh wrote:

The following patch works for me, and passes test-all,
though I'm not sure if the fix is right. I'll commit
it if there is no objection.

It seems right.
Since brace_block already sets the correct line number, there should not be these fixpos()es.
Please commit it.

Updated by charlton (Charlton Wang) over 13 years ago Actions #4 [ruby-core:42320]

Yusuke Endoh wrote:

Do you really like the old behavior? Line 8 points
the end of the block. I prefer the beginning (Line 7).
IOW, a result I expect is: ["-", 7]

I think the old behaviour will point you to the first line after the do. I agree that line 7 would be better but at least line 8 is deterministic so either 7 or 8 would be preferable to 5.

The following patch works for me, and passes test-all,
though I'm not sure if the fix is right. I'll commit
it if there is no objection.

Thanks for the patch!

Charlton

P.S. The patch works wonderfully. Thanks!

Updated by ko1 (Koichi Sasada) over 13 years ago Actions #5

  • Assignee set to mame (Yusuke Endoh)

Updated by shyouhei (Shyouhei Urabe) over 13 years ago Actions #6

  • Status changed from Open to Assigned

Updated by mame (Yusuke Endoh) over 13 years ago Actions #7

  • Status changed from Assigned to Closed
  • % Done changed from 0 to 100

This issue was solved with changeset r35447.
Charlton, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.


  • parse.y (primary): remove wrong "fixpos" that caused incorrect
    source_location of blocks. [ruby-core:42232] [Bug #5930]

  • test/ruby/test_proc.rb: add a test for above.

Actions

Also available in: PDF Atom