summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornagachika <nagachika@ruby-lang.org>2021-03-20 14:26:30 +0900
committernagachika <nagachika@ruby-lang.org>2021-03-20 14:26:30 +0900
commit82d72f14e7071218f3fd710f770d1ba31390f027 (patch)
tree5d5cad384cba513f46290df64835aafe7fb7c943
parent6ef46f71c743507a0e2ae0eef14dce0539b0ff52 (diff)
merge revision(s) ebb96fa8808317ad53a4977bff26cf755d68077e: [Backport #17321]
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> --- class.c | 31 ++++++++++++++++++++++--------- test/ruby/test_class.rb | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 9 deletions(-)
-rw-r--r--class.c31
-rw-r--r--test/ruby/test_class.rb47
-rw-r--r--version.h2
3 files changed, 70 insertions, 10 deletions
diff --git a/class.c b/class.c
index c866d1d727..2e4b31d7c6 100644
--- a/class.c
+++ b/class.c
@@ -32,6 +32,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)
{
@@ -372,22 +375,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));
@@ -411,7 +427,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;
@@ -433,11 +451,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)
diff --git a/test/ruby/test_class.rb b/test/ruby/test_class.rb
index ca78473026..46485c4fd2 100644
--- a/test/ruby/test_class.rb
+++ b/test/ruby/test_class.rb
@@ -483,6 +483,53 @@ class TestClass < Test::Unit::TestCase
assert_equal(:foo, d.foo)
end
+ def test_clone_singleton_class_exists
+ klass = Class.new do
+ def self.bar; :bar; end
+ end
+
+ o = klass.new
+ o.singleton_class
+ clone = o.clone
+
+ assert_empty(o.singleton_class.instance_methods(false))
+ assert_empty(clone.singleton_class.instance_methods(false))
+ assert_empty(o.singleton_class.singleton_class.instance_methods(false))
+ assert_empty(clone.singleton_class.singleton_class.instance_methods(false))
+ end
+
+ def test_clone_when_singleton_class_of_singleton_class_exists
+ klass = Class.new do
+ def self.bar; :bar; end
+ end
+
+ o = klass.new
+ o.singleton_class.singleton_class
+ clone = o.clone
+
+ assert_empty(o.singleton_class.instance_methods(false))
+ assert_empty(clone.singleton_class.instance_methods(false))
+ assert_empty(o.singleton_class.singleton_class.instance_methods(false))
+ assert_empty(clone.singleton_class.singleton_class.instance_methods(false))
+ end
+
+ def test_clone_when_method_exists_on_singleton_class_of_singleton_class
+ klass = Class.new do
+ def self.bar; :bar; end
+ end
+
+ o = klass.new
+ o.singleton_class.singleton_class.define_method(:s2_method) { :s2 }
+ clone = o.clone
+
+ assert_empty(o.singleton_class.instance_methods(false))
+ assert_empty(clone.singleton_class.instance_methods(false))
+ assert_equal(:s2, o.singleton_class.s2_method)
+ assert_equal(:s2, clone.singleton_class.s2_method)
+ assert_equal([:s2_method], o.singleton_class.singleton_class.instance_methods(false))
+ assert_equal([:s2_method], clone.singleton_class.singleton_class.instance_methods(false))
+ end
+
def test_singleton_class_p
feature7609 = '[ruby-core:51087] [Feature #7609]'
assert_predicate(self.singleton_class, :singleton_class?, feature7609)
diff --git a/version.h b/version.h
index b5c6533881..c0e2f3f081 100644
--- a/version.h
+++ b/version.h
@@ -2,7 +2,7 @@
# define RUBY_VERSION_MINOR RUBY_API_VERSION_MINOR
#define RUBY_VERSION_TEENY 3
#define RUBY_RELEASE_DATE RUBY_RELEASE_YEAR_STR"-"RUBY_RELEASE_MONTH_STR"-"RUBY_RELEASE_DAY_STR
-#define RUBY_PATCHLEVEL 168
+#define RUBY_PATCHLEVEL 169
#define RUBY_RELEASE_YEAR 2021
#define RUBY_RELEASE_MONTH 3