From 4f7b435c955792af780fe9e94f98d3dde839e056 Mon Sep 17 00:00:00 2001 From: Jeremy Evans 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] --- kernel.rb | 10 ++--- lib/delegate.rb | 2 +- lib/set.rb | 2 +- object.c | 74 +++++++++++++++++++++++++------------ spec/ruby/core/kernel/clone_spec.rb | 12 ++++-- test/ruby/test_object.rb | 12 ++++++ 6 files changed, 78 insertions(+), 34 deletions(-) diff --git a/kernel.rb b/kernel.rb index 46d9438f85..d3fa9d8053 100644 --- a/kernel.rb +++ b/kernel.rb @@ -1,13 +1,13 @@ module Kernel # # call-seq: - # obj.clone(freeze: true) -> an_object + # obj.clone(freeze: nil) -> an_object # # Produces a shallow copy of obj---the instance variables of # obj are copied, but not the objects they reference. - # #clone copies the frozen (unless +:freeze+ keyword argument is - # given with a false value) state of obj. See - # also the discussion under Object#dup. + # #clone copies the frozen value state of obj, 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 # 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 e3b3488c16..c540c2ec77 100644 --- a/lib/delegate.rb +++ b/lib/delegate.rb @@ -218,7 +218,7 @@ class Delegator < BasicObject 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 684115539b..b8ab6ab172 100644 --- a/lib/set.rb +++ b/lib/set.rb @@ -137,7 +137,7 @@ class Set 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 69a9fd0bf1..a4450f17f1 100644 --- a/object.c +++ b/object.c @@ -369,9 +369,9 @@ init_copy(VALUE dest, VALUE obj) } } -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) } } -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) 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) 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) 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 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 f20ea618b5..1bbef64f85 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 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 55709bf3dc..3aa0a1b652 100644 --- a/test/ruby/test_object.rb +++ b/test/ruby/test_object.rb @@ -47,15 +47,27 @@ class TestObject < Test::Unit::TestCase 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 v1.2.3