Project

General

Profile

Actions

Bug #14226

closed

Revert line number of "return", "b-return" and "end" events

Added by ko1 (Koichi Sasada) over 6 years ago. Updated about 6 years ago.

Status:
Closed
Target version:
-
ruby -v:
2.5
[ruby-core:84417]

Description

[Feature #14104] changes line numbers of several events,

Line numbers on :return/:b_return events show the last executed lines, instead of end lines (without return statement).

But byebug, a well-known debugger depends on this behavior.

1: def foo
2:  bar
3: end

For example, a debugger command "break 3" means "the end of method call of `foo'". It is reasonable feature but on current Ruby 2.5dev, "return" event invoked with line number "2". This change breaks this byebug feature, so that I revert this incompatible behavior soon (before 2.5 release).

The patch is here:

Index: compile.c
===================================================================
--- compile.c	(リビジョン 61423)
+++ compile.c	(作業コピー)
@@ -646,6 +646,7 @@
 		CHECK(COMPILE(ret, "block body", node->nd_body));
 		ADD_LABEL(ret, end);
 		ADD_TRACE(ret, RUBY_EVENT_B_RETURN);
+		ISEQ_COMPILE_DATA(iseq)->last_line = iseq->body->location.code_range.last_loc.lineno;
 
 		/* wide range catch handler must put at last */
 		ADD_CATCH_ENTRY(CATCH_TYPE_REDO, start, end, NULL, start);
@@ -657,6 +658,7 @@
 		ADD_TRACE(ret, RUBY_EVENT_CLASS);
 		CHECK(COMPILE(ret, "scoped node", node->nd_body));
 		ADD_TRACE(ret, RUBY_EVENT_END);
+		ISEQ_COMPILE_DATA(iseq)->last_line = nd_line(node);
 		break;
 	    }
 	  case ISEQ_TYPE_METHOD:
@@ -664,6 +666,7 @@
 		ADD_TRACE(ret, RUBY_EVENT_CALL);
 		CHECK(COMPILE(ret, "scoped node", node->nd_body));
 		ADD_TRACE(ret, RUBY_EVENT_RETURN);
+		ISEQ_COMPILE_DATA(iseq)->last_line = nd_line(node);
 		break;
 	    }
 	  default: {
Index: test/ruby/test_settracefunc.rb
===================================================================
--- test/ruby/test_settracefunc.rb	(リビジョン 61423)
+++ test/ruby/test_settracefunc.rb	(作業コピー)
@@ -77,7 +77,7 @@
                  events.shift)
     assert_equal(["c-return", 5, :+, Integer],
                  events.shift)
-    assert_equal(["return", 5, :add, self.class],
+    assert_equal(["return", 6, :add, self.class],
                  events.shift)
     assert_equal(["line", 8, __method__, self.class],
                  events.shift)
@@ -116,7 +116,7 @@
                  events.shift)
     assert_equal(["c-return", 5, :method_added, Module],
                  events.shift)
-    assert_equal(["end", 5, nil, nil],
+    assert_equal(["end", 7, nil, nil],
                  events.shift)
     assert_equal(["line", 8, __method__, self.class],
                  events.shift)
@@ -130,7 +130,7 @@
                  events.shift)
     assert_equal(["call", 5, :bar, Foo],
                  events.shift)
-    assert_equal(["return", 5, :bar, Foo],
+    assert_equal(["return", 6, :bar, Foo],
                  events.shift)
     assert_equal(["line", 9, __method__, self.class],
                  events.shift)
@@ -176,7 +176,7 @@
                  events.shift)
     assert_equal(["line", 5, :meth_return, self.class],
                  events.shift)
-    assert_equal(["return", 5, :meth_return, self.class],
+    assert_equal(["return", 7, :meth_return, self.class],
                  events.shift)
     assert_equal(["line", 10, :test_return, self.class],
                  events.shift)
@@ -215,7 +215,7 @@
                  events.shift)
     assert_equal(["line", 6, :meth_return2, self.class],
                  events.shift)
-    assert_equal(["return", 6, :meth_return2, self.class],
+    assert_equal(["return", 7, :meth_return2, self.class],
                  events.shift)
     assert_equal(["line", 9, :test_return2, self.class],
                  events.shift)
@@ -343,7 +343,7 @@
      ["line", 4, nil, nil],
      ["c-call", 4, :method_added, Module],
      ["c-return", 4, :method_added, Module],
-     ["end", 4, nil, nil],
+     ["end", 7, nil, nil],
      ["line", 8, __method__, self.class],
      ["c-call", 8, :new, Class],
      ["c-call", 8, :initialize, BasicObject],
@@ -353,7 +353,7 @@
      ["line", 5, :foo, ThreadTraceInnerClass],
      ["c-call", 5, :+, Integer],
      ["c-return", 5, :+, Integer],
-     ["return", 5, :foo, ThreadTraceInnerClass],
+     ["return", 6, :foo, ThreadTraceInnerClass],
      ["line", 9, __method__, self.class],
      ["c-call", 9, :set_trace_func, Thread]].each do |e|
       [:set, :add].each do |type|
@@ -487,7 +487,7 @@
      [:line,    13, "xyzzy", nil,         nil,              xyzzy.class, :XYZZY_outer, :nothing],
      [:c_call,  13, "xyzzy", Module,      :method_added,    xyzzy.class, :XYZZY_outer, :nothing],
      [:c_return,13, "xyzzy", Module,      :method_added,    xyzzy.class, :XYZZY_outer, nil],
-     [:end,     13, "xyzzy", nil,         nil,              xyzzy.class, :XYZZY_outer, :nothing],
+     [:end,     17, "xyzzy", nil,         nil,              xyzzy.class, :XYZZY_outer, :nothing],
      [:line,    18, "xyzzy", TestSetTraceFunc, method,      self,        :outer, :nothing],
      [:c_call,  18, "xyzzy", Class,       :new,             xyzzy.class, :outer, :nothing],
      [:c_call,  18, "xyzzy", BasicObject, :initialize,      xyzzy,       :outer, :nothing],
@@ -502,8 +502,8 @@
      [:line,    15, "xyzzy", xyzzy.class, :bar,             xyzzy,       :XYZZY_bar, :nothing],
      [:c_call,  15, "xyzzy", Kernel,      :tap,             xyzzy,       :XYZZY_bar, :nothing],
      [:c_return,15, "xyzzy", Kernel,      :tap,             xyzzy,       :XYZZY_bar, xyzzy],
-     [:return,  15, "xyzzy", xyzzy.class, :bar,             xyzzy,       :XYZZY_bar, xyzzy],
-     [:return,  11, "xyzzy", xyzzy.class, :foo,             xyzzy,       :XYZZY_foo, xyzzy],
+     [:return,  16, "xyzzy", xyzzy.class, :bar,             xyzzy,       :XYZZY_bar, xyzzy],
+     [:return,  12, "xyzzy", xyzzy.class, :foo,             xyzzy,       :XYZZY_foo, xyzzy],
      [:line,    20, "xyzzy", TestSetTraceFunc, method,      self,        :outer, :nothing],
      [:c_call,  20, "xyzzy", Kernel,      :raise,           self,        :outer, :nothing],
      [:c_call,  20, "xyzzy", Exception,   :exception,       RuntimeError, :outer, :nothing],
@@ -1800,7 +1800,7 @@
     assert_equal ["line", base_line + 7],     events[4]
     assert_equal ["line", base_line + 8],     events[5]
     assert_equal ["call", base_line + -6],    events[6]
-    assert_equal ["return", base_line + -6],  events[7]
+    assert_equal ["return", base_line + -4],  events[7]
     assert_equal ["line", base_line + 9],     events[8]
     assert_equal nil,                         events[9]
 
@@ -1835,7 +1835,7 @@
     assert_equal ["line", base_line + 32],     events[1]
     assert_equal ["line", base_line + 33],     events[2]
     assert_equal ["call", base_line + -6],     events[3]
-    assert_equal ["return", base_line + -6],   events[4]
+    assert_equal ["return", base_line + -4],   events[4]
     assert_equal ["line", base_line + 34],     events[5]
     assert_equal ["line", base_line + 35],     events[6]
     assert_equal ["c-call", base_line + 35],   events[7] # Thread.current

Related issues 1 (1 open0 closed)

Related to Ruby master - Feature #14276: Ruby core debugger APIOpenActions
Actions #1

Updated by ko1 (Koichi Sasada) over 6 years ago

  • Description updated (diff)

Updated by ko1 (Koichi Sasada) over 6 years ago

  • Subject changed from Revert line number of "return", "b-return" and "c-return" events to Revert line number of "return", "b-return" and "end" events

Modify title.
Not "c-return" but "end" (end of class/module statements) event.

Actions #3

Updated by dsferreira (Daniel Ferreira) over 6 years ago

I take the chance to ask here about something I have been thinking for a very long time.
Doesn’t it make sense to have the debugger API in ruby core instead of a gem?
We already got debugger gems broken in the past due to ruby core changes.
Now we rely on byebug and pry for debug purposes.
Wouldn’t it make more sense to have the debugger controlled by ruby core itself?
The way I see it, a language like ruby without a debugger is not usable in practical terms.
For that reason I tend to see the debug API as a core feature.
I would compare this logic with documentation functionality and also coverage.
I’m talking about a clear API that would then be able to be extended by third party gems for integration purposes with IDEs and other programming tools.

My vision on this is that once ruby is installed we should be able to start to use TDD from start without the need for gems. Debug, Coverage, Documentation and Unit tests are a must on that regard. Maybe we can add here to this list benchmarks and performance as well and maybe the list should be longer.

At least in ruby-lang.org it would make sense to have clear guidelines on these essential programming tools promoting ruby best practices from the very beginning.

Lots of times before I felt the lack of this integration.

What do you think?

Actions #4

Updated by naruse (Yui NARUSE) over 6 years ago

  • Target version deleted (2.5)

Updated by duerst (Martin Dürst) about 6 years ago

dsferreira (Daniel Ferreira) wrote:

I take the chance to ask here about something I have been thinking for a very long time.
Doesn’t it make sense to have the debugger API in ruby core instead of a gem?

That's something more general than this issue. I propose you create a new feature for this, so that it can be discussed in its own right. I'm generally in favor, although there is always the issue that Ruby is a volunteer project, so some things may not get done even if everybody agrees they are desirable.

Updated by dsferreira (Daniel Ferreira) about 6 years ago

I propose you create a new feature for this

Thanks Martin.
I will create it.

Updated by dsferreira (Daniel Ferreira) about 6 years ago

The new issue has been created: Ruby core debugger API

Actions #8

Updated by duerst (Martin Dürst) about 6 years ago

Actions #9

Updated by ko1 (Koichi Sasada) about 6 years ago

  • Status changed from Open to Closed
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0