[前][次][番号順一覧][スレッド一覧]

ruby-changes:59613

From: Jeremy <ko1@a...>
Date: Sat, 4 Jan 2020 13:13:39 +0900 (JST)
Subject: [ruby-changes:59613] 04eb7c7e46 (master): Call initialize_clone with freeze: false if clone called with freeze: false

https://git.ruby-lang.org/ruby.git/commit/?id=04eb7c7e46

From 04eb7c7e462d1fcbab9efc7022c1b43636ab878a Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@j...>
Date: Sun, 25 Aug 2019 21:11:46 -0700
Subject: Call initialize_clone with freeze: false if clone called with freeze:
 false

This makes it possible to initialize_clone to correctly not freeze
internal state if the freeze: false keyword is passed to clone.

If clone is called with freeze: true or no keyword, do not pass
a second argument to initialize_clone to keep backwards
compatibility.

This makes it so that external libraries that override
initialize_clone but do not support the freeze keyword will fail
with ArgumentError if passing freeze: false to clone.  I think that
is better than the current behavior, which succeeds but results in
an unfrozen object with frozen internals.

Fix related issues in set and delegate in stdlib.

Fixes [Bug #14266]

diff --git a/lib/delegate.rb b/lib/delegate.rb
index 8c176dc..0bbe211 100644
--- a/lib/delegate.rb
+++ b/lib/delegate.rb
@@ -211,8 +211,8 @@ class Delegator < BasicObject https://github.com/ruby/ruby/blob/trunk/lib/delegate.rb#L211
     end
   end
 
-  def initialize_clone(obj) # :nodoc:
-    self.__setobj__(obj.__getobj__.clone)
+  def initialize_clone(obj, freeze: true) # :nodoc:
+    self.__setobj__(obj.__getobj__.clone(freeze: freeze))
   end
   def initialize_dup(obj) # :nodoc:
     self.__setobj__(obj.__getobj__.dup)
diff --git a/lib/set.rb b/lib/set.rb
index e7d1be4..6841155 100644
--- a/lib/set.rb
+++ b/lib/set.rb
@@ -137,9 +137,9 @@ class Set https://github.com/ruby/ruby/blob/trunk/lib/set.rb#L137
   end
 
   # Clone internal hash.
-  def initialize_clone(orig)
+  def initialize_clone(orig, freeze: true)
     super
-    @hash = orig.instance_variable_get(:@hash).clone
+    @hash = orig.instance_variable_get(:@hash).clone(freeze: freeze)
   end
 
   def freeze    # :nodoc:
diff --git a/object.c b/object.c
index f02b45a..0080c35 100644
--- a/object.c
+++ b/object.c
@@ -476,11 +476,25 @@ mutable_obj_clone(VALUE obj, int kwfreeze) https://github.com/ruby/ruby/blob/trunk/object.c#L476
     }
 
     init_copy(clone, obj);
-    rb_funcall(clone, id_init_clone, 1, obj);
 
     if (kwfreeze) {
+        rb_funcall(clone, id_init_clone, 1, obj);
 	RBASIC(clone)->flags |= RBASIC(obj)->flags & FL_FREEZE;
     }
+    else {
+        static VALUE freeze_false_hash;
+        VALUE argv[2];
+        if (!freeze_false_hash) {
+            freeze_false_hash = rb_hash_new();
+            rb_hash_aset(freeze_false_hash, ID2SYM(rb_intern("freeze")), Qfalse);
+            rb_obj_freeze(freeze_false_hash);
+            rb_gc_register_mark_object(freeze_false_hash);
+        }
+
+        argv[0] = obj;
+        argv[1] = freeze_false_hash;
+        rb_funcallv_kw(clone, id_init_clone, 2, argv, RB_PASS_CALLED_KEYWORDS);
+    }
 
     return clone;
 }
@@ -637,10 +651,10 @@ rb_obj_init_copy(VALUE obj, VALUE orig) https://github.com/ruby/ruby/blob/trunk/object.c#L651
 /*!
  * :nodoc:
  *--
- * Default implementation of \c #initialize_dup and \c #initialize_clone
+ * Default implementation of \c #initialize_dup
  *
  * \param[in,out] obj the receiver being initialized
- * \param[in] orig    the object to be dup or cloned from.
+ * \param[in] orig    the object to be dup from.
  *++
  **/
 VALUE
@@ -650,6 +664,27 @@ rb_obj_init_dup_clone(VALUE obj, VALUE orig) https://github.com/ruby/ruby/blob/trunk/object.c#L664
     return obj;
 }
 
+/*!
+ * :nodoc:
+ *--
+ * Default implementation of \c #initialize_clone
+ *
+ * \param[in] The number of arguments
+ * \param[in] The array of arguments
+ * \param[in] obj the receiver being initialized
+ *++
+ **/
+static VALUE
+rb_obj_init_clone(int argc, VALUE *argv, VALUE obj)
+{
+    VALUE orig, opts;
+    rb_scan_args(argc, argv, "1:", &orig, &opts);
+    /* Ignore a freeze keyword */
+    if (argc == 2) (void)freeze_opt(1, &opts);
+    rb_funcall(obj, id_init_copy, 1, orig);
+    return obj;
+}
+
 /**
  *  call-seq:
  *     obj.to_s    -> string
@@ -1968,10 +2003,11 @@ rb_mod_initialize(VALUE module) https://github.com/ruby/ruby/blob/trunk/object.c#L2003
 
 /* :nodoc: */
 static VALUE
-rb_mod_initialize_clone(VALUE clone, VALUE orig)
+rb_mod_initialize_clone(int argc, VALUE* argv, VALUE clone)
 {
-    VALUE ret;
-    ret = rb_obj_init_dup_clone(clone, orig);
+    VALUE ret, orig, opts;
+    rb_scan_args(argc, argv, "1:", &orig, &opts);
+    ret = rb_obj_init_clone(argc, argv, clone);
     if (OBJ_FROZEN(orig))
         rb_class_name(clone);
     return ret;
@@ -4579,7 +4615,7 @@ InitVM_Object(void) https://github.com/ruby/ruby/blob/trunk/object.c#L4615
     rb_define_method(rb_mKernel, "then", rb_obj_yield_self, 0);
     rb_define_method(rb_mKernel, "initialize_copy", rb_obj_init_copy, 1);
     rb_define_method(rb_mKernel, "initialize_dup", rb_obj_init_dup_clone, 1);
-    rb_define_method(rb_mKernel, "initialize_clone", rb_obj_init_dup_clone, 1);
+    rb_define_method(rb_mKernel, "initialize_clone", rb_obj_init_clone, -1);
 
     rb_define_method(rb_mKernel, "taint", rb_obj_taint, 0);
     rb_define_method(rb_mKernel, "tainted?", rb_obj_tainted, 0);
@@ -4666,7 +4702,7 @@ InitVM_Object(void) https://github.com/ruby/ruby/blob/trunk/object.c#L4702
 
     rb_define_alloc_func(rb_cModule, rb_module_s_alloc);
     rb_define_method(rb_cModule, "initialize", rb_mod_initialize, 0);
-    rb_define_method(rb_cModule, "initialize_clone", rb_mod_initialize_clone, 1);
+    rb_define_method(rb_cModule, "initialize_clone", rb_mod_initialize_clone, -1);
     rb_define_method(rb_cModule, "instance_methods", rb_class_instance_methods, -1); /* in class.c */
     rb_define_method(rb_cModule, "public_instance_methods",
 		     rb_class_public_instance_methods, -1);    /* in class.c */
diff --git a/test/ruby/test_module.rb b/test/ruby/test_module.rb
index 2e7e580..9373c48 100644
--- a/test/ruby/test_module.rb
+++ b/test/ruby/test_module.rb
@@ -2491,6 +2491,12 @@ class TestModule < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_module.rb#L2491
     assert_equal :M2, CloneTestC2.new.foo, '[Bug #15877]'
   end
 
+  def test_clone_freeze
+    m = Module.new.freeze
+    assert_predicate m.clone, :frozen?
+    assert_not_predicate m.clone(freeze: false), :frozen?
+  end
+
   private
 
   def assert_top_method_is_private(method)
diff --git a/test/ruby/test_object.rb b/test/ruby/test_object.rb
index add5b9f..bfc7d7d 100644
--- a/test/ruby/test_object.rb
+++ b/test/ruby/test_object.rb
@@ -75,6 +75,28 @@ class TestObject < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_object.rb#L75
     assert_raise_with_message(ArgumentError, /\u{1f4a9}/) do
       Object.new.clone(freeze: x)
     end
+
+    c = Class.new do
+      attr_reader :f
+    end
+    o = c.new
+    f = true
+    def o.initialize_clone(_, freeze: true)
+      @f = freeze
+      super
+    end
+    clone = o.clone
+    assert_kind_of c, clone
+    assert_equal true, clone.f
+    clone = o.clone(freeze: false)
+    assert_kind_of c, clone
+    assert_equal false, clone.f
+
+    def o.initialize_clone(_)
+      super
+    end
+    assert_kind_of c, o.clone
+    assert_raise(ArgumentError) { o.clone(freeze: false) }
   end
 
   def test_init_dupclone
diff --git a/test/test_delegate.rb b/test/test_delegate.rb
index 303cd16..80bf416 100644
--- a/test/test_delegate.rb
+++ b/test/test_delegate.rb
@@ -50,6 +50,21 @@ class TestDelegateClass < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/test_delegate.rb#L50
     assert_equal(SimpleDelegator,simple.clone.class)
   end
 
+  def test_simpledelegator_clone
+    simple=SimpleDelegator.new([])
+    simple.freeze
+
+    clone = simple.clone
+    assert_predicate clone, :frozen?
+    assert_predicate clone.__getobj__, :frozen?
+    assert_equal true, Kernel.instance_method(:frozen?).bind(clone).call
+
+    clone = simple.clone(freeze: false)
+    assert_not_predicate clone, :frozen?
+    assert_not_predicate clone.__getobj__, :frozen?
+    assert_equal false, Kernel.instance_method(:frozen?).bind(clone).call
+  end
+
   class Object
     def m
       :o
diff --git a/test/test_set.rb b/test/test_set.rb
index b0f669c..68ee7ce 100644
--- a/test/test_set.rb
+++ b/test/test_set.rb
@@ -730,6 +730,17 @@ class TC_Set < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/test_set.rb#L730
     }
   end
 
+  def test_freeze_clone_false
+    set1 = Set[1,2,3]
+    set1.freeze
+    set2 = set1.clone(freeze: false)
+
+    assert_not_predicate set2, :frozen?
+    set2.add 5
+    assert_equal Set[1,2,3,5], set2
+    assert_equal Set[1,2,3], set1
+  end
+
   def test_inspect
     set1 = Set[1, 2]
     assert_equal('#<Set: {1, 2}>', set1.inspect)
-- 
cgit v0.10.2


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]