Bug #5930
closedsource_location of blocks incorrect
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 12 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 charlton.wang@gmail.com:
def foo(*args, &block)
p block.source_location
endfoo(1,
2,
3) do
endUnder 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 mame@tsg.ne.jp
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!
Updated by ko1 (Koichi Sasada) over 12 years ago
- Assignee set to mame (Yusuke Endoh)
Updated by shyouhei (Shyouhei Urabe) over 12 years ago
- Status changed from Open to Assigned
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.