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

ruby-changes:60472

From: Jeremy <ko1@a...>
Date: Mon, 23 Mar 2020 01:30:26 +0900 (JST)
Subject: [ruby-changes:60472] 4f7b435c95 (master): Support obj.clone(freeze: true) for freezing clone

https://git.ruby-lang.org/ruby.git/commit/?id=4f7b435c95

From 4f7b435c955792af780fe9e94f98d3dde839e056 Mon Sep 17 00:00:00 2001
From: Jeremy Evans <code@j...>
Date: Mon, 23 Sep 2019 16:03:15 -0700
Subject: Support obj.clone(freeze: true) for freezing clone

This freezes the clone even if the receiver is not frozen.  It
is only for consistency with freeze: false not freezing the clone
even if the receiver is frozen.

Because Object#clone is now partially implemented in Ruby and
not fully implemented in C, freeze: nil must be supported to
provide the default behavior of only freezing the clone if the
receiver is frozen.

This requires modifying delegate and set, to set freeze: nil
instead of freeze: true as the keyword parameter for
initialize_clone.  Those are the two libraries in stdlib that
override initialize_clone.

Implements [Feature #16175]

diff --git a/kernel.rb b/kernel.rb
index 46d9438..d3fa9d8 100644
--- a/kernel.rb
+++ b/kernel.rb
@@ -1,13 +1,13 @@ https://github.com/ruby/ruby/blob/trunk/kernel.rb#L1
 module Kernel
   #
   #  call-seq:
-  #     obj.clone(freeze: true) -> an_object
+  #     obj.clone(freeze: nil) -> an_object
   #
   #  Produces a shallow copy of <i>obj</i>---the instance variables of
   #  <i>obj</i> are copied, but not the objects they reference.
-  #  #clone copies the frozen (unless +:freeze+ keyword argument is
-  #  given with a false value) state of <i>obj</i>.  See
-  #  also the discussion under Object#dup.
+  #  #clone copies the frozen value state of <i>obj</i>, unless the
+  #  +:freeze+ keyword argument is given with a false or true value.
+  #  See also the discussion under Object#dup.
   #
   #     class Klass
   #        attr_accessor :str
@@ -23,7 +23,7 @@ module Kernel https://github.com/ruby/ruby/blob/trunk/kernel.rb#L23
   #  behavior will be documented under the #+initialize_copy+ method of
   #  the class.
   #
-  def clone(freeze: true)
+  def clone(freeze: nil)
     __builtin_rb_obj_clone2(freeze)
   end
 end
diff --git a/lib/delegate.rb b/lib/delegate.rb
index e3b3488..c540c2e 100644
--- a/lib/delegate.rb
+++ b/lib/delegate.rb
@@ -218,7 +218,7 @@ class Delegator < BasicObject https://github.com/ruby/ruby/blob/trunk/lib/delegate.rb#L218
     end
   end
 
-  def initialize_clone(obj, freeze: true) # :nodoc:
+  def initialize_clone(obj, freeze: nil) # :nodoc:
     self.__setobj__(obj.__getobj__.clone(freeze: freeze))
   end
   def initialize_dup(obj) # :nodoc:
diff --git a/lib/set.rb b/lib/set.rb
index 6841155..b8ab6ab 100644
--- a/lib/set.rb
+++ b/lib/set.rb
@@ -137,7 +137,7 @@ class Set https://github.com/ruby/ruby/blob/trunk/lib/set.rb#L137
   end
 
   # Clone internal hash.
-  def initialize_clone(orig, freeze: true)
+  def initialize_clone(orig, freeze: nil)
     super
     @hash = orig.instance_variable_get(:@hash).clone(freeze: freeze)
   end
diff --git a/object.c b/object.c
index 69a9fd0..a4450f1 100644
--- a/object.c
+++ b/object.c
@@ -369,9 +369,9 @@ init_copy(VALUE dest, VALUE obj) https://github.com/ruby/ruby/blob/trunk/object.c#L369
     }
 }
 
-static int freeze_opt(int argc, VALUE *argv);
-static VALUE immutable_obj_clone(VALUE obj, int kwfreeze);
-static VALUE mutable_obj_clone(VALUE obj, int kwfreeze);
+static VALUE freeze_opt(int argc, VALUE *argv);
+static VALUE immutable_obj_clone(VALUE obj, VALUE kwfreeze);
+static VALUE mutable_obj_clone(VALUE obj, VALUE kwfreeze);
 PUREFUNC(static inline int special_object_p(VALUE obj)); /*!< \private */
 static inline int
 special_object_p(VALUE obj)
@@ -390,21 +390,25 @@ special_object_p(VALUE obj) https://github.com/ruby/ruby/blob/trunk/object.c#L390
     }
 }
 
-static int
+static VALUE
 obj_freeze_opt(VALUE freeze)
 {
-    if (freeze == Qfalse) return FALSE;
-
-    if (freeze != Qtrue)
+    switch(freeze) {
+      case Qfalse:
+      case Qtrue:
+      case Qnil:
+        break;
+      default:
         rb_raise(rb_eArgError, "unexpected value for freeze: %"PRIsVALUE, rb_obj_class(freeze));
+    }
 
-    return TRUE;
+    return freeze;
 }
 
 static VALUE
 rb_obj_clone2(rb_execution_context_t *ec, VALUE obj, VALUE freeze)
 {
-    int kwfreeze = obj_freeze_opt(freeze);
+    VALUE kwfreeze = obj_freeze_opt(freeze);
     if (!special_object_p(obj))
 	return mutable_obj_clone(obj, kwfreeze);
     return immutable_obj_clone(obj, kwfreeze);
@@ -414,17 +418,16 @@ rb_obj_clone2(rb_execution_context_t *ec, VALUE obj, VALUE freeze) https://github.com/ruby/ruby/blob/trunk/object.c#L418
 VALUE
 rb_immutable_obj_clone(int argc, VALUE *argv, VALUE obj)
 {
-    int kwfreeze = freeze_opt(argc, argv);
+    VALUE kwfreeze = freeze_opt(argc, argv);
     return immutable_obj_clone(obj, kwfreeze);
 }
 
-static int
+static VALUE
 freeze_opt(int argc, VALUE *argv)
 {
     static ID keyword_ids[1];
     VALUE opt;
-    VALUE kwfreeze;
-    int ret = 1;
+    VALUE kwfreeze = Qnil;
 
     if (!keyword_ids[0]) {
 	CONST_ID(keyword_ids[0], "freeze");
@@ -432,24 +435,26 @@ freeze_opt(int argc, VALUE *argv) https://github.com/ruby/ruby/blob/trunk/object.c#L435
     rb_scan_args(argc, argv, "0:", &opt);
     if (!NIL_P(opt)) {
 	rb_get_kwargs(opt, keyword_ids, 0, 1, &kwfreeze);
-        if (kwfreeze != Qundef) ret = obj_freeze_opt(kwfreeze);
+        if (kwfreeze != Qundef)
+            kwfreeze = obj_freeze_opt(kwfreeze);
     }
-    return ret;
+    return kwfreeze;
 }
 
 static VALUE
-immutable_obj_clone(VALUE obj, int kwfreeze)
+immutable_obj_clone(VALUE obj, VALUE kwfreeze)
 {
-    if (!kwfreeze)
+    if (kwfreeze == Qfalse)
 	rb_raise(rb_eArgError, "can't unfreeze %"PRIsVALUE,
 		 rb_obj_class(obj));
     return obj;
 }
 
 static VALUE
-mutable_obj_clone(VALUE obj, int kwfreeze)
+mutable_obj_clone(VALUE obj, VALUE kwfreeze)
 {
     VALUE clone, singleton;
+    VALUE argv[2];
 
     clone = rb_obj_alloc(rb_obj_class(obj));
 
@@ -461,23 +466,44 @@ mutable_obj_clone(VALUE obj, int kwfreeze) https://github.com/ruby/ruby/blob/trunk/object.c#L466
 
     init_copy(clone, obj);
 
-    if (kwfreeze) {
+    switch (kwfreeze) {
+      case Qnil:
         rb_funcall(clone, id_init_clone, 1, obj);
 	RBASIC(clone)->flags |= RBASIC(obj)->flags & FL_FREEZE;
-    }
-    else {
+        break;
+      case Qtrue:
+        {
+        static VALUE freeze_true_hash;
+        if (!freeze_true_hash) {
+            freeze_true_hash = rb_hash_new();
+            rb_gc_register_mark_object(freeze_true_hash);
+            rb_hash_aset(freeze_true_hash, ID2SYM(rb_intern("freeze")), Qtrue);
+            rb_obj_freeze(freeze_true_hash);
+        }
+
+        argv[0] = obj;
+        argv[1] = freeze_true_hash;
+        rb_funcallv_kw(clone, id_init_clone, 2, argv, RB_PASS_KEYWORDS);
+        RBASIC(clone)->flags |= FL_FREEZE;
+        break;
+        }
+      case Qfalse:
+        {
         static VALUE freeze_false_hash;
-        VALUE argv[2];
         if (!freeze_false_hash) {
             freeze_false_hash = rb_hash_new();
+            rb_gc_register_mark_object(freeze_false_hash);
             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_KEYWORDS);
+        break;
+        }
+      default:
+        rb_bug("invalid kwfreeze passed to mutable_obj_clone");
     }
 
     return clone;
@@ -493,7 +519,7 @@ VALUE https://github.com/ruby/ruby/blob/trunk/object.c#L519
 rb_obj_clone(VALUE obj)
 {
     if (special_object_p(obj)) return obj;
-    return mutable_obj_clone(obj, Qtrue);
+    return mutable_obj_clone(obj, Qnil);
 }
 
 /**
diff --git a/spec/ruby/core/kernel/clone_spec.rb b/spec/ruby/core/kernel/clone_spec.rb
index f20ea61..1bbef64 100644
--- a/spec/ruby/core/kernel/clone_spec.rb
+++ b/spec/ruby/core/kernel/clone_spec.rb
@@ -37,11 +37,17 @@ describe "Kernel#clone" do https://github.com/ruby/ruby/blob/trunk/spec/ruby/core/kernel/clone_spec.rb#L37
     o3.frozen?.should == true
   end
 
-  it 'takes an option to copy freeze state or not' do
-    @obj.clone(freeze: true).frozen?.should == false
+  ruby_version_is '2.8' do
+    it 'takes an freeze: true option to frozen copy' do
+      @obj.clone(freeze: true).frozen?.should == true
+      @obj.freeze
+      @obj.clone(freeze: true).frozen?.should == true
+    end
+  end
+
+  it 'takes an freeze: false option to not return frozen copy' do
     @obj.clone(freeze: false).frozen?.should == false
     @obj.freeze
-    @obj.clone(freeze: true).frozen?.should == true
     @obj.clone(freeze: false).frozen?.should == false
   end
 
diff --git a/test/ruby/test_object.rb b/test/ruby/test_object.rb
index 55709bf..3aa0a1b 100644
--- a/test/ruby/test_object.rb
+++ b/test/ruby/test_object.rb
@@ -47,15 +47,27 @@ class TestObject < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_object.rb#L47
     a = Object.new
     def a.b; 2 end
 
+    c = a.clone
+    assert_equal(false, c.frozen?)
+    assert_equal(false, a.frozen?)
+    assert_equal(2, c.b)
+
+    c = a.clone(freeze: true)
+    assert_equal(true, c.frozen?)
+    assert_equal(false, a.frozen?)
+    assert_equal(2, c.b)
+
     a.freeze
     c = a.clone
     assert_equal(true, c.frozen?)
+    assert_equal(true, a.frozen?)
     assert_equal(2, c.b)
 
     assert_raise(ArgumentError) {a.clone(freeze: [])}
     d = a.clone(freeze: false)
     def d.e; 3; end
     assert_equal(false, d.frozen?)
+    assert_equal(true, a.frozen?)
     assert_equal(2, d.b)
     assert_equal(3, d.e)
 
-- 
cgit v0.10.2


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

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