Ruby » ruby-trunk

Hi,

At Mon, 8 Dec 2008 18:23:04 +0900,
Roger Pack wrote in [ruby-core:20416]:
> This patch adds output to unrescued exceptions' output to the command line:

Rereading from scripts would have problems:
* they can be removed or changed
* -e and stdin are N/A
* slow.

Instead, isn't it enough only when debugging?

$ ./ruby -rtracer -e 'def foo;raise;end' -e foo
-e:1:in `foo': unhandled exception
		 foo
	from -e:2:in `<main>'


Index: error.c
===================================================================
--- error.c	(revision 20580)
+++ error.c	(working copy)
@@ -510,16 +510,31 @@ rb_check_backtrace(VALUE bt)
     long i;
     static const char err[] = "backtrace must be Array of String";
+    extern VALUE rb_cBacktrace;

     if (!NIL_P(bt)) {
-	int t = TYPE(bt);
-
-	if (t == T_STRING) return rb_ary_new3(1, bt);
-	if (t != T_ARRAY) {
+	if (IMMEDIATE_P(bt)) {
+	    rb_raise(rb_eTypeError, err);
+	}
+	switch (BUILTIN_TYPE(bt)) {
+	  case T_STRING:
+	    return rb_ary_new3(1, bt);
+	  case T_ARRAY:
+	    break;
+	  case T_STRUCT:
+	    if (CLASS_OF(bt) == rb_cBacktrace)
+		return rb_ary_new3(1, bt);
+	  default:
 	    rb_raise(rb_eTypeError, err);
 	}
 	for (i=0;i<RARRAY_LEN(bt);i++) {
-	    if (TYPE(RARRAY_PTR(bt)[i]) != T_STRING) {
-		rb_raise(rb_eTypeError, err);
+	    VALUE a = RARRAY_PTR(bt)[i];
+	    if (!IMMEDIATE_P(a)) {
+		switch (BUILTIN_TYPE(a)) {
+		  case T_STRING: continue;
+		  case T_STRUCT:
+		    if (CLASS_OF(a) == rb_cBacktrace) continue;
+		}
 	    }
+	    rb_raise(rb_eTypeError, err);
 	}
     }
Index: eval_error.c
===================================================================
--- eval_error.c	(revision 20580)
+++ eval_error.c	(working copy)
@@ -64,4 +64,25 @@ set_backtrace(VALUE info, VALUE bt)
 }

+static void 
+print_debug_line(VALUE *debug_lines, VALUE at)
+{
+    extern VALUE rb_cBacktrace;
+    VALUE hash = *debug_lines, lines, line, *p;
+
+    if (TYPE(at) != T_STRUCT || CLASS_OF(at) != rb_cBacktrace) return;
+    if (!hash) {
+	if (!rb_const_defined_at(rb_cObject, rb_intern("SCRIPT_LINES__"))) return;
+	hash = rb_const_get_at(rb_cObject, rb_intern("SCRIPT_LINES__"));
+	if (TYPE(hash) != T_HASH) return;
+	*debug_lines = hash;
+    }
+    p = RSTRUCT_PTR(at);
+    lines = rb_hash_lookup(hash, p[0]);
+    if (NIL_P(lines)) return;
+    line = rb_ary_entry(lines, NUM2INT(p[1]));
+    if (NIL_P(line)) return;
+    warn_printf("\t\t %s", RSTRING_PTR(line));
+}
+
 static void
 error_print(void)
@@ -72,4 +93,5 @@ error_print(void)
     const char *einfo;
     long elen;
+    VALUE debug_lines = 0;

     if (NIL_P(errinfo))
@@ -99,4 +121,5 @@ error_print(void)
 	VALUE mesg = RARRAY_PTR(errat)[0];

+	mesg = rb_check_string_type(mesg);
 	if (NIL_P(mesg))
 	    error_pos();
@@ -121,4 +144,7 @@ error_print(void)
     if (eclass == rb_eRuntimeError && elen == 0) {
 	warn_print(": unhandled exception\n");
+        if (!NIL_P(errat)) {
+	    print_debug_line(&debug_lines, RARRAY_PTR(errat)[0]);
+	}
     }
     else {
@@ -166,6 +192,9 @@ error_print(void)

 	for (i = 1; i < len; i++) {
-	    if (TYPE(ptr[i]) == T_STRING) {
-		warn_printf("\tfrom %s\n", RSTRING_PTR(ptr[i]));
+	    VALUE a = ptr[i], s;
+	    s = rb_check_string_type(a);
+	    if (!NIL_P(s)) {
+		warn_printf("\tfrom %s\n", RSTRING_PTR(s));
+		print_debug_line(&debug_lines, a);
 	    }
 	    if (skip && i == TRACE_HEAD && len > TRACE_MAX) {
Index: vm.c
===================================================================
--- vm.c	(revision 20580)
+++ vm.c	(working copy)
@@ -33,4 +33,5 @@ VALUE rb_cThread;
 VALUE rb_cEnv;
 VALUE rb_mRubyVMFrozenCore;
+VALUE rb_cBacktrace;

 VALUE ruby_vm_global_state_version = 1;
@@ -629,4 +630,22 @@ rb_lastline_set(VALUE val)
 /* backtrace */

+static VALUE
+backtrace_to_str(VALUE self)
+{
+    VALUE *p = RSTRUCT_PTR(self);
+    VALUE file = p[0], line = p[1], name = p[2];
+    VALUE str = p[3];
+    if (NIL_P(str)) {
+	str = rb_sprintf("%s:%d:in `%s'",
+			 NIL_P(file) ? "" : StringValueCStr(file),
+			 NUM2INT(line), StringValueCStr(name));
+	if (!OBJ_FROZEN(self) &&
+	    (OBJ_UNTRUSTED(self) || rb_safe_level() < 4)) {
+	    p[3] = str;
+	}
+    }
+    return str;
+}
+
 int
 vm_get_sourceline(const rb_control_frame_t *cfp)
@@ -654,10 +673,10 @@ static VALUE
 vm_backtrace_each(rb_thread_t *th,
 		  const rb_control_frame_t *limit_cfp, const rb_control_frame_t *cfp,
-		  const char * file, int line_no, VALUE ary)
+		  VALUE ary)
 {
-    VALUE str;
+    VALUE bt, b[3];
+    int line_no;

     while (cfp > limit_cfp) {
-	str = 0;
 	if (cfp->iseq != 0) {
 	    if (cfp->pc != 0) {
@@ -665,15 +684,17 @@ vm_backtrace_each(rb_thread_t *th,

 		line_no = vm_get_sourceline(cfp);
-		file = RSTRING_PTR(iseq->filename);
-		str = rb_sprintf("%s:%d:in `%s'",
-				 file, line_no, RSTRING_PTR(iseq->name));
-		rb_ary_push(ary, str);
+		b[0] = iseq->filename;
+		b[1] = INT2NUM(line_no);
+		b[2] = iseq->name;
+		bt = rb_class_new_instance(sizeof(b) / sizeof(*b), b, rb_cBacktrace);
+		rb_ary_push(ary, bt);
 	    }
 	}
 	else if (RUBYVM_CFUNC_FRAME_P(cfp)) {
-	    str = rb_sprintf("%s:%d:in `%s'",
-			     file, line_no,
-			     rb_id2name(cfp->method_id));
-	    rb_ary_push(ary, str);
+	    b[0] = Qnil;
+	    b[1] = INT2FIX(0);
+	    b[2] = rb_id2str(cfp->method_id);
+	    bt = rb_class_new_instance(sizeof(b) / sizeof(*b), b, rb_cBacktrace);
+	    rb_ary_push(ary, bt);
 	}
 	cfp = RUBY_VM_NEXT_CONTROL_FRAME(cfp);
@@ -704,6 +725,5 @@ vm_backtrace(rb_thread_t *th, int lev)
     }

-    ary = vm_backtrace_each(th, RUBY_VM_NEXT_CONTROL_FRAME(cfp),
-			    top_of_cfp, "", 0, ary);
+    ary = vm_backtrace_each(th, RUBY_VM_NEXT_CONTROL_FRAME(cfp), top_of_cfp, ary);
     return ary;
 }
@@ -1847,4 +1867,9 @@ Init_VM(void)
     rb_define_const(rb_cRubyVM, "INSTRUCTION_NAMES", ruby_insns_name_array());

+    rb_cBacktrace = rb_struct_define((char*)0, "file", "line", "name", "str", (char*)0);
+    rb_define_const(rb_cRubyVM, "Backtrace", rb_cBacktrace);
+    rb_define_method(rb_cBacktrace, "to_str", backtrace_to_str, 0);
+    rb_define_method(rb_cBacktrace, "inspect", backtrace_to_str, 0);
+
     /* debug functions ::VM::SDR(), ::VM::NSDR() */
 #if VMDEBUG


-- 
Nobu Nakada

I'd love to try it out, but am having trouble patching TRUNK with it.  Could you give me a hint as to which revision it is a patch from?  Is it from trunk?
Thanks!
-=R

heh--I see it--revision 20850, from the diff.  That one works for applying it.

You're right--something that didn't require reading files would be nice.
I suppose the only drawback would be...shouldn't tracer output
>> enter x
<< leave x


and if so, wouldn't requiring users to include tracer possibly cause too much console output to be worth the added verbose output?

Here's an interesting related request:

I also wonder if someday something like this would be possible [1]
NoMethodError: undefined method `capitalize' for nil:NilClass
    from /home/pwilliams/projects/tmp/test.rb:18:in main.greet_user(nil, "williams")
    from /home/pwilliams/projects/tmp/test.rb:14:in main.third(nil, "williams")
    from /home/pwilliams/projects/tmp/test.rb:10:in main.second("williams, peter")
    from /home/pwilliams/projects/tmp/test.rb:4:in main.first("Peter Williams")
    from (irb):41
    from :0

I can think of a way to do this using set_trace_func style stuff to track parameters but that's about it...

I'll look at it more sometime.

Thoughts?
-=R

[1] http://barelyenough.org/blog/2005/04/ruby-backtraces/

How about a compromise to propagate SCRIPT_LINES__ after the fact with the appropriate files if they're not in it yet?  I'd be happy to do it, and it thus wouldn't result in a large slowdown [and if files have changed recently, the user will probably be quite aware of that fact].
Or maybe running it -r"something" would be sufficient.

Thoughts?
-=R

I suppose another option would be to only retrieve the line for the topmost line of the backtrace. Thoughts?

I like this patch (nobu's).  Would it be possible to get it applied?
-r

I still like this patch (applied to caller and/or Exception#backtrace).

Who needs to approve it...?

Thanks.
-r

Related previous requests:

http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/4822
http://barelyenough.org/blog/2005/04/ruby-backtraces
http://www.ruby-forum.com/topic/198847
http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/4822

Hi,

Matz, what do you think about this ticket and #1906?

There is a patch nobu has written (though I have not tested):
http://redmine.ruby-lang.org/issues/show/839#note-1

But it might take some time to make stable the feature, so please
let us know your opinion or decision as soon as possible.

-- 
Yusuke Endoh <mame@tsg.ne.jp>

Matz if you get a chance to look at this patch, it "objectify's" Exception#backtrace which might be nice.
Thank you.
-roger

Any feedback on this? (objectify Exception#backtrace et al)
Thanks.