From: eregontp@...
Date: 2018-12-27T22:34:22+00:00
Subject: [ruby-core:90757] [Ruby trunk Feature#15473] TracePoint#enable(target_thread:) to specify a Thread

Issue #15473 has been updated by Eregon (Benoit Daloze).


ko1 (Koichi Sasada) wrote:
> Maybe `Thread.current` is frequent pattern, so accepting :current` symbol is reasonable for me.

I think it's not needed, and that passing `Thread.current` is clearer and simpler, so I would propose to not add this special shortcut.

The feature seems useful and good to me, and clearer (more explicit & compatible) than #13483.

BTW, I noticed `ri TracePoint#enable` doesn't show any documentation (because it's attached as `__enable`), and that target: and target_line: are not documented.

----------------------------------------
Feature #15473: TracePoint#enable(target_thread:) to specify a Thread
https://bugs.ruby-lang.org/issues/15473#change-75930

* Author: ko1 (Koichi Sasada)
* Status: Open
* Priority: Normal
* Assignee: ko1 (Koichi Sasada)
* Target version: 
----------------------------------------
We introduced `TracePoint#enable(target:, target_line:)` keyword.
How about to introduce `target_thread:` keyword which specify enabling threads?

```ruby
TracePoint.new(:line){|tp| p tp}.enable(target_thread: Thread.current) do
  p 1
  Thread.new{
    p 10
    p 12
  }.join
  p 2
end
```

```
#<TracePoint:line@/home/ko1/src/ruby/trunk/test.rb:3>
1
#<TracePoint:line@/home/ko1/src/ruby/trunk/test.rb:4>
10
12
#<TracePoint:line@/home/ko1/src/ruby/trunk/test.rb:8>
2
```

Maybe `Thread.current` is frequent pattern, so accepting :current` symbol is reasonable for me.

```
TracePoint.new(:line){|tp| p tp}.enable(target_thread: :current) do
  p 1
  Thread.new{
    p 10
    p 12
  }.join
  p 2
end

# same result
```


This is a patch.

```diff
Index: prelude.rb
===================================================================
--- prelude.rb	(revision 66594)
+++ prelude.rb	(working copy)
@@ -133,8 +133,8 @@ class IO
 end
 
 class TracePoint
-  def enable target: nil, target_line: nil, &blk
-    self.__enable target, target_line, &blk
+  def enable target: nil, target_line: nil, target_thread: nil, &blk
+    self.__enable target, target_line, target_thread, &blk
   end
 end
 
Index: vm_trace.c
===================================================================
--- vm_trace.c	(revision 66594)
+++ vm_trace.c	(working copy)
@@ -1398,11 +1398,26 @@ rb_hook_list_remove_tracepoint(rb_hook_l
  *
  */
 static VALUE
-tracepoint_enable_m(VALUE tpval, VALUE target, VALUE target_line)
+tracepoint_enable_m(VALUE tpval, VALUE target, VALUE target_line, VALUE target_thread)
 {
     rb_tp_t *tp = tpptr(tpval);
     int previous_tracing = tp->tracing;
 
+    if (RTEST(target_thread)) {
+        static VALUE sym_current = 0;
+        if (sym_current == 0) sym_current = rb_id2sym(rb_intern("current"));
+
+        if (target_thread == sym_current) {
+            tp->target_th = GET_THREAD();
+        }
+        else {
+            tp->target_th = rb_thread_ptr(target_thread);
+        }
+    }
+    else {
+        tp->target_th = NULL;
+    }
+
     if (NIL_P(target)) {
         if (!NIL_P(target_line)) {
             rb_raise(rb_eArgError, "only target_line is specified");
@@ -1801,7 +1816,7 @@ Init_vm_trace(void)
      */
     rb_define_singleton_method(rb_cTracePoint, "trace", tracepoint_trace_s, -1);
 
-    rb_define_method(rb_cTracePoint, "__enable", tracepoint_enable_m, 2);
+    rb_define_method(rb_cTracePoint, "__enable", tracepoint_enable_m, 3);
     rb_define_method(rb_cTracePoint, "disable", tracepoint_disable_m, 0);
     rb_define_method(rb_cTracePoint, "enabled?", rb_tracepoint_enabled_p, 0);
 
```

This is a similar feature proposed at https://bugs.ruby-lang.org/issues/13483 but no compatibility issue.




-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:ruby-core-request@ruby-lang.org?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>