1berry/ruby
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix class instance variable inside namespaces

Browse Source 
Now that classes fields are delegated to an object with its own
shape_id, we no longer need to mark all classes as TOO_COMPLEX.
This commit is contained in:
Jean Boussier 2025-06-12 12:02:56 +02:00
parent 8120971932
commit 8b5ac5abf2
Notes: git 2025-06-12 11:43:42 +00:00 
Merged: https://github.com/ruby/ruby/pull/13595
8 changed files with 71 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
imemo.c
View File

@ -155,11 +155,14 @@ rb_imemo_class_fields_clone(VALUE fields_obj)
if (rb_shape_too_complex_p(shape_id)) {
clone = rb_imemo_class_fields_new_complex(CLASS_OF(fields_obj), 0);
RBASIC_SET_SHAPE_ID(clone, shape_id);
st_table *src_table = rb_imemo_class_fields_complex_tbl(fields_obj);
st_replace(rb_imemo_class_fields_complex_tbl(clone), src_table);
}
else {
clone = imemo_class_fields_new(CLASS_OF(fields_obj), RSHAPE_CAPACITY(shape_id));
RBASIC_SET_SHAPE_ID(clone, shape_id);
MEMCPY(rb_imemo_class_fields_ptr(clone), rb_imemo_class_fields_ptr(fields_obj), VALUE, RSHAPE_LEN(shape_id));
}

36
internal/class.h
View File

@ -403,10 +403,6 @@ RCLASS_EXT_WRITABLE_LOOKUP(VALUE obj, const rb_namespace_t *ns)
if (ext)
return ext;
if (!rb_shape_obj_too_complex_p(obj)) {
rb_evict_ivars_to_hash(obj); // fallback to ivptr for ivars from shapes
}
RB_VM_LOCKING() {
// re-check the classext is not created to avoid the multi-thread race
ext = RCLASS_EXT_TABLE_LOOKUP_INTERNAL(obj, ns);
@ -525,23 +521,23 @@ RCLASS_WRITE_SUPER(VALUE klass, VALUE super)
}
static inline VALUE
RCLASS_FIELDS_OBJ(VALUE obj)
RCLASS_WRITABLE_ENSURE_FIELDS_OBJ(VALUE obj)
{
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
return RCLASSEXT_FIELDS_OBJ(RCLASS_EXT_READABLE(obj));
}
static inline VALUE
RCLASS_ENSURE_FIELDS_OBJ(VALUE obj)
{
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
rb_classext_t *ext = RCLASS_EXT_READABLE(obj);
rb_classext_t *ext = RCLASS_EXT_WRITABLE(obj);
if (!ext->fields_obj) {
RB_OBJ_WRITE(obj, &ext->fields_obj, rb_imemo_class_fields_new(obj, 1));
}
return ext->fields_obj;
}
static inline void
RCLASSEXT_SET_FIELDS_OBJ(VALUE obj, rb_classext_t *ext, VALUE fields_obj)
{
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
RB_OBJ_WRITE(obj, &ext->fields_obj, fields_obj);
}
static inline VALUE
RCLASS_WRITABLE_FIELDS_OBJ(VALUE obj)
{
@ -550,25 +546,19 @@ RCLASS_WRITABLE_FIELDS_OBJ(VALUE obj)
}
static inline void
RCLASSEXT_SET_FIELDS_OBJ(VALUE obj, rb_classext_t *ext, VALUE fields_obj)
{
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
RB_OBJ_WRITE(obj, &ext->fields_obj, fields_obj);
}
static inline void
RCLASS_SET_FIELDS_OBJ(VALUE obj, VALUE fields_obj)
RCLASS_WRITABLE_SET_FIELDS_OBJ(VALUE obj, VALUE fields_obj)
{
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
RCLASSEXT_SET_FIELDS_OBJ(obj, RCLASS_EXT_PRIME(obj), fields_obj);
RCLASSEXT_SET_FIELDS_OBJ(obj, RCLASS_EXT_WRITABLE(obj), fields_obj);
}
static inline uint32_t
RCLASS_FIELDS_COUNT(VALUE obj)
{
RUBY_ASSERT(RB_TYPE_P(obj, RUBY_T_CLASS) || RB_TYPE_P(obj, RUBY_T_MODULE));
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
if (fields_obj) {
if (rb_shape_obj_too_complex_p(fields_obj)) {
return (uint32_t)rb_st_table_size(rb_imemo_class_fields_complex_tbl(fields_obj));

3
namespace.c
View File

@ -450,9 +450,6 @@ namespace_initialize(VALUE namespace)
// If a code in the namespace adds a constant, the constant will be visible even from root/main.
RCLASS_SET_PRIME_CLASSEXT_WRITABLE(namespace, true);
// fallback to ivptr for ivars from shapes to manipulate the constant table
rb_evict_ivars_to_hash(namespace);
// Get a clean constant table of Object even by writable one
// because ns was just created, so it has not touched any constants yet.
object_classext = RCLASS_EXT_WRITABLE_IN_NS(rb_cObject, ns);

2
shape.c
View File

@ -397,7 +397,7 @@ rb_obj_shape_id(VALUE obj)
}
if (BUILTIN_TYPE(obj) == T_CLASS || BUILTIN_TYPE(obj) == T_MODULE) {
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
if (fields_obj) {
return RBASIC_SHAPE_ID(fields_obj);
}

21
test/ruby/namespace/instance_variables.rb Normal file
View File

@ -0,0 +1,21 @@
class String
class << self
attr_reader :str_ivar1
def str_ivar2
@str_ivar2
end
end
@str_ivar1 = 111
@str_ivar2 = 222
end
class StringDelegator < BasicObject
private
def method_missing(...)
::String.public_send(...)
end
end
StringDelegatorObj = StringDelegator.new

20
test/ruby/test_namespace.rb
View File

@ -222,6 +222,26 @@ class TestNamespace < Test::Unit::TestCase
end;
end
def test_instance_variable
pend unless Namespace.enabled?
@n.require_relative('namespace/instance_variables')
assert_equal [], String.instance_variables
assert_equal [:@str_ivar1, :@str_ivar2], @n::StringDelegatorObj.instance_variables
assert_equal 111, @n::StringDelegatorObj.str_ivar1
assert_equal 222, @n::StringDelegatorObj.str_ivar2
assert_equal 222, @n::StringDelegatorObj.instance_variable_get(:@str_ivar2)
@n::StringDelegatorObj.instance_variable_set(:@str_ivar3, 333)
assert_equal 333, @n::StringDelegatorObj.instance_variable_get(:@str_ivar3)
@n::StringDelegatorObj.remove_instance_variable(:@str_ivar1)
assert_nil @n::StringDelegatorObj.str_ivar1
assert_equal [:@str_ivar2, :@str_ivar3], @n::StringDelegatorObj.instance_variables
assert_equal [], String.instance_variables
end
def test_methods_added_in_namespace_are_invisible_globally
pend unless Namespace.enabled?

25
variable.c
View File

@ -1307,7 +1307,7 @@ rb_obj_field_get(VALUE obj, shape_id_t target_shape_id)
if (BUILTIN_TYPE(obj) == T_CLASS || BUILTIN_TYPE(obj) == T_MODULE) {
ASSERT_vm_locking();
VALUE field_obj = RCLASS_FIELDS_OBJ(obj);
VALUE field_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
if (field_obj) {
return rb_obj_field_get(field_obj, target_shape_id);
}
@ -1375,7 +1375,7 @@ rb_ivar_lookup(VALUE obj, ID id, VALUE undef)
VALUE val = undef;
RB_VM_LOCK_ENTER();
{
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
if (fields_obj) {
val = rb_ivar_lookup(fields_obj, id, undef);
}
@ -1492,7 +1492,7 @@ rb_ivar_delete(VALUE obj, ID id, VALUE undef)
if (BUILTIN_TYPE(obj) == T_CLASS || BUILTIN_TYPE(obj) == T_MODULE) {
IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(id);
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
if (fields_obj) {
RB_VM_LOCK_ENTER();
{
@ -1614,8 +1614,7 @@ static shape_id_t
obj_transition_too_complex(VALUE obj, st_table *table)
{
if (BUILTIN_TYPE(obj) == T_CLASS || BUILTIN_TYPE(obj) == T_MODULE) {
RUBY_ASSERT(RCLASS_FIELDS_OBJ(obj));
return obj_transition_too_complex(RCLASS_FIELDS_OBJ(obj), table);
return obj_transition_too_complex(RCLASS_WRITABLE_ENSURE_FIELDS_OBJ(obj), table);
}
RUBY_ASSERT(!rb_shape_obj_too_complex_p(obj));
@ -2062,7 +2061,7 @@ rb_obj_set_shape_id(VALUE obj, shape_id_t shape_id)
if (BUILTIN_TYPE(obj) == T_CLASS || BUILTIN_TYPE(obj) == T_MODULE) {
// Avoid creating the fields_obj just to freeze the class
if (!(shape_id == SPECIAL_CONST_SHAPE_ID && old_shape_id == ROOT_SHAPE_ID)) {
RBASIC_SET_SHAPE_ID(RCLASS_ENSURE_FIELDS_OBJ(obj), shape_id);
RBASIC_SET_SHAPE_ID(RCLASS_WRITABLE_ENSURE_FIELDS_OBJ(obj), shape_id);
}
}
// FIXME: How to do multi-shape?
@ -2201,7 +2200,7 @@ rb_ivar_defined(VALUE obj, ID id)
case T_CLASS:
case T_MODULE:
RB_VM_LOCKING() {
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
if (fields_obj) {
defined = ivar_defined0(fields_obj, id);
}
@ -2455,7 +2454,7 @@ rb_field_foreach(VALUE obj, rb_ivar_foreach_callback_func *func, st_data_t arg,
case T_MODULE:
IVAR_ACCESSOR_SHOULD_BE_MAIN_RACTOR(0);
RB_VM_LOCKING() {
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
if (fields_obj) {
class_fields_each(fields_obj, func, arg, ivar_only);
}
@ -2488,7 +2487,7 @@ rb_ivar_count(VALUE obj)
case T_CLASS:
case T_MODULE:
{
VALUE fields_obj = RCLASS_FIELDS_OBJ(obj);
VALUE fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
if (!fields_obj) {
return 0;
}
@ -4706,7 +4705,7 @@ static int
class_ivar_set(VALUE obj, ID id, VALUE val)
{
bool existing = true;
const VALUE original_fields_obj = RCLASS_FIELDS_OBJ(obj);
const VALUE original_fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
VALUE fields_obj = original_fields_obj ? original_fields_obj : rb_imemo_class_fields_new(obj, 1);
shape_id_t next_shape_id = 0;
@ -4758,7 +4757,7 @@ class_ivar_set(VALUE obj, ID id, VALUE val)
}
if (fields_obj != original_fields_obj) {
RCLASS_SET_FIELDS_OBJ(obj, fields_obj);
RCLASS_WRITABLE_SET_FIELDS_OBJ(obj, fields_obj);
// TODO: What should we set as the T_CLASS shape_id?
// In most case we can replicate the single `fields_obj` shape
// but in namespaced case?
@ -4777,7 +4776,7 @@ too_complex:
if (fields_obj != original_fields_obj) {
RBASIC_SET_SHAPE_ID(fields_obj, next_shape_id);
RCLASS_SET_FIELDS_OBJ(obj, fields_obj);
RCLASS_WRITABLE_SET_FIELDS_OBJ(obj, fields_obj);
// TODO: What should we set as the T_CLASS shape_id?
// In most case we can replicate the single `fields_obj` shape
// but in namespaced case?
@ -4809,7 +4808,7 @@ static void
class_field_set(VALUE obj, shape_id_t target_shape_id, VALUE val)
{
RUBY_ASSERT(RB_TYPE_P(obj, T_CLASS) || RB_TYPE_P(obj, T_MODULE));
obj_field_set(RCLASS_ENSURE_FIELDS_OBJ(obj), target_shape_id, val);
obj_field_set(RCLASS_WRITABLE_ENSURE_FIELDS_OBJ(obj), target_shape_id, val);
}
static int

2
vm_insnhelper.c
View File

@ -1248,7 +1248,7 @@ vm_getivar(VALUE obj, ID id, const rb_iseq_t *iseq, IVC ic, const struct rb_call
}
}
fields_obj = RCLASS_FIELDS_OBJ(obj);
fields_obj = RCLASS_WRITABLE_FIELDS_OBJ(obj);
if (!fields_obj) {
return default_value;
}