[ruby-core:59504] [ruby-trunk - Feature #8998] string keys for hash literals should use fstrings

Issue #8998 has been updated by headius (Charles Nutter).


I ran into a small snag so I think it will be useful to have my JRuby commits here.

First, a background commit... adding a weak dedup cache: https://github.com/jruby/jruby/commit/926ca89075a4a4c84592add729531189263c143f

It's not particularly complicated; a double-weak hash synchronized for concurrency purposes.

Here's the commit that adds dedup to literal hashes with literal string keys: https://github.com/jruby/jruby/commit/aedcbc7b4024cf651a1df3bb65a8490a74a66562

This commit does all of the following:

* Interpreter and compiler logic for literal hash with literal keys will only freeze-dup them once, using the dedup cache.
* Hash will use dedup cache for all incoming strings.

The first problem I ran into was that there are tests in CSV that appear to depend on already-frozen strings not being deduplicated during hash insertion. This brought about the following failures:

  1) Failure:
TestCSV::Interface#test_write_hash_with_string_keys [/Users/headius/projects/jruby/test/mri/csv/test_interface.rb:214]:
Expected "a" (oid=9726) to be the same as "a" (oid=9728).

  2) Failure:
TestCSV::Interface::DifferentOFS#test_write_hash_with_string_keys [/Users/headius/projects/jruby/test/mri/csv/test_interface.rb:214]:
Expected "a" (oid=9726) to be the same as "a" (oid=9746).

  3) Failure:
TestCSV::Row#test_to_hash [/Users/headius/projects/jruby/test/mri/csv/test_row.rb:304]:
Expected "A" (oid=9748) to be the same as "A" (oid=9750).

  4) Failure:
TestCSV::Row::DifferentOFS#test_to_hash [/Users/headius/projects/jruby/test/mri/csv/test_row.rb:304]:
Expected "A" (oid=9752) to be the same as "A" (oid=9750).

I fixed these by adding a check to only dedup incoming strings if they weren't already frozen. I'm not sure if that's the right course of action or if the tests should be changed.

Then I ran into a more unusual snag. In RubySpec, there are tests for Hash#compare_by_identity/= that fail if I dedup incoming strings, since they test that the same string content in two different objects can hold two different values in the Hash. Specifically:

1)
Hash#compare_by_identity perists over #dups FAILED
Expected {"foo"=>:glark}
 to equal {"foo"=>:bar, "foo"=>:glark}

/Users/headius/projects/jruby/spec/ruby/core/hash/compare_by_identity_spec.rb:77:in `(root)'
org/jruby/RubyBasicObject.java:1573:in `instance_eval'
org/jruby/RubyEnumerable.java:1437:in `all?'
org/jruby/RubyFixnum.java:269:in `times'
org/jruby/RubyArray.java:1556:in `each'
/Users/headius/projects/jruby/spec/ruby/core/hash/compare_by_identity_spec.rb:2:in `(root)'
/Users/headius/projects/jruby/spec/ruby/core/hash/compare_by_identity_spec.rb:1:in `(root)'
org/jruby/RubyKernel.java:881:in `load'
org/jruby/RubyBasicObject.java:1573:in `instance_eval'
org/jruby/RubyArray.java:1556:in `each'

2)
Hash#compare_by_identity persists over #clones FAILED
Expected {"foo"=>:glark}
 to equal {"foo"=>:bar, "foo"=>:glark}

/Users/headius/projects/jruby/spec/ruby/core/hash/compare_by_identity_spec.rb:84:in `(root)'
org/jruby/RubyBasicObject.java:1573:in `instance_eval'
org/jruby/RubyEnumerable.java:1437:in `all?'
org/jruby/RubyFixnum.java:269:in `times'
org/jruby/RubyArray.java:1556:in `each'
/Users/headius/projects/jruby/spec/ruby/core/hash/compare_by_identity_spec.rb:2:in `(root)'
/Users/headius/projects/jruby/spec/ruby/core/hash/compare_by_identity_spec.rb:1:in `(root)'
org/jruby/RubyKernel.java:881:in `load'
org/jruby/RubyBasicObject.java:1573:in `instance_eval'
org/jruby/RubyArray.java:1556:in `each'

I fixed this issue by only deduping when compare_by_identity is false, but I'm not sure if it's correct.

So the basic question that came out of my experiment:

* Should string freeze-duping in Hash#[]= always dedup, regardless of whether the string has already been frozen or the Hash has compare_by_identity == true? Or else what combination of those conditions should lead to deduping?
----------------------------------------
Feature #8998: string keys for hash literals should use fstrings
https://bugs.ruby-lang.org/issues/8998#change-44049

Author: normalperson (Eric Wong)
Status: Closed
Priority: Low
Assignee: 
Category: core
Target version: 2.1.0


While we're introducing optimizations from frozen strings,
string keys inside hashes should be frozen at the compiler level
to prevent duplication.

	a = { "ABC" => :t }
	b = { "ABC" => :t }

	# the following ought to print true
	p(a.keys[0].object_id == b.keys[0].object_id)

This should introduce no incompatibilities and be transparent to users of
older rubies.



-- 
http://bugs.ruby-lang.org/