diff options
author | Alan Wu <XrXr@users.noreply.github.com> | 2020-11-11 16:38:03 -0500 |
---|---|---|
committer | Alan Wu <XrXr@users.noreply.github.com> | 2020-11-16 17:41:17 -0500 |
commit | ebb96fa8808317ad53a4977bff26cf755d68077e (patch) | |
tree | b7e3dd43bbc16ce71235b773df598f8138cb1065 /class.c | |
parent | 084e7e31b257f6ac859769553b4c1c6ca2930152 (diff) |
Fix singleton class cloning
Before this commit, `clone` gave different results depending on whether the original object
had an attached singleton class or not.
Consider the following setup:
```
class Foo; end
Foo.singleton_class.define_method(:foo) {}
obj = Foo.new
obj.singleton_class if $call_singleton
clone = obj.clone
```
When `$call_singleton = false`, neither `obj.singleton_class.singleton_class` nor
`clone.singleton_class.singleton_class` own any methods.
However, when `$call_singleton = true`, `clone.singleton_class.singleton_class` would own a copy of
`foo` from `Foo.singleton_class`, even though `obj.singleton_class.singleton_class` does not.
The latter case is unexpected and results in a visibly different clone, depending on if the original object
had an attached class or not.
Co-authored-by: Ufuk Kayserilioglu <ufuk.kayserilioglu@shopify.com>
Notes
Notes:
Merged: https://github.com/ruby/ruby/pull/3761
Diffstat (limited to 'class.c')
-rw-r--r-- | class.c | 31 |
1 files changed, 22 insertions, 9 deletions
@@ -40,6 +40,9 @@ #define id_attached id__attached__ +#define METACLASS_OF(k) RBASIC(k)->klass +#define SET_METACLASS_OF(k, cls) RBASIC_SET_CLASS(k, cls) + void rb_class_subclass_add(VALUE super, VALUE klass) { @@ -457,22 +460,35 @@ rb_singleton_class_clone(VALUE obj) return rb_singleton_class_clone_and_attach(obj, Qundef); } +// Clone and return the singleton class of `obj` if it has been created and is attached to `obj`. VALUE rb_singleton_class_clone_and_attach(VALUE obj, VALUE attach) { const VALUE klass = RBASIC(obj)->klass; - if (!FL_TEST(klass, FL_SINGLETON)) - return klass; + // Note that `rb_singleton_class()` can create situations where `klass` is + // attached to an object other than `obj`. In which case `obj` does not have + // a material singleton class attached yet and there is no singleton class + // to clone. + if (!(FL_TEST(klass, FL_SINGLETON) && rb_attr_get(klass, id_attached) == obj)) { + // nothing to clone + return klass; + } else { /* copy singleton(unnamed) class */ + bool klass_of_clone_is_new; VALUE clone = class_alloc(RBASIC(klass)->flags, 0); if (BUILTIN_TYPE(obj) == T_CLASS) { + klass_of_clone_is_new = true; RBASIC_SET_CLASS(clone, clone); } else { - RBASIC_SET_CLASS(clone, rb_singleton_class_clone(klass)); + VALUE klass_metaclass_clone = rb_singleton_class_clone(klass); + // When `METACLASS_OF(klass) == klass_metaclass_clone`, it means the + // recursive call did not clone `METACLASS_OF(klass)`. + klass_of_clone_is_new = (METACLASS_OF(klass) != klass_metaclass_clone); + RBASIC_SET_CLASS(clone, klass_metaclass_clone); } RCLASS_SET_SUPER(clone, RCLASS_SUPER(klass)); @@ -496,7 +512,9 @@ rb_singleton_class_clone_and_attach(VALUE obj, VALUE attach) arg.new_klass = clone; rb_id_table_foreach(RCLASS_M_TBL(klass), clone_method_i, &arg); } - rb_singleton_class_attached(RBASIC(clone)->klass, clone); + if (klass_of_clone_is_new) { + rb_singleton_class_attached(RBASIC(clone)->klass, clone); + } FL_SET(clone, FL_SINGLETON); return clone; @@ -518,11 +536,6 @@ rb_singleton_class_attached(VALUE klass, VALUE obj) } } - - -#define METACLASS_OF(k) RBASIC(k)->klass -#define SET_METACLASS_OF(k, cls) RBASIC_SET_CLASS(k, cls) - /*! * whether k is a meta^(n)-class of Class class * @retval 1 if \a k is a meta^(n)-class of Class class (n >= 0) |