From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Original-To: poffice@blade.nagaokaut.ac.jp Delivered-To: poffice@blade.nagaokaut.ac.jp Received: from kankan.nagaokaut.ac.jp (kankan.nagaokaut.ac.jp [133.44.2.24]) by blade.nagaokaut.ac.jp (Postfix) with ESMTP id 3069819C0072 for ; Fri, 4 Dec 2015 08:09:40 +0900 (JST) Received: from voscc.nagaokaut.ac.jp (voscc.nagaokaut.ac.jp [133.44.1.100]) by kankan.nagaokaut.ac.jp (Postfix) with ESMTP id BA29AB5D88D for ; Fri, 4 Dec 2015 08:41:08 +0900 (JST) Received: from neon.ruby-lang.org (neon.ruby-lang.org [221.186.184.75]) by voscc.nagaokaut.ac.jp (Postfix) with ESMTP id BA12518CC7B8 for ; Fri, 4 Dec 2015 08:41:08 +0900 (JST) Received: from [221.186.184.76] (localhost [IPv6:::1]) by neon.ruby-lang.org (Postfix) with ESMTP id F314112050C; Fri, 4 Dec 2015 08:41:06 +0900 (JST) X-Original-To: ruby-core@ruby-lang.org Delivered-To: ruby-core@ruby-lang.org Received: from dcvr.yhbt.net (dcvr.yhbt.net [64.71.152.64]) by neon.ruby-lang.org (Postfix) with ESMTP id DDFD31204B6 for ; Fri, 4 Dec 2015 08:41:03 +0900 (JST) Received: from localhost (dcvr.yhbt.net [127.0.0.1]) (using TLSv1.2 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPSA id 3AA9C205F2; Thu, 3 Dec 2015 23:41:03 +0000 (UTC) Date: Thu, 3 Dec 2015 23:41:03 +0000 From: Eric Wong To: Ruby developers Message-ID: <20151203234103.GA30841@dcvr.yhbt.net> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-ML-Name: ruby-core X-Mail-Count: 71820 Subject: [ruby-core:71820] Re: [Ruby trunk - Bug #11759] URI breaks with frozen strings X-BeenThere: ruby-core@ruby-lang.org X-Mailman-Version: 2.1.15 Precedence: list Reply-To: Ruby developers List-Id: Ruby developers List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: ruby-core-bounces@ruby-lang.org Sender: "ruby-core" 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....