Bug #14226: Revert line number of "return", "b-return" and "end" events

• Status: Closed
• Assignee: ko1 (Koichi Sasada)
• Target version: -
• ruby -v: 2.5
• Backport: 2.3: UNKNOWN, 2.4: UNKNOWN

[ko1 (Koichi Sasada)]

[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

[ko1 (Koichi Sasada)]

• Description updated (diff)

[ko1 (Koichi Sasada)]

• 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.

[dsferreira (Daniel Ferreira)]

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?

[naruse (Yui NARUSE)]

• Target version deleted (2.5)

[duerst (Martin Dürst)]

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.

[dsferreira (Daniel Ferreira)]

I propose you create a new feature for this

Thanks Martin.
I will create it.

[dsferreira (Daniel Ferreira)]

The new issue has been created: Ruby core debugger API

[duerst (Martin Dürst)]

• Related to Feature #14276: Ruby core debugger API added

[ko1 (Koichi Sasada)]

• Status changed from Open to Closed