https://redmine.ruby-lang.org/https://redmine.ruby-lang.org/favicon.ico?17113305112012-03-29T06:29:24ZRuby Issue Tracking SystemRuby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253202012-03-29T06:29:24Zaprescott (Adam Prescott)
<ul></ul><p>On Wed, Mar 28, 2012 at 22:18, Adam Prescott <a href="mailto:adam@aprescott.com" class="email">adam@aprescott.com</a> wrote:</p>
<blockquote>
<p>Assignment always returns the right hand side, so this isn't quite an<br>
accurate demonstration of #store.</p>
</blockquote>
<p>Although, I should also mention, this is due to the syntax of assignment<br>
here being also a method call, and not simply that #foo= will always return<br>
the argument it's given.</p>
<p>Or, since it's easier shown than described, what I mean is:</p>
<p>X.new.send(:foo=, 10) #=> :nope</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253232012-03-29T06:40:29ZMartinBosslet (Martin Bosslet)Martin.Bosslet@gmail.com
<ul></ul><p>Right, good point. OK, then let me rephrase it for explicitly calling store. What I would prefer is:</p>
<p>h = { a: 1 }<br>
h.store(:a, 2) # => 1<br>
h.store(:b, 3) # => nil</p>
<p>That way I can check for collisions in one pass without having to call has_key? first. Otherwise<br>
the result is evaluating the internal hash function twice, once for has_key? and once for the<br>
following call to store.</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253272012-03-29T07:24:42Zcjheath (Clifford Heath)clifford.heath@gmail.com
<ul></ul><p>On 29/03/2012, at 8:40 AM, MartinBosslet (Martin Bosslet) wrote:</p>
<blockquote>
<p>Right, good point. OK, then let me rephrase it for explicitly calling store.<br>
...<br>
That way I can check for collisions in one pass without having to call has_key? first.</p>
</blockquote>
<p>It's a nice thought, but about fifteen years too late - you can't just change<br>
fundamental things like this without introducing all sorts of subtle bugs into<br>
people's programs. Write a new method (e.g. put(x)) that has the behaviour<br>
you want.</p>
<p>Clifford Heath.</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253302012-03-29T08:09:54ZMartinBosslet (Martin Bosslet)Martin.Bosslet@gmail.com
<ul></ul><p>cjheath (Clifford Heath) wrote:</p>
<blockquote>
<p>On 29/03/2012, at 8:40 AM, MartinBosslet (Martin Bosslet) wrote:</p>
<blockquote>
<p>Right, good point. OK, then let me rephrase it for explicitly calling store.<br>
...<br>
That way I can check for collisions in one pass without having to call has_key? first.</p>
</blockquote>
<p>It's a nice thought, but about fifteen years too late - you can't just change<br>
fundamental things like this without introducing all sorts of subtle bugs into<br>
people's programs. Write a new method (e.g. put(x)) that has the behaviour<br>
you want.</p>
</blockquote>
<p>Doing so in one pass is only possible on the C level. No use in composing it in<br>
Ruby, this still means that the hash function is evaluated twice.</p>
<p>Hash is probably way too popular for no one to rely on the current<br>
return value, I agree. But put isn't taken yet - how about Hash#put with the described<br>
behavior?</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253472012-03-29T11:44:28Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul></ul><p>=begin</p>
<p>Hmm, here you are a patch (not tested though).</p>
<p>From c55a9c9fab30d51be77821bce36054fe365b49af Mon Sep 17 00:00:00 2001<br>
Message-Id: <a href="mailto:c55a9c9fab30d51be77821bce36054fe365b49af.1332988729.git.shyouhei@ruby-lang.org" class="email">c55a9c9fab30d51be77821bce36054fe365b49af.1332988729.git.shyouhei@ruby-lang.org</a><br>
From: URABE, Shyouhei <a href="mailto:shyouhei@ruby-lang.org" class="email">shyouhei@ruby-lang.org</a><br>
Date: Thu, 29 Mar 2012 11:38:17 +0900<br>
Subject: [PATCH 1/1] Hash#store to return former content [feature <a class="issue tracker-2 status-7 priority-4 priority-default closed" title="Feature: Return value of Hash#store (Feedback)" href="https://redmine.ruby-lang.org/issues/6219">#6219</a>]</p>
<p>Signed-off-by: URABE, Shyouhei <a href="mailto:shyouhei@ruby-lang.org" class="email">shyouhei@ruby-lang.org</a></p>
<p>diff --git a/hash.c b/hash.c<br>
index e9937ff..0cd4e43 100644<br>
--- a/hash.c<br>
+++ b/hash.c<br>
@@ -1099,6 +1099,20 @@ copy_str_key(st_data_t str)<br>
return (st_data_t)rb_str_new4((VALUE)str);<br>
}</p>
<p>+struct hash_aset_tuple {</p>
<ul>
<li>st_data_t old;</li>
<li>st_data_t new;<br>
+};</li>
<li>
</ul>
<p>+static int<br>
+hash_aset_i(st_data_t key, st_data_t *value, st_data_t arg)<br>
+{</p>
<ul>
<li>struct hash_aset_tuple *ptr = (void *)arg;</li>
<li>ptr->old = *value;</li>
<li>*value = ptr->new;</li>
<li>return ST_CONTINUE;<br>
+}</li>
<li>
</ul>
<p>/*</p>
<ul>
<li>call-seq:</li>
<li>
<pre><code>hsh[key] = value -> value
</code></pre>
</li>
</ul>
<p>@@ -1120,15 +1134,19 @@ copy_str_key(st_data_t str)<br>
VALUE<br>
rb_hash_aset(VALUE hash, VALUE key, VALUE val)<br>
{</p>
<ul>
<li>struct hash_aset_tuple t;<br>
rb_hash_modify(hash);<br>
hash_update(hash, key);</li>
<li>t.new = val;</li>
<li>t.old = Qundef;<br>
if (RHASH(hash)->ntbl->type == &identhash || rb_obj_class(key) != rb_cString) {</li>
</ul>
<ul>
<li>st_insert(RHASH(hash)->ntbl, key, val);</li>
</ul>
<ul>
<li>st_update(RHASH(hash)->ntbl, key, hash_aset_i, (st_data_t)&t);<br>
}<br>
else {</li>
</ul>
<ul>
<li>st_insert2(RHASH(hash)->ntbl, key, val, copy_str_key);</li>
</ul>
<ul>
<li>st_data_t k = copy_str_key(key);</li>
<li>st_update(RHASH(hash)->ntbl, k, hash_aset_i, (st_data_t)&t);<br>
}</li>
</ul>
<ul>
<li>return val;</li>
</ul>
<ul>
<li>return t.old;<br>
}</li>
</ul>
<a name="static-int"></a>
<h2 >static int<a href="#static-int" class="wiki-anchor">¶</a></h2>
<p>1.7.0.4</p>
<p>=end</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253552012-03-29T20:38:36ZMartinBosslet (Martin Bosslet)Martin.Bosslet@gmail.com
<ul></ul><p>shyouhei (Shyouhei Urabe) wrote:</p>
<blockquote>
<p>=begin</p>
<p>Hmm, here you are a patch (not tested though).</p>
</blockquote>
<p>Awesome, thank you Shyouhei. I'll be happy to test this tonight!</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253592012-03-29T21:33:37Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul></ul><p>Note however, that nobu changed st_update API<a href="http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?revision=35170&view=revision" class="external">1</a> after I posted that. So it might not apply cleanly to the latest ruby trunk...</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253662012-03-29T22:56:47Znobu (Nobuyoshi Nakada)nobu@ruby-lang.org
<ul></ul><p>And, what should be returned if the key wasn't set?</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253702012-03-29T23:32:21Zrosenfeld (Rodrigo Rosenfeld Rosas)rr.rosas@gmail.com
<ul></ul><p>nobu (Nobuyoshi Nakada) wrote:</p>
<blockquote>
<p>And, what should be returned if the key wasn't set?</p>
</blockquote>
<p>Its default value?</p>
<p>h = Hash.new {[]}<br>
h.store('key', [1, 2]) # => []</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253722012-03-29T23:42:15ZMartinBosslet (Martin Bosslet)Martin.Bosslet@gmail.com
<ul></ul><p>rosenfeld (Rodrigo Rosenfeld Rosas) wrote:</p>
<blockquote>
<p>nobu (Nobuyoshi Nakada) wrote:</p>
<blockquote>
<p>And, what should be returned if the key wasn't set?</p>
</blockquote>
<p>Its default value?</p>
</blockquote>
<p>Seems reasonable to me. To detect whether a key hasn't been set yet,<br>
one would check for the presence of the default value. That<br>
doesn't catch cases where a key with a default value is actually<br>
set, but that seems fine to me. Voices against this?</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253802012-03-30T00:25:04Zmame (Yusuke Endoh)mame@ruby-lang.org
<ul><li><strong>Status</strong> changed from <i>Open</i> to <i>Assigned</i></li><li><strong>Assignee</strong> set to <i>matz (Yukihiro Matsumoto)</i></li><li><strong>Target version</strong> set to <i>3.0</i></li></ul><p>Hello,</p>
<p>I tentatively mark this as 3.0 issue because it changes the behavior.<br>
But if matz accepts the change, 2.0 may be able to include it.</p>
<p>Personally, I'm neutral to the proposal itself.</p>
<p>--<br>
Yusuke Endoh <a href="mailto:mame@tsg.ne.jp" class="email">mame@tsg.ne.jp</a></p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253912012-03-30T01:09:11Znobu (Nobuyoshi Nakada)nobu@ruby-lang.org
<ul><li><strong>File</strong> <a href="/attachments/2547">0001-Hash-store-return-old-value.patch</a> <a class="icon-only icon-download" title="Download" href="/attachments/download/2547/0001-Hash-store-return-old-value.patch">0001-Hash-store-return-old-value.patch</a> added</li></ul><p>rosenfeld (Rodrigo Rosenfeld Rosas) wrote:</p>
<blockquote>
<blockquote>
<p>And, what should be returned if the key wasn't set?</p>
</blockquote>
<p>Its default value?</p>
</blockquote>
<p>Also calling its default proc?</p>
<p>MartinBosslet (Martin Bosslet) wrote:</p>
<blockquote>
<p>Seems reasonable to me. To detect whether a key hasn't been set yet,<br>
one would check for the presence of the default value. That<br>
doesn't catch cases where a key with a default value is actually<br>
set, but that seems fine to me. Voices against this?</p>
</blockquote>
<p>Calling default proc can be expensive. So I don't think it's good idea to call it always.<br>
A patch to separate Hash#store from Hash#[]=, and let it return the old value.</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253962012-03-30T01:27:12Zrosenfeld (Rodrigo Rosenfeld Rosas)rr.rosas@gmail.com
<ul></ul><p>How about hash.store(new_value, nil_value: :<em>default</em>)</p>
<p>This way, one could write "a = hash.store(1, nil_value: nil)", while the default :<em>default</em> would call the default proc if it exists.</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=253972012-03-30T01:29:21Zrosenfeld (Rodrigo Rosenfeld Rosas)rr.rosas@gmail.com
<ul></ul><p>Or Hash#store(new_value, return_nil_if_not_set: false)</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=254152012-03-30T07:48:42ZMartinBosslet (Martin Bosslet)Martin.Bosslet@gmail.com
<ul></ul><p>nobu (Nobuyoshi Nakada) wrote:</p>
<blockquote>
<p>Calling default proc can be expensive. So I don't think it's good idea to call it always.<br>
A patch to separate Hash#store from Hash#[]=, and let it return the old value.</p>
</blockquote>
<p>You are right, the default proc is a real "party pooper" here... Could something like Rodrigo<br>
proposed work for the default proc case? Did you mean something like it when you proposed<br>
separating Hash#store from #[]=, or did you think about something different?</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=254892012-03-31T10:07:50Zmatz (Yukihiro Matsumoto)matz@ruby.or.jp
<ul></ul><p>by spec, all assignment should return the assigning value (to enable assignment chain), so that I have to reject the original proposal to change the value from "a[k] = v".<br>
but there's still room to change the return value of Hash#store.</p>
<p>Matz.</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=255082012-03-31T11:13:06Zmatz (Yukihiro Matsumoto)matz@ruby.or.jp
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Feedback</i></li></ul> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=262022012-04-26T09:20:00ZMartinBosslet (Martin Bosslet)Martin.Bosslet@gmail.com
<ul></ul><p>Yes, I neglected assignment in my initial proposal, silly idea :) Hash#store would be great, though! Apart from the problem of how to handle the default proc, are there still other reasons against this?</p> Ruby master - Feature #6219: Return value of Hash#storehttps://redmine.ruby-lang.org/issues/6219?journal_id=890852020-12-10T08:46:55Znaruse (Yui NARUSE)naruse@airemix.jp
<ul><li><strong>Target version</strong> deleted (<del><i>3.0</i></del>)</li></ul>