https://bugs.ruby-lang.org/https://bugs.ruby-lang.org/favicon.ico?17113305112016-10-20T13:52:55ZRuby Issue Tracking SystemRuby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=609682016-10-20T13:52:55Zjeremyevans0 (Jeremy Evans)merch-redmine@jeremyevans.net
<ul></ul><p>While this was a behavior change between 2.1 and 2.2, I'm not sure I would consider it a regression. It seems unlikely anyone who uses compare_by_identity hashes would also be using string literals and want string literals keys to have different values.</p>
<p>If we do decide to revert to 2.1 behavior for string literals, at the very least we should make sure that on ruby 2.1+:</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="n">h</span> <span class="o">=</span> <span class="p">{}.</span><span class="nf">compare_by_identity</span>
<span class="n">h</span><span class="p">[</span><span class="s1">'pear'</span><span class="p">.</span><span class="nf">freeze</span><span class="p">]</span> <span class="o">=</span> <span class="mi">1</span>
<span class="n">h</span><span class="p">[</span><span class="s1">'pear'</span><span class="p">.</span><span class="nf">freeze</span><span class="p">]</span> <span class="o">=</span> <span class="mi">2</span>
<span class="nb">p</span> <span class="n">h</span><span class="p">.</span><span class="nf">size</span> <span class="c1"># => 1</span>
<span class="c1"># because 'pear'.freeze.equal?('pear'.freeze)</span>
</code></pre>
<p>and on ruby 2.3+:</p>
<pre><code class="ruby syntaxhl" data-language="ruby"><span class="c1"># frozen-string-literal: true</span>
<span class="c1"># or when using --enable-frozen-string-literal</span>
<span class="n">h</span> <span class="o">=</span> <span class="p">{}.</span><span class="nf">compare_by_identity</span>
<span class="n">h</span><span class="p">[</span><span class="s1">'pear'</span><span class="p">]</span> <span class="o">=</span> <span class="mi">1</span>
<span class="n">h</span><span class="p">[</span><span class="s1">'pear'</span><span class="p">]</span> <span class="o">=</span> <span class="mi">2</span>
<span class="nb">p</span> <span class="n">h</span><span class="p">.</span><span class="nf">size</span> <span class="c1"># => 1</span>
<span class="c1"># because 'pear'.equal?('pear')</span>
</code></pre> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=609702016-10-20T17:29:12ZEregon (Benoit Daloze)
<ul></ul><p>Jeremy Evans wrote:</p>
<blockquote>
<p>While this was a behavior change between 2.1 and 2.2, I'm not sure I would consider it a regression.<br>
It seems unlikely anyone who uses compare_by_identity hashes would also be using string literals and want string literals keys to have different values.</p>
</blockquote>
<p>The main reason I consider it a bug is that it contradicts the very basic intuition that replacing a literal with an expression producing it has identical behavior.<br>
For example, <code>"z = #{3*4}"</code> vs <code>r = 12; "z = #{r}"</code>.<br>
While of course a much smaller area of exposition,<br>
I think there is no reason to introduce this change (and I'm fairly confident it was not intended).</p> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=621672016-12-21T07:39:23Zshyouhei (Shyouhei Urabe)shyouhei@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>normalperson (Eric Wong)</i></li></ul> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=621882016-12-21T14:47:07Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-5 priority-4 priority-default closed" href="/issues/9382">Bug #9382</a>: [patch] add opt_aref_str and opt_aset_str</i> added</li></ul> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=621902016-12-21T14:48:33Zshyouhei (Shyouhei Urabe)shyouhei@ruby-lang.org
<ul><li><strong>Assignee</strong> changed from <i>normalperson (Eric Wong)</i> to <i>tmm1 (Aman Karmani)</i></li></ul><p>We looked at this issue at today's developer meeting. It seems over-optimization as Benoit originally guessed. Let me assign this to Aman.</p> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=622032016-12-22T01:03:33Znormalperson (Eric Wong)normalperson@yhbt.net
<ul></ul><p>Unintentional, yes, but maybe moot since frozen_string_literal<br>
is going to be default in the future (maybe that case changed?)</p>
<p>Anyways, I hate this patch because it makes insns.def bigger,<br>
and insns.def is already too big IMHO:</p>
<p><a href="https://80x24.org/spew/20161222004752.4086-1-e@80x24.org/raw" class="external">https://80x24.org/spew/20161222004752.4086-1-e@80x24.org/raw</a></p>
<p>Somebody else can commit; I hate it too much.</p> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=622122016-12-22T12:45:36ZEregon (Benoit Daloze)
<ul></ul><p>Eric Wong wrote:</p>
<blockquote>
<p>Unintentional, yes, but maybe moot since frozen_string_literal<br>
is going to be default in the future (maybe that case changed?)</p>
<p>Anyways, I hate this patch because it makes insns.def bigger,<br>
and insns.def is already too big IMHO:</p>
<p><a href="https://80x24.org/spew/20161222004752.4086-1-e@80x24.org/raw" class="external">https://80x24.org/spew/20161222004752.4086-1-e@80x24.org/raw</a></p>
<p>Somebody else can commit; I hate it too much.</p>
</blockquote>
<p>Thank you for the patch!<br>
I would like to commit it then, unless there is an objection.</p>
<p>IMHO optimizations have to be correct, even in edge cases.<br>
Such instructions are already a trade-off of duplication and complexity for speed improvement,<br>
so this small change in size does not matter much to me.<br>
Optimizations need to be totally transparent to the user to be called as such.</p> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=624142017-01-07T11:31:58ZEregon (Benoit Daloze)
<ul><li><strong>Status</strong> changed from <i>Assigned</i> to <i>Closed</i></li></ul><p>Applied in changeset r57278.</p>
<hr>
<p>fix optimization for hash aset/aref with fstring</p>
<p>Patch by Eric Wong <a href="/issues/12855">[ruby-core:78797]</a>.<br>
I don't like the idea of making insns.def any bigger to support<br>
a corner case, and "test_hash_aref_fstring_identity" shows<br>
how contrived this is.</p>
<p><a href="/issues/12855">[ruby-core:78783]</a> [Bug <a class="issue tracker-1 status-5 priority-4 priority-default closed" title="Bug: Inconsistent keys identity in compare_by_identity Hash when using literals (Closed)" href="https://bugs.ruby-lang.org/issues/12855">#12855</a>]</p> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=634422017-03-11T14:48:09Znaruse (Yui NARUSE)naruse@airemix.jp
<ul><li><strong>Backport</strong> changed from <i>2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN</i> to <i>2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: DONE</i></li></ul><p>ruby_2_4 r57848 merged revision(s) 57278,57279.</p> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=635132017-03-13T01:00:54Znagachika (Tomoyuki Chikanaga)nagachika00@gmail.com
<ul><li><strong>Backport</strong> changed from <i>2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: DONE</i> to <i>2.1: UNKNOWN, 2.2: REQUIRED, 2.3: REQUIRED, 2.4: DONE</i></li></ul> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=638082017-03-25T16:31:58Zusa (Usaku NAKAMURA)usa@garbagecollect.jp
<ul><li><strong>Backport</strong> changed from <i>2.1: UNKNOWN, 2.2: REQUIRED, 2.3: REQUIRED, 2.4: DONE</i> to <i>2.1: UNKNOWN, 2.2: DONE, 2.3: REQUIRED, 2.4: DONE</i></li></ul><p>ruby_2_2 r58099 merged revision(s) 57278,57279.</p> Ruby master - Bug #12855: Inconsistent keys identity in compare_by_identity Hash when using literalshttps://bugs.ruby-lang.org/issues/12855?journal_id=638812017-03-27T16:02:54Znagachika (Tomoyuki Chikanaga)nagachika00@gmail.com
<ul><li><strong>Backport</strong> changed from <i>2.1: UNKNOWN, 2.2: DONE, 2.3: REQUIRED, 2.4: DONE</i> to <i>2.1: UNKNOWN, 2.2: DONE, 2.3: DONE, 2.4: DONE</i></li></ul><p>ruby_2_3 r58166 merged revision(s) 57278,57279.</p>