From: Eric Wong <normalperson@...>
Date: 2015-12-03T23:41:03+00:00
Subject: [ruby-core:71820] Re: [Ruby trunk - Bug #11759] URI breaks with frozen strings

colin@invoca.com wrote:
> Compared to the surrounding code in the full method, would the extra
> constant lookup make a measurable difference in code size?

64 bytes on ruby 2.3.0dev (2015-12-03 trunk 52872) [x86_64-linux].

require 'objspace'
iseq = RubyVM::InstructionSequence
p ObjectSpace.memsize_of(iseq.compile("str = String.new"))
p ObjectSpace.memsize_of(iseq.compile("str = ''.dup"))
p ObjectSpace.memsize_of(iseq.compile("str = ''.freeze.dup"))

Outputs:
480
416
416

> because the `.freeze` is temporary for Ruby 2.1 and 2.2, right?  When
> would this copy of generic.rb ever be run with Ruby versions earlier
> than 2.3?

No, we shouldn't worry about performance with older versions of Ruby
with the stdlib.

> Assuming it will just be used for Ruby 2.3 and later, the
> magic comment included in the patch will implicitly freeze the string
> literal.  Hence this would also be sufficient (and in my opinion,
> nearly as clear in intention as `String.new`):
> 
> ~~~
> ''.dup
> ~~~

I prefer that if we go for "frozen_string_literal: true" in the comment.

I've also added this test for to_s.  However, checking more closely,
the "path" accessor also gets frozen changes because we call
set_path with literal strings.

There may be code out in the wild which relies on "path" being mutable.

--- a/test/uri/test_generic.rb
+++ b/test/uri/test_generic.rb
@@ -14,6 +14,13 @@ def uri_to_ary(uri)
     uri.class.component.collect {|c| uri.send(c)}
   end
 
+  def test_to_s
+    exp = 'http://example.com/'.freeze
+    str = URI(exp).to_s
+    assert_equal exp, str
+    refute_predicate str, :frozen?, '[ruby-core:71785] [Bug #11759]'
+  end
+
   def test_parse
     # 0
     assert_kind_of(URI::HTTP, @base_url)

...  So perhaps at least one additional change is needed:

--- a/lib/uri/generic.rb
+++ b/lib/uri/generic.rb
@@ -786,7 +786,7 @@ def check_path(v)
     # see also URI::Generic.path=
     #
     def set_path(v)
-      @path = v
+      @path = v.frozen? ? v.dup : v
     end
     protected :set_path
 

All these frozen literal changes will require going every single
method and and call site with a very fine tooth comb....