https://bugs.ruby-lang.org/
https://bugs.ruby-lang.org/favicon.ico?1711330511
2014-12-20T00:50:10Z
Ruby Issue Tracking System
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50520
2014-12-20T00:50:10Z
tmm1 (Aman Karmani)
ruby@tmm1.net
<ul></ul><p>This regression was introduced in r48114 to fix <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Unexpected block invocation with unknown keywords (Closed)" href="https://bugs.ruby-lang.org/issues/10413">#10413</a></p>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50521
2014-12-20T01:29:22Z
tmm1 (Aman Karmani)
ruby@tmm1.net
<ul></ul><p>Proposed patch: <a href="https://github.com/github/ruby/commit/a2b6dadbabf935ec9126c8bd9210fbb133b55c84.diff" class="external">https://github.com/github/ruby/commit/a2b6dadbabf935ec9126c8bd9210fbb133b55c84.diff</a></p>
<p>/cc <a class="user active user-mention" href="https://bugs.ruby-lang.org/users/4">@nobu (Nobuyoshi Nakada)</a></p>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50524
2014-12-20T08:11:25Z
nobu (Nobuyoshi Nakada)
nobu@ruby-lang.org
<ul></ul><p>I think a function can tell no keys from deleting <code>nil</code> value, like <code>hash_delete</code> in your patch, is necessary.</p>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50578
2014-12-23T05:28:46Z
tmm1 (Aman Karmani)
ruby@tmm1.net
<ul></ul><p><a class="user active user-mention" href="https://bugs.ruby-lang.org/users/4">@nobu (Nobuyoshi Nakada)</a> Do you think we need to change <code>rb_hash_delete</code> behavior? Or do you mean we should add another public c-api function for the undef behavior, like <code>rb_hash_delete_xxx</code></p>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50580
2014-12-23T08:57:32Z
ko1 (Koichi Sasada)
<ul></ul><p>At Comment#3, Nobu said that we still need a function which returns no such entry.</p>
<p>We have three possibility:</p>
<p>(a) reutrn Qundef when no entry (current rb_hash_delete())<br>
(b) return nil when no entry<br>
(c) return passed block results or nil when no entry (previous rb_hash_delete())</p>
<p>So my recommendation is:</p>
<p>(1) Implement (a) with other name (ex: rb_hash_delete_key())<br>
(2) Implement (b) named rb_hash_delete() using (1)<br>
(3) Implement (c) for Hash#delete with (1).</p>
<p><a class="user active user-mention" href="https://bugs.ruby-lang.org/users/390">@tmm1 (Aman Karmani)</a>: is it okay?</p>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50599
2014-12-24T02:31:16Z
ko1 (Koichi Sasada)
<ul></ul><p>How about it?</p>
<p>rb_hash_delete_entry() for (1).</p>
<pre><code class="diff syntaxhl" data-language="diff"><span class="gh">Index: ChangeLog
===================================================================
</span><span class="gd">--- ChangeLog (revision 48955)
</span><span class="gi">+++ ChangeLog (working copy)
</span><span class="p">@@ -1,3 +1,19 @@</span>
<span class="gi">+Wed Dec 24 11:24:03 2014 Koichi Sasada <ko1@atdot.net>
+
+ * hash.c (rb_hash_delete): return Qnil if there are no corresponding
+ entry. [Bug #10623]
+
+ * hash.c (rb_hash_delete_entry): try delete and return Qundef if there
+ are no corresponding entry.
+
+ * internal.h: add rb_hash_delete_entry()'s declaration.
+
+ * symbol.c: use rb_hash_delete_entry().
+
+ * thread.c: use rb_hash_delete_entry().
+
+ * ext/-test-/hash/delete.c: use rb_hash_delete_entry().
+
</span> Wed Dec 24 09:35:11 2014 NAKAMURA Usaku <usa@ruby-lang.org>
* ext/fiddle/extconf.rb: remove ffitarget.h generated by configure on
<span class="gh">Index: ext/-test-/hash/delete.c
===================================================================
</span><span class="gd">--- ext/-test-/hash/delete.c (revision 48955)
</span><span class="gi">+++ ext/-test-/hash/delete.c (working copy)
</span><span class="p">@@ -3,7 +3,7 @@</span>
static VALUE
hash_delete(VALUE hash, VALUE key)
{
<span class="gd">- VALUE ret = rb_hash_delete(hash, key);
</span><span class="gi">+ VALUE ret = rb_hash_delete_entry(hash, key);
</span> return ret == Qundef ? Qnil : rb_ary_new_from_values(1, &ret);
}
<span class="gh">Index: hash.c
===================================================================
</span><span class="gd">--- hash.c (revision 48955)
</span><span class="gi">+++ hash.c (working copy)
</span><span class="p">@@ -969,23 +969,49 @@</span> rb_hash_index(VALUE hash, VALUE value)
return rb_hash_key(hash, value);
}
<span class="gi">+/*
+ * delete a specified entry a given key.
+ * if there is the corresponding entry, return a value of the entry.
+ * if there is no corresponding entry, return Qundef.
+ */
</span> VALUE
<span class="gd">-rb_hash_delete(VALUE hash, VALUE key)
</span><span class="gi">+rb_hash_delete_entry(VALUE hash, VALUE key)
</span> {
st_data_t ktmp = (st_data_t)key, val;
<span class="gd">- if (!RHASH(hash)->ntbl)
</span><span class="gi">+ if (!RHASH(hash)->ntbl) {
</span> return Qundef;
<span class="gd">- if (RHASH_ITER_LEV(hash) > 0) {
- if (st_delete_safe(RHASH(hash)->ntbl, &ktmp, &val, (st_data_t)Qundef)) {
</span><span class="gi">+ }
+ else if (RHASH_ITER_LEV(hash) > 0 &&
+ (st_delete_safe(RHASH(hash)->ntbl, &ktmp, &val, (st_data_t)Qundef))) {
</span> FL_SET(hash, HASH_DELETED);
return (VALUE)val;
}
<span class="gd">- }
- else if (st_delete(RHASH(hash)->ntbl, &ktmp, &val))
</span><span class="gi">+ else if (st_delete(RHASH(hash)->ntbl, &ktmp, &val)) {
</span> return (VALUE)val;
<span class="gi">+ }
+ else {
</span> return Qundef;
}
<span class="gi">+}
+
+/*
+ * delete a specified entry by a given key.
+ * if there is the corresponding entry, return a value of the entry.
+ * if there is no corresponding entry, return Qnil.
+ */
+VALUE
+rb_hash_delete(VALUE hash, VALUE key)
+{
+ VALUE deleted_value = rb_hash_delete_entry(hash, key);
+
+ if (deleted_value != Qundef) { /* likely pass */
+ return deleted_value;
+ }
+ else {
+ return Qnil;
+ }
+}
</span>
/*
* call-seq:
<span class="p">@@ -1011,13 +1037,20 @@</span> rb_hash_delete_m(VALUE hash, VALUE key)
VALUE val;
rb_hash_modify_check(hash);
<span class="gd">- val = rb_hash_delete(hash, key);
- if (val != Qundef) return val;
</span><span class="gi">+ val = rb_hash_delete_entry(hash, key);
+
+ if (val != Qundef) {
+ return val;
+ }
+ else {
</span> if (rb_block_given_p()) {
return rb_yield(key);
}
<span class="gi">+ else {
</span> return Qnil;
}
<span class="gi">+ }
+}
</span>
struct shift_var {
VALUE key;
<span class="p">@@ -1063,7 +1096,7 @@</span> rb_hash_shift(VALUE hash)
else {
rb_hash_foreach(hash, shift_i_safe, (VALUE)&var);
if (var.key != Qundef) {
<span class="gd">- rb_hash_delete(hash, var.key);
</span><span class="gi">+ rb_hash_delete_entry(hash, var.key);
</span> return rb_assoc_new(var.key, var.val);
}
}
<span class="gh">Index: internal.h
===================================================================
</span><span class="gd">--- internal.h (revision 48955)
</span><span class="gi">+++ internal.h (working copy)
</span><span class="p">@@ -1119,6 +1119,9 @@</span> int rb_bug_reporter_add(void (*func)(FIL
VALUE rb_str_normalize_ospath(const char *ptr, long len);
#endif
<span class="gi">+/* hash.c (export) */
+VALUE rb_hash_delete_entry(VALUE hash, VALUE key);
+
</span> /* io.c (export) */
void rb_maygvl_fd_fix_cloexec(int fd);
<span class="gh">Index: symbol.c
===================================================================
</span><span class="gd">--- symbol.c (revision 48955)
</span><span class="gi">+++ symbol.c (working copy)
</span><span class="p">@@ -740,7 +740,7 @@</span> rb_sym2id(VALUE sym)
RSYMBOL(sym)->id = id |= num;
/* make it permanent object */
set_id_entry(num >>= ID_SCOPE_SHIFT, fstr, sym);
<span class="gd">- rb_hash_delete(global_symbols.dsymbol_fstr_hash, fstr);
</span><span class="gi">+ rb_hash_delete_entry(global_symbols.dsymbol_fstr_hash, fstr);
</span> }
}
else {
<span class="gh">Index: thread.c
===================================================================
</span><span class="gd">--- thread.c (revision 48955)
</span><span class="gi">+++ thread.c (working copy)
</span><span class="p">@@ -4806,13 +4806,13 @@</span> recursive_pop(VALUE list, VALUE obj, VAL
return 0;
}
if (RB_TYPE_P(pair_list, T_HASH)) {
<span class="gd">- rb_hash_delete(pair_list, paired_obj);
</span><span class="gi">+ rb_hash_delete_entry(pair_list, paired_obj);
</span> if (!RHASH_EMPTY_P(pair_list)) {
return 1; /* keep hash until is empty */
}
}
}
<span class="gd">- rb_hash_delete(list, obj);
</span><span class="gi">+ rb_hash_delete_entry(list, obj);
</span> return 1;
}
</code></pre>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50600
2014-12-24T02:53:56Z
ko1 (Koichi Sasada)
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Closed</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Applied in changeset r48958.</p>
<hr>
<ul>
<li>hash.c (rb_hash_delete): return Qnil if there are no corresponding<br>
entry. [Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: rb_hash_delete() can return Qundef in 2.2-rc1 (Closed)" href="https://bugs.ruby-lang.org/issues/10623">#10623</a>]</li>
<li>hash.c (rb_hash_delete_entry): try delete and return Qundef if there<br>
are no corresponding entry.</li>
<li>internal.h: add rb_hash_delete_entry()'s declaration.</li>
<li>symbol.c: use rb_hash_delete_entry().</li>
<li>thread.c: use rb_hash_delete_entry().</li>
<li>ext/-test-/hash/delete.c: use rb_hash_delete_entry().</li>
</ul>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50603
2014-12-24T03:09:53Z
nagachika (Tomoyuki Chikanaga)
nagachika00@gmail.com
<ul><li><strong>Backport</strong> changed from <i>2.0.0: UNKNOWN, 2.1: UNKNOWN</i> to <i>2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: REQUIRED</i></li></ul>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50604
2014-12-24T03:14:20Z
nagachika (Tomoyuki Chikanaga)
nagachika00@gmail.com
<ul><li><strong>Backport</strong> changed from <i>2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: REQUIRED</i> to <i>2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: REQUIRED</i></li></ul><p>I didn't confirm it by myyself, but r48114 was already backported into ruby_2_1/ruby_2_0_0. So this should be backported into them too.</p>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50638
2014-12-26T06:39:46Z
naruse (Yui NARUSE)
naruse@airemix.jp
<ul><li><strong>Backport</strong> changed from <i>2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: REQUIRED</i> to <i>2.0.0: REQUIRED, 2.1: REQUIRED, 2.2: DONE</i></li></ul>
Ruby master - Bug #10623: rb_hash_delete() can return Qundef in 2.2-rc1
https://bugs.ruby-lang.org/issues/10623?journal_id=50996
2015-01-14T07:37:55Z
usa (Usaku NAKAMURA)
usa@garbagecollect.jp
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/10413">Bug #10413</a>: Unexpected block invocation with unknown keywords</i> added</li></ul>