ruby-changes:36526
From: ko1 <ko1@a...>
Date: Thu, 27 Nov 2014 19:16:02 +0900 (JST)
Subject: [ruby-changes:36526] ko1:r48608 (trunk): * vm_args.c: fix backtrace location for keyword related exceptions.
ko1 2014-11-27 19:15:47 +0900 (Thu, 27 Nov 2014) New Revision: 48608 http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=48608 Log: * vm_args.c: fix backtrace location for keyword related exceptions. For example, the following program def foo(k1: 1); end # line 1 foo(k2: 2) # line 2 causes "unknown keyword: k2 (ArgumentError)". Before this patch, the backtrace location is only line 2. However, error should be located at line 1 (over line 2 in stack trace). This patch fix this problem. * class.c (rb_keyword_error_new): separate exception creation logic from rb_keyword_error(), to use in vm_args.c. * vm_insnhelper.c (rb_arg_error_new): rename to rb_arity_error_new(). * vm_args.c (argument_arity_error): rename to argument_arity_error(). * vm_args.c (arugment_kw_error): added to fix backtrace. * test/ruby/test_keyword.rb: add tests. Modified files: trunk/ChangeLog trunk/class.c trunk/test/ruby/test_keyword.rb trunk/vm_args.c trunk/vm_insnhelper.c Index: ChangeLog =================================================================== --- ChangeLog (revision 48607) +++ ChangeLog (revision 48608) @@ -1,3 +1,27 @@ https://github.com/ruby/ruby/blob/trunk/ChangeLog#L1 +Thu Nov 27 19:04:50 2014 Koichi Sasada <ko1@a...> + + * vm_args.c: fix backtrace location for keyword related exceptions. + + For example, the following program + def foo(k1: 1); end # line 1 + foo(k2: 2) # line 2 + causes "unknown keyword: k2 (ArgumentError)". + + Before this patch, the backtrace location is only line 2. + However, error should be located at line 1 (over line 2 in + stack trace). This patch fix this problem. + + * class.c (rb_keyword_error_new): separate exception creation logic + from rb_keyword_error(), to use in vm_args.c. + + * vm_insnhelper.c (rb_arg_error_new): rename to rb_arity_error_new(). + + * vm_args.c (argument_arity_error): rename to argument_arity_error(). + + * vm_args.c (arugment_kw_error): added to fix backtrace. + + * test/ruby/test_keyword.rb: add tests. + Thu Nov 27 17:31:58 2014 Nobuyoshi Nakada <nobu@r...> * common.mk (prelude.c): no longer depends on miniruby, since not Index: class.c =================================================================== --- class.c (revision 48607) +++ class.c (revision 48608) @@ -1857,11 +1857,12 @@ rb_scan_args(int argc, const VALUE *argv https://github.com/ruby/ruby/blob/trunk/class.c#L1857 return argc; } -NORETURN(void rb_keyword_error(const char *error, VALUE keys)); -void -rb_keyword_error(const char *error, VALUE keys) +VALUE +rb_keyword_error_new(const char *error, VALUE keys) { const char *msg = ""; + VALUE error_message; + if (RARRAY_LEN(keys) == 1) { keys = RARRAY_AREF(keys, 0); } @@ -1869,7 +1870,17 @@ rb_keyword_error(const char *error, VALU https://github.com/ruby/ruby/blob/trunk/class.c#L1870 keys = rb_ary_join(keys, rb_usascii_str_new2(", ")); msg = "s"; } - rb_raise(rb_eArgError, "%s keyword%s: %"PRIsVALUE, error, msg, keys); + + error_message = rb_sprintf("%s keyword%s: %"PRIsVALUE, error, msg, keys); + + return rb_exc_new_str(rb_eArgError, error_message); +} + +NORETURN(static void rb_keyword_error(const char *error, VALUE keys)); +static void +rb_keyword_error(const char *error, VALUE keys) +{ + rb_exc_raise(rb_keyword_error_new(error, keys)); } NORETURN(static void unknown_keyword_error(VALUE hash, const ID *table, int keywords)); Index: vm_insnhelper.c =================================================================== --- vm_insnhelper.c (revision 48607) +++ vm_insnhelper.c (revision 48608) @@ -115,7 +115,7 @@ vm_pop_frame(rb_thread_t *th) https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L115 /* method dispatch */ static inline VALUE -rb_arg_error_new(int argc, int min, int max) +rb_arity_error_new(int argc, int min, int max) { VALUE err_mess = 0; if (min == max) { @@ -133,7 +133,7 @@ rb_arg_error_new(int argc, int min, int https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L133 void rb_error_arity(int argc, int min, int max) { - rb_exc_raise(rb_arg_error_new(argc, min, max)); + rb_exc_raise(rb_arity_error_new(argc, min, max)); } /* svar */ @@ -1064,7 +1064,7 @@ vm_callee_setup_block_arg(rb_thread_t *t https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1064 ci->argc = vm_callee_setup_block_arg_arg0_splat(cfp, iseq, argv, arg0); } else { - argument_error(iseq, ci->argc, iseq->param.lead_num, iseq->param.lead_num); + argument_arity_error(th, iseq, ci->argc, iseq->param.lead_num, iseq->param.lead_num); } } @@ -1084,7 +1084,7 @@ vm_callee_setup_arg(rb_thread_t *th, rb_ https://github.com/ruby/ruby/blob/trunk/vm_insnhelper.c#L1084 CALLER_SETUP_ARG(cfp, ci); /* splat arg */ if (ci->argc != iseq->param.lead_num) { - argument_error(iseq, ci->argc, iseq->param.lead_num, iseq->param.lead_num); + argument_arity_error(th, iseq, ci->argc, iseq->param.lead_num, iseq->param.lead_num); } ci->aux.opt_pc = 0; Index: test/ruby/test_keyword.rb =================================================================== --- test/ruby/test_keyword.rb (revision 48607) +++ test/ruby/test_keyword.rb (revision 48608) @@ -310,11 +310,16 @@ class TestKeywordArguments < Test::Unit: https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L310 feature7701 = '[ruby-core:51454] [Feature #7701] required keyword argument' o = Object.new assert_nothing_raised(SyntaxError, feature7701) do - eval("def o.foo(a:) a; end") + eval("def o.foo(a:) a; end", nil, "xyzzy") eval("def o.bar(a:,**b) [a, b]; end") end assert_raise_with_message(ArgumentError, /missing keyword/, feature7701) {o.foo} assert_raise_with_message(ArgumentError, /unknown keyword/, feature7701) {o.foo(a:0, b:1)} + begin + o.foo(a: 0, b: 1) + rescue => e + assert_equal('xyzzy', e.backtrace_locations[0].path) + end assert_equal(42, o.foo(a: 42), feature7701) assert_equal([[:keyreq, :a]], o.method(:foo).parameters, feature7701) @@ -362,10 +367,16 @@ class TestKeywordArguments < Test::Unit: https://github.com/ruby/ruby/blob/trunk/test/ruby/test_keyword.rb#L367 def test_block_required_keyword feature7701 = '[ruby-core:51454] [Feature #7701] required keyword argument' b = assert_nothing_raised(SyntaxError, feature7701) do - break eval("proc {|a:| a}", nil, __FILE__, __LINE__) + break eval("proc {|a:| a}", nil, 'xyzzy', __LINE__) end assert_raise_with_message(ArgumentError, /missing keyword/, feature7701) {b.call} assert_raise_with_message(ArgumentError, /unknown keyword/, feature7701) {b.call(a:0, b:1)} + begin + b.call(a: 0, b: 1) + rescue => e + assert_equal('xyzzy', e.backtrace_locations[0].path) + end + assert_equal(42, b.call(a: 42), feature7701) assert_equal([[:keyreq, :a]], b.parameters, feature7701) Index: vm_args.c =================================================================== --- vm_args.c (revision 48607) +++ vm_args.c (revision 48608) @@ -8,6 +8,11 @@ https://github.com/ruby/ruby/blob/trunk/vm_args.c#L8 **********************************************************************/ +NORETURN(static void raise_argument_error(rb_thread_t *th, const rb_iseq_t *iseq, const VALUE exc)); +NORETURN(static void argument_arity_error(rb_thread_t *th, const rb_iseq_t *iseq, const int miss_argc, const int min_argc, const int max_argc)); +NORETURN(static void arugment_kw_error(rb_thread_t *th, const rb_iseq_t *iseq, const char *error, const VALUE keys)); +VALUE rb_keyword_error_new(const char *error, VALUE keys); /* class.c */ + struct args_info { /* basic args info */ rb_call_info_t *ci; @@ -359,8 +364,6 @@ make_unused_kw_hash(const ID *passed_key https://github.com/ruby/ruby/blob/trunk/vm_args.c#L364 return obj; } -void rb_keyword_error(const char *error, VALUE keys); - static inline int args_setup_kw_parameters_lookup(const ID key, VALUE *ptr, const ID * const passed_keywords, VALUE *passed_values, const int passed_keyword_len) { @@ -401,7 +404,7 @@ args_setup_kw_parameters(VALUE* const pa https://github.com/ruby/ruby/blob/trunk/vm_args.c#L404 } } - if (missing) rb_keyword_error("missing", missing); + if (missing) arugment_kw_error(GET_THREAD(), iseq, "missing", missing); for (di=0; i<key_num; i++, di++) { if (args_setup_kw_parameters_lookup(acceptable_keywords[i], &locals[i], passed_keywords, passed_values, passed_keyword_len)) { @@ -442,7 +445,7 @@ args_setup_kw_parameters(VALUE* const pa https://github.com/ruby/ruby/blob/trunk/vm_args.c#L445 else { if (found != passed_keyword_len) { VALUE keys = make_unused_kw_hash(passed_keywords, passed_keyword_len, passed_values, TRUE); - rb_keyword_error("unknown", keys); + arugment_kw_error(GET_THREAD(), iseq, "unknown", keys); } } @@ -495,29 +498,6 @@ fill_keys_values(st_data_t key, st_data_ https://github.com/ruby/ruby/blob/trunk/vm_args.c#L498 return ST_CONTINUE; } -NORETURN(static void argument_error(const rb_iseq_t *iseq, int miss_argc, int min_argc, int max_argc)); -static void -argument_error(const rb_iseq_t *iseq, int miss_argc, int min_argc, int max_argc) -{ - rb_thread_t *th = GET_THREAD(); - VALUE exc = rb_arg_error_new(miss_argc, min_argc, max_argc); - VALUE at; - - if (iseq) { - vm_push_frame(th, iseq, VM_FRAME_MAGIC_METHOD, Qnil /* self */, Qnil /* klass */, Qnil /* specval*/, - iseq->iseq_encoded, th->cfp->sp, 0 /* local_size */, 0 /* me */, 0 /* stack_max */); - at = rb_vm_backtrace_object(); - vm_pop_frame(th); - } - else { - at = rb_vm_backtrace_object(); - } - - rb_iv_set(exc, "bt_locations", at); - rb_funcall(exc, rb_intern("set_backtrace"), 1, at); - rb_exc_raise(exc); -} - static int setup_parameters_complex(rb_thread_t * const th, const rb_iseq_t * const iseq, rb_call_info_t * const ci, VALUE * const locals, const enum arg_setup_type arg_setup_type) @@ -599,7 +579,7 @@ setup_parameters_complex(rb_thread_t * c https://github.com/ruby/ruby/blob/trunk/vm_args.c#L579 args_extend(args, min_argc); } else { - argument_error(iseq, given_argc, min_argc, max_argc); + argument_arity_error(th, iseq, given_argc, min_argc, max_argc); } } } @@ -619,7 +599,7 @@ setup_parameters_complex(rb_thread_t * c https://github.com/ruby/ruby/blob/trunk/vm_args.c#L599 given_argc = max_argc; } else { - argument_error(iseq, given_argc, min_argc, max_argc); + argument_arity_error(th, iseq, given_argc, min_argc, max_argc); } } @@ -684,6 +664,38 @@ setup_parameters_complex(rb_thread_t * c https://github.com/ruby/ruby/blob/trunk/vm_args.c#L664 return opt_pc; } +static void +raise_argument_error(rb_thread_t *th, const rb_iseq_t *iseq, const VALUE exc) +{ + VALUE at; + + if (iseq) { + vm_push_frame(th, iseq, VM_FRAME_MAGIC_METHOD, Qnil /* self */, Qnil /* klass */, Qnil /* specval*/, + iseq->iseq_encoded, th->cfp->sp, 0 /* local_size */, 0 /* me */, 0 /* stack_max */); + at = rb_vm_backtrace_object(); + vm_pop_frame(th); + } + else { + at = rb_vm_backtrace_object(); + } + + rb_iv_set(exc, "bt_locations", at); + rb_funcall(exc, rb_intern("set_backtrace"), 1, at); + rb_exc_raise(exc); +} + +static void +argument_arity_error(rb_thread_t *th, const rb_iseq_t *iseq, const int miss_argc, const int min_argc, const int max_argc) +{ + raise_argument_error(th, iseq, rb_arity_error_new(miss_argc, min_argc, max_argc)); +} + +static void +arugment_kw_error(rb_thread_t *th, const rb_iseq_t *iseq, const char *error, const VALUE keys) +{ + raise_argument_error(th, iseq, rb_keyword_error_new(error, keys)); +} + static inline void vm_caller_setup_arg_splat(rb_control_frame_t *cfp, rb_call_info_t *ci) { -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/