* vm_trace.c: TracePoint#enable should not cause an error

  when it is already enabled. TracePoint#disable is too.
  [ruby-core:50561] [ruby-trunk - Bug #7513]
* test/ruby/test_settracefunc.rb: add tests.



git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@38227 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

3 files changed, 57 insertions, 26 deletions

diff --git a/ChangeLog b/ChangeLog
index 916ac062c0..db950ecff0 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+Thu Dec 06 12:07:11 2012  Koichi Sasada  <ko1@atdot.net>
+
+	* vm_trace.c: TracePoint#enable should not cause an error
+	  when it is already enabled. TracePoint#disable is too.
+	  [ruby-core:50561] [ruby-trunk - Bug #7513]
+
+	* test/ruby/test_settracefunc.rb: add tests.
+
 Thu Dec  6 07:19:58 2012  Eric Hodel  <drbrain@segment7.net>
 
 	* lib/rdoc*:  Improved display of ChangeLog files as HTML.
diff --git a/test/ruby/test_settracefunc.rb b/test/ruby/test_settracefunc.rb
index 6ffb2dc6e4..6775b72b89 100644
--- a/test/ruby/test_settracefunc.rb
+++ b/test/ruby/test_settracefunc.rb
@@ -623,6 +623,16 @@ class TestSetTraceFunc < Test::Unit::TestCase
     }
     foo
     assert_equal([:foo], ary)
+
+    trace = TracePoint.new{}
+    begin
+      assert_equal(false, trace.enable)
+      assert_equal(true, trace.enable)
+      trace.enable{}
+      assert_equal(true, trace.enable)
+    ensure
+      trace.disable
+    end
   end
 
   def test_tracepoint_disable
@@ -637,6 +647,14 @@ class TestSetTraceFunc < Test::Unit::TestCase
     foo
     trace.disable
     assert_equal([:foo, :foo], ary)
+
+    trace = TracePoint.new{}
+    trace.enable{
+      assert_equal(true, trace.disable)
+      assert_equal(false, trace.disable)
+      trace.disable{}
+      assert_equal(false, trace.disable)
+    }
   end
 
   def test_tracepoint_enabled
diff --git a/vm_trace.c b/vm_trace.c
index bedf41a9b6..d4c31ec941 100644
--- a/vm_trace.c
+++ b/vm_trace.c
@@ -952,20 +952,23 @@ rb_tracepoint_disable(VALUE tpval)
 
 /*
  * call-seq:
- *	trace.enable		-> trace
+ *	trace.enable		-> true or false
  *	trace.enable { block }	-> obj
  *
  * Activates the trace
  *
- * Will raise a RuntimeError if the trace is already activated
+ * Return true if trace was enabled.
+ * Return false if trace was disabled.
  *
  *	trace.enabled?  #=> false
- *	trace.enable    #=> #<TracePoint:0x007fa3fad4aaa8>
+ *	trace.enable    #=> false (previous state)
+ *                      #   trace is enabled
  *	trace.enabled?  #=> true
- *	trace.enable    #=> RuntimeError
+ *	trace.enable    #=> true (previous state)
+ *                      #   trace is still enabled
  *
  * If a block is given, the trace will only be enabled within the scope of the
- * block. Note: You cannot access event hooks within the block.
+ * block.
  *
  *	trace.enabled?
  *	#=> false
@@ -978,6 +981,8 @@ rb_tracepoint_disable(VALUE tpval)
  *	trace.enabled?
  *	#=> false
  *
+ * Note: You cannot access event hooks within the block.
+ *
  *	trace.enable { p tp.lineno }
  *	#=> RuntimeError: access from outside
  *
@@ -986,36 +991,36 @@ static VALUE
 tracepoint_enable_m(VALUE tpval)
 {
     rb_tp_t *tp = tpptr(tpval);
-
-    if (tp->tracing) {
-	rb_raise(rb_eRuntimeError, "trace is already enable");
-    }
-
+    int previous_tracing = tp->tracing;
     rb_tracepoint_enable(tpval);
+
     if (rb_block_given_p()) {
-	return rb_ensure(rb_yield, Qnil, rb_tracepoint_disable, tpval);
+	return rb_ensure(rb_yield, Qnil,
+			 previous_tracing ? rb_tracepoint_enable : rb_tracepoint_disable,
+			 tpval);
     }
     else {
-	return tpval;
+	return previous_tracing ? Qtrue : Qfalse;
     }
 }
 
 /*
  * call-seq:
- *	trace.disable		-> trace
+ *	trace.disable		-> tru eo rfalse
  *	trace.disable { block } -> obj
  *
  * Deactivates the trace
  *
- * Will raise a RuntimeError if the trace is already deactivated
+ * Return true if trace was enabled.
+ * Return false if trace was disabled.
  *
  *	trace.enabled?	#=> true
- *	trace.disable	#=> #<TracePoint:0x007fa3fad4aaa8>
+ *	trace.disable	#=> false (previous status)
  *	trace.enabled?	#=> false
- *	trace.disable	#=> RuntimeError
+ *	trace.disable	#=> false
  *
  * If a block is given, the trace will only be disable within the scope of the
- * block. Note: You cannot access event hooks within the block.
+ * block.
  *
  *	trace.enabled?
  *	#=> true
@@ -1028,25 +1033,25 @@ tracepoint_enable_m(VALUE tpval)
  *	trace.enabled?
  *	#=> true
  *
- *	trace.enable { p trace.lineno }
- *	#=> RuntimeError: access from outside
+ * Note: You cannot access event hooks within the block.
  *
+ *	trace.disable { p tp.lineno }
+ *	#=> RuntimeError: access from outside
  */
 static VALUE
 tracepoint_disable_m(VALUE tpval)
 {
     rb_tp_t *tp = tpptr(tpval);
-
-    if (!tp->tracing) {
-	rb_raise(rb_eRuntimeError, "trace is not enable");
-    }
-
+    int previous_tracing = tp->tracing;
     rb_tracepoint_disable(tpval);
+
     if (rb_block_given_p()) {
-	return rb_ensure(rb_yield, Qnil, rb_tracepoint_enable, tpval);
+	return rb_ensure(rb_yield, Qnil,
+			 previous_tracing ? rb_tracepoint_enable : rb_tracepoint_disable,
+			 tpval);
     }
     else {
-	return tpval;
+	return previous_tracing ? Qtrue : Qfalse;
     }
 }