mirror of
https://github.com/ruby/ruby.git
synced 2022-11-09 12:17:21 -05:00
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]
This commit is contained in:
parent
0eeed5bcc5
commit
04eb7c7e46
Notes:
git
2020-01-04 13:13:48 +09:00
7 changed files with 102 additions and 12 deletions
|
@ -211,8 +211,8 @@ class Delegator < BasicObject
|
|||
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)
|
||||
|
|
|
@ -137,9 +137,9 @@ class Set
|
|||
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:
|
||||
|
|
52
object.c
52
object.c
|
@ -476,11 +476,25 @@ mutable_obj_clone(VALUE obj, int kwfreeze)
|
|||
}
|
||||
|
||||
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)
|
|||
/*!
|
||||
* :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)
|
|||
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)
|
|||
|
||||
/* :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)
|
|||
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)
|
|||
|
||||
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 */
|
||||
|
|
|
@ -2491,6 +2491,12 @@ class TestModule < Test::Unit::TestCase
|
|||
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)
|
||||
|
|
|
@ -75,6 +75,28 @@ class TestObject < Test::Unit::TestCase
|
|||
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
|
||||
|
|
|
@ -50,6 +50,21 @@ class TestDelegateClass < Test::Unit::TestCase
|
|||
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
|
||||
|
|
|
@ -730,6 +730,17 @@ class TC_Set < Test::Unit::TestCase
|
|||
}
|
||||
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)
|
||||
|
|
Loading…
Reference in a new issue