From b67711b17a40c839af1415853d56bdf9ce82a142 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Thu, 8 May 2025 20:24:17 +0200 Subject: [PATCH] Fix `remove_instance_variable` on complex objects Introduced in: https://github.com/ruby/ruby/pull/13159 Now that there is no longer a unique TOO_COMPLEX shape with no children, checking `shape->type == TOO_COMPLEX` is incorrect. --- shape.c | 10 +++++----- test/ruby/test_shapes.rb | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/shape.c b/shape.c index fb739d3614..16eb5b50f4 100644 --- a/shape.c +++ b/shape.c @@ -618,13 +618,13 @@ remove_shape_recursive(rb_shape_t *shape, ID id, rb_shape_t **removed_shape) // We found a new parent. Create a child of the new parent that // has the same attributes as this shape. if (new_parent) { - if (UNLIKELY(new_parent->type == SHAPE_OBJ_TOO_COMPLEX)) { + if (UNLIKELY(rb_shape_too_complex_p(new_parent))) { return new_parent; } bool dont_care; rb_shape_t *new_child = get_next_shape_internal(new_parent, shape->edge_name, shape->type, &dont_care, true); - if (UNLIKELY(new_child->type == SHAPE_OBJ_TOO_COMPLEX)) { + if (UNLIKELY(rb_shape_too_complex_p(new_child))) { return new_child; } @@ -644,7 +644,7 @@ remove_shape_recursive(rb_shape_t *shape, ID id, rb_shape_t **removed_shape) bool rb_shape_transition_shape_remove_ivar(VALUE obj, ID id, rb_shape_t *shape, VALUE *removed) { - if (UNLIKELY(shape->type == SHAPE_OBJ_TOO_COMPLEX)) { + if (UNLIKELY(rb_shape_too_complex_p(shape))) { return false; } @@ -653,7 +653,7 @@ rb_shape_transition_shape_remove_ivar(VALUE obj, ID id, rb_shape_t *shape, VALUE if (new_shape) { RUBY_ASSERT(removed_shape != NULL); - if (UNLIKELY(new_shape->type == SHAPE_OBJ_TOO_COMPLEX)) { + if (UNLIKELY(rb_shape_too_complex_p(new_shape))) { return false; } @@ -797,7 +797,7 @@ static inline rb_shape_t * shape_get_next(rb_shape_t *shape, VALUE obj, ID id, bool emit_warnings) { RUBY_ASSERT(!is_instance_id(id) || RTEST(rb_sym2str(ID2SYM(id)))); - if (UNLIKELY(shape->type == SHAPE_OBJ_TOO_COMPLEX)) { + if (UNLIKELY(rb_shape_too_complex_p(shape))) { return shape; } diff --git a/test/ruby/test_shapes.rb b/test/ruby/test_shapes.rb index d37448f16f..94670a4055 100644 --- a/test/ruby/test_shapes.rb +++ b/test/ruby/test_shapes.rb @@ -788,9 +788,42 @@ class TestShapes < Test::Unit::TestCase assert_equal 3, tc.a3_m # make sure IV is initialized assert tc.instance_variable_defined?(:@a3) tc.remove_instance_variable(:@a3) + refute tc.instance_variable_defined?(:@a3) assert_nil tc.a3 end + def test_delete_iv_after_complex_and_object_id + ensure_complex + + tc = TooComplex.new + tc.send("a#{RubyVM::Shape::SHAPE_MAX_VARIATIONS}_m") + assert_predicate RubyVM::Shape.of(tc), :too_complex? + + assert_equal 3, tc.a3_m # make sure IV is initialized + assert tc.instance_variable_defined?(:@a3) + tc.object_id + tc.remove_instance_variable(:@a3) + refute tc.instance_variable_defined?(:@a3) + assert_nil tc.a3 + end + + def test_delete_iv_after_complex_and_freeze + ensure_complex + + tc = TooComplex.new + tc.send("a#{RubyVM::Shape::SHAPE_MAX_VARIATIONS}_m") + assert_predicate RubyVM::Shape.of(tc), :too_complex? + + assert_equal 3, tc.a3_m # make sure IV is initialized + assert tc.instance_variable_defined?(:@a3) + tc.freeze + assert_raise FrozenError do + tc.remove_instance_variable(:@a3) + end + assert tc.instance_variable_defined?(:@a3) + assert_equal 3, tc.a3 + end + def test_delete_undefined_after_complex ensure_complex