Project

General

Profile

Actions

Bug #5930

closed

source_location of blocks incorrect

Added by charlton (Charlton Wang) almost 13 years ago. Updated over 12 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) almost 13 years ago

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 12 years ago

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 12 years ago

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 12 years ago

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!

Actions #5

Updated by ko1 (Koichi Sasada) over 12 years ago

  • Assignee set to mame (Yusuke Endoh)
Actions #6

Updated by shyouhei (Shyouhei Urabe) over 12 years ago

  • Status changed from Open to Assigned
Actions #7

Updated by mame (Yusuke Endoh) over 12 years ago

  • 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: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0