From 14bd875d702dd6e2a16a59100418e91906bd7724 Mon Sep 17 00:00:00 2001 From: leviongit Date: Fri, 12 Apr 2024 17:26:12 +0200 Subject: [PATCH 1/5] fix `Array#delete` reimplement `Array#delete` in c, fixing `ary.delete(nil, &blk)` firing the block regardless of removal minimal reproduction: ```rb ary = [nil] ret = ary.delete(nil) { "not deleted?" } ``` --- mrblib/array.rb | 15 -------------- src/array.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+), 15 deletions(-) diff --git a/mrblib/array.rb b/mrblib/array.rb index 6c2b6cfee..da51437f1 100644 --- a/mrblib/array.rb +++ b/mrblib/array.rb @@ -184,21 +184,6 @@ class Array end end - ## - # call-seq: - # array.delete(obj) -> deleted_object - # array.delete(obj) {|nosuch| ... } -> deleted_object or block_return - # - # Delete element with index +key+ - def delete(key, &block) - while i = self.index(key) - self.delete_at(i) - ret = key - end - return block.call if ret.nil? && block - ret - end - ## # call-seq: # array.sort! -> self diff --git a/src/array.c b/src/array.c index 7e2d03c65..c2920a03a 100644 --- a/src/array.c +++ b/src/array.c @@ -1397,6 +1397,59 @@ mrb_ary_svalue(mrb_state *mrb, mrb_value ary) } } +/* + * call-seq: + * array.delete(obj) -> deleted_object + * array.delete(obj) {|nosuch| ... } -> deleted_object or block_return + * + * Delete element with index +key+ + */ + +static mrb_value +mrb_ary_delete(mrb_state *mrb, mrb_value self) +{ + struct RArray *ary = RARRAY(self); + mrb_value *val_ptr = ARY_PTR(ary); + size_t len = ARY_LEN(ary); + mrb_bool modified = FALSE; + + mrb_value obj, blk; + mrb_get_args(mrb, "o&", &obj, &blk); + mrb_value ret = obj; + + size_t i = 0; + size_t j = 0; + for (; i < len; ++i) { + mrb_value elem = val_ptr[i]; + + if (mrb_equal(mrb, obj, elem)) { + ret = elem; + continue; + } + + if (i != j) { + if (!modified) { + ary_modify(mrb, ary); + modified = TRUE; + } + val_ptr[j] = elem; + } + + ++j; + } + + if (i == j) { + if (mrb_proc_p(blk)) { + return mrb_yield_argv(mrb, blk, 1, &obj); + } + return mrb_nil_value(); + } + + ARY_SET_LEN(ary, j); + + return ret; +} + void mrb_init_array(mrb_state *mrb) { @@ -1414,6 +1467,7 @@ mrb_init_array(mrb_state *mrb) mrb_define_method_id(mrb, a, MRB_OPSYM(aset), mrb_ary_aset, MRB_ARGS_ARG(2,1)); /* 15.2.12.5.5 */ mrb_define_method_id(mrb, a, MRB_SYM(clear), mrb_ary_clear_m, MRB_ARGS_NONE()); /* 15.2.12.5.6 */ mrb_define_method_id(mrb, a, MRB_SYM(concat), mrb_ary_concat_m, MRB_ARGS_REQ(1)); /* 15.2.12.5.8 */ + mrb_define_method_id(mrb, a, MRB_SYM(delete), mrb_ary_delete, MRB_ARGS_REQ(1)); mrb_define_method_id(mrb, a, MRB_SYM(delete_at), mrb_ary_delete_at, MRB_ARGS_REQ(1)); /* 15.2.12.5.9 */ mrb_define_method_id(mrb, a, MRB_SYM_Q(empty), mrb_ary_empty_p, MRB_ARGS_NONE()); /* 15.2.12.5.12 */ mrb_define_method_id(mrb, a, MRB_SYM(first), mrb_ary_first, MRB_ARGS_OPT(1)); /* 15.2.12.5.13 */ From d46e9a9480c0cdbe32dcc1f0550cb528d21f6370 Mon Sep 17 00:00:00 2001 From: leviongit Date: Fri, 12 Apr 2024 18:12:51 +0200 Subject: [PATCH 2/5] fix shared array mistake --- src/array.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/array.c b/src/array.c index c2920a03a..8297e8aa6 100644 --- a/src/array.c +++ b/src/array.c @@ -1422,7 +1422,7 @@ mrb_ary_delete(mrb_state *mrb, mrb_value self) for (; i < len; ++i) { mrb_value elem = val_ptr[i]; - if (mrb_equal(mrb, obj, elem)) { + if (mrb_equal(mrb, elem, obj)) { ret = elem; continue; } @@ -1430,6 +1430,7 @@ mrb_ary_delete(mrb_state *mrb, mrb_value self) if (i != j) { if (!modified) { ary_modify(mrb, ary); + val_ptr = ARY_PTR(ary); modified = TRUE; } val_ptr[j] = elem; From c04afb9e99a01023178286b6fcc1fd9a7e607ff8 Mon Sep 17 00:00:00 2001 From: leviongit Date: Sat, 13 Apr 2024 06:52:37 +0200 Subject: [PATCH 3/5] revert moving `Array#delete` to c --- mrblib/array.rb | 15 ++++++++++++++ src/array.c | 55 ------------------------------------------------- 2 files changed, 15 insertions(+), 55 deletions(-) diff --git a/mrblib/array.rb b/mrblib/array.rb index da51437f1..6c2b6cfee 100644 --- a/mrblib/array.rb +++ b/mrblib/array.rb @@ -184,6 +184,21 @@ class Array end end + ## + # call-seq: + # array.delete(obj) -> deleted_object + # array.delete(obj) {|nosuch| ... } -> deleted_object or block_return + # + # Delete element with index +key+ + def delete(key, &block) + while i = self.index(key) + self.delete_at(i) + ret = key + end + return block.call if ret.nil? && block + ret + end + ## # call-seq: # array.sort! -> self diff --git a/src/array.c b/src/array.c index 8297e8aa6..7e2d03c65 100644 --- a/src/array.c +++ b/src/array.c @@ -1397,60 +1397,6 @@ mrb_ary_svalue(mrb_state *mrb, mrb_value ary) } } -/* - * call-seq: - * array.delete(obj) -> deleted_object - * array.delete(obj) {|nosuch| ... } -> deleted_object or block_return - * - * Delete element with index +key+ - */ - -static mrb_value -mrb_ary_delete(mrb_state *mrb, mrb_value self) -{ - struct RArray *ary = RARRAY(self); - mrb_value *val_ptr = ARY_PTR(ary); - size_t len = ARY_LEN(ary); - mrb_bool modified = FALSE; - - mrb_value obj, blk; - mrb_get_args(mrb, "o&", &obj, &blk); - mrb_value ret = obj; - - size_t i = 0; - size_t j = 0; - for (; i < len; ++i) { - mrb_value elem = val_ptr[i]; - - if (mrb_equal(mrb, elem, obj)) { - ret = elem; - continue; - } - - if (i != j) { - if (!modified) { - ary_modify(mrb, ary); - val_ptr = ARY_PTR(ary); - modified = TRUE; - } - val_ptr[j] = elem; - } - - ++j; - } - - if (i == j) { - if (mrb_proc_p(blk)) { - return mrb_yield_argv(mrb, blk, 1, &obj); - } - return mrb_nil_value(); - } - - ARY_SET_LEN(ary, j); - - return ret; -} - void mrb_init_array(mrb_state *mrb) { @@ -1468,7 +1414,6 @@ mrb_init_array(mrb_state *mrb) mrb_define_method_id(mrb, a, MRB_OPSYM(aset), mrb_ary_aset, MRB_ARGS_ARG(2,1)); /* 15.2.12.5.5 */ mrb_define_method_id(mrb, a, MRB_SYM(clear), mrb_ary_clear_m, MRB_ARGS_NONE()); /* 15.2.12.5.6 */ mrb_define_method_id(mrb, a, MRB_SYM(concat), mrb_ary_concat_m, MRB_ARGS_REQ(1)); /* 15.2.12.5.8 */ - mrb_define_method_id(mrb, a, MRB_SYM(delete), mrb_ary_delete, MRB_ARGS_REQ(1)); mrb_define_method_id(mrb, a, MRB_SYM(delete_at), mrb_ary_delete_at, MRB_ARGS_REQ(1)); /* 15.2.12.5.9 */ mrb_define_method_id(mrb, a, MRB_SYM_Q(empty), mrb_ary_empty_p, MRB_ARGS_NONE()); /* 15.2.12.5.12 */ mrb_define_method_id(mrb, a, MRB_SYM(first), mrb_ary_first, MRB_ARGS_OPT(1)); /* 15.2.12.5.13 */ From 312d49c59f2c91cbbbda2348fd126d004b9f11f0 Mon Sep 17 00:00:00 2001 From: leviongit Date: Sat, 13 Apr 2024 07:21:51 +0200 Subject: [PATCH 4/5] reimplement `Array#delete` in ruby --- mrblib/array.rb | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/mrblib/array.rb b/mrblib/array.rb index 6c2b6cfee..066f1d3a0 100644 --- a/mrblib/array.rb +++ b/mrblib/array.rb @@ -191,11 +191,32 @@ class Array # # Delete element with index +key+ def delete(key, &block) - while i = self.index(key) - self.delete_at(i) - ret = key + ret = key + i = 0 + j = 0 + len = self.length + while i < len + elem = self[i] + + if key == elem + ret = elem + i += 1 + next + end + + self[j] = elem if i != j + + i += 1 + j += 1 end - return block.call if ret.nil? && block + + if i == j + return block.call if block + return nil + end + + self.replace(self[0...j]) + ret end From bb78c2cbc9698541857c8e85a57fd1583d070ae7 Mon Sep 17 00:00:00 2001 From: leviongit Date: Sat, 13 Apr 2024 07:41:00 +0200 Subject: [PATCH 5/5] reimplement `Array#delete` with a helper method --- mrblib/array.rb | 26 ++------------------------ src/array.c | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 24 deletions(-) diff --git a/mrblib/array.rb b/mrblib/array.rb index 066f1d3a0..1ae20489e 100644 --- a/mrblib/array.rb +++ b/mrblib/array.rb @@ -191,31 +191,9 @@ class Array # # Delete element with index +key+ def delete(key, &block) - ret = key - i = 0 - j = 0 len = self.length - while i < len - elem = self[i] - - if key == elem - ret = elem - i += 1 - next - end - - self[j] = elem if i != j - - i += 1 - j += 1 - end - - if i == j - return block.call if block - return nil - end - - self.replace(self[0...j]) + ret = self.__delete(key) + return block.call() if len == self.length ret end diff --git a/src/array.c b/src/array.c index 7e2d03c65..cbea95dbe 100644 --- a/src/array.c +++ b/src/array.c @@ -1397,6 +1397,48 @@ mrb_ary_svalue(mrb_state *mrb, mrb_value ary) } } +static mrb_value +mrb_ary_delete(mrb_state *mrb, mrb_value self) +{ + struct RArray *ary = RARRAY(self); + mrb_value *val_ptr = ARY_PTR(ary); + size_t len = ARY_LEN(ary); + mrb_bool modified = FALSE; + + mrb_value obj = mrb_get_arg1(mrb); + mrb_value ret = obj; + + size_t i = 0; + size_t j = 0; + for (; i < len; ++i) { + mrb_value elem = val_ptr[i]; + + if (mrb_equal(mrb, elem, obj)) { + ret = elem; + continue; + } + + if (i != j) { + if (!modified) { + ary_modify(mrb, ary); + val_ptr = ARY_PTR(ary); + modified = TRUE; + } + val_ptr[j] = elem; + } + + ++j; + } + + if (i == j) { + return mrb_nil_value(); + } + + ARY_SET_LEN(ary, j); + + return ret; +} + void mrb_init_array(mrb_state *mrb) { @@ -1438,5 +1480,6 @@ mrb_init_array(mrb_state *mrb) mrb_define_method_id(mrb, a, MRB_SYM(__ary_eq), mrb_ary_eq, MRB_ARGS_REQ(1)); mrb_define_method_id(mrb, a, MRB_SYM(__ary_cmp), mrb_ary_cmp, MRB_ARGS_REQ(1)); mrb_define_method_id(mrb, a, MRB_SYM(__ary_index), mrb_ary_index_m, MRB_ARGS_REQ(1)); /* kept for mruby-array-ext */ + mrb_define_method_id(mrb, a, MRB_SYM(__delete), mrb_ary_delete, MRB_ARGS_REQ(1)); mrb_define_method_id(mrb, a, MRB_SYM(__svalue), mrb_ary_svalue, MRB_ARGS_NONE()); }