From 494d55c9f7f53c3af8840d86f8802ff03c0ef9f4 Mon Sep 17 00:00:00 2001 From: "Yukihiro \"Matz\" Matsumoto" Date: Thu, 19 Feb 2026 14:57:50 +0900 Subject: [PATCH] class.c: use tombstone for remove_method with ROM method tables Previously, removing a ROM method required flattening all chain layers into a single mutable table. This was O(n) and allocated RAM for all previously-ROM methods. Use a tombstone marker (MT_FUNC flag with func=NULL) instead. The mt_get() lookup treats this as "not found" and stops the chain walk, hiding the ROM entry while allowing superclass lookup. Co-authored-by: Claude --- doc/guides/rom-method-table.md | 15 ++++-- include/mruby/internal.h | 7 +++ src/class.c | 89 +++++++++++++++------------------- 3 files changed, 56 insertions(+), 55 deletions(-) diff --git a/doc/guides/rom-method-table.md b/doc/guides/rom-method-table.md index 32f247935..3f814c8a8 100644 --- a/doc/guides/rom-method-table.md +++ b/doc/guides/rom-method-table.md @@ -325,10 +325,17 @@ end ### Method Removal -`remove_method` and `undef_method` work on ROM methods. If the target -method is only in a ROM layer, the chain is flattened into a single -mutable table first, then the entry is deleted. This is an O(n) -operation but is extremely rare for built-in methods. +`remove_method` works on ROM methods using a tombstone marker. When a +method in a ROM layer is removed, a special entry (`MT_FUNC` flag with +`func=NULL`) is inserted into the mutable layer. The `mt_get()` lookup +treats this marker as "not found" and stops searching the chain, +effectively hiding the ROM entry. Unlike `undef_method` (which blocks +superclass lookup), `remove_method`'s tombstone allows the superclass +method to be found. + +`undef_method` uses a different tombstone (`proc=NULL` without +`MT_FUNC`), which is returned by `mt_get()` so the caller raises +NoMethodError without searching the superclass. ### Class Duplication diff --git a/include/mruby/internal.h b/include/mruby/internal.h index 235757996..7048197ff 100644 --- a/include/mruby/internal.h +++ b/include/mruby/internal.h @@ -49,6 +49,13 @@ typedef struct mt_tbl { #define MT_PUBLIC 0 /* MRB_METHOD_PUBLIC_FL */ #define MT_PRIVATE 1 /* MRB_METHOD_PRIVATE_FL */ +/* "removed" tombstone: MT_FUNC flag set with NULL function pointer. + This combination never occurs naturally (C functions are never NULL). + Unlike undef (proc=NULL without MT_FUNC), a removed marker makes + mt_get() return 0 ("not found"), blocking ROM chain walk while + allowing superclass lookup. */ +#define MT_REMOVED_P(key, val) (((key)&MT_FUNC) && (val).func==NULL) + void mrb_mt_init_rom(struct RClass *c, mt_tbl *rom); #endif diff --git a/src/class.c b/src/class.c index a4e8ac563..d744938a3 100644 --- a/src/class.c +++ b/src/class.c @@ -162,6 +162,7 @@ mt_get(mrb_state *mrb, mt_tbl *t, mrb_sym sym, union mt_ptr *pp) union mt_ptr *vals = mt_vals(t); int lo = mt_bsearch_idx(keys, t->size, sym); if (lo < t->size && MT_KEY_SYM(keys[lo]) == sym) { + if (MT_REMOVED_P(keys[lo], vals[lo])) return 0; /* removed tombstone */ *pp = vals[lo]; return keys[lo]; } @@ -213,36 +214,6 @@ mt_chain_has(mt_tbl *t, mrb_sym sym) return FALSE; } -/* Flattens all chain layers into a single mutable table */ -static void -mt_flatten(mrb_state *mrb, struct RClass *c) -{ - mt_tbl *t = c->mt; - if (!t || (!t->next && !mt_readonly_p(t))) return; - - mt_tbl *flat = mt_new(mrb); - /* collect all layers into an array, then iterate backwards */ - mt_tbl *layers[16]; - int n = 0; - for (mt_tbl *l = t; l && n < 16; l = l->next) - layers[n++] = l; - for (int i = n - 1; i >= 0; i--) { - mt_tbl *l = layers[i]; - mrb_sym *keys = mt_keys(l); - union mt_ptr *vals = mt_vals(l); - for (int j = 0; j < l->size; j++) - mt_put(mrb, flat, MT_KEY_SYM(keys[j]), MT_KEY_FLG(keys[j]), vals[j]); - } - /* free only mutable layers */ - for (mt_tbl *l = t; l && !mt_readonly_p(l); ) { - mt_tbl *next = l->next; - mrb_free(mrb, l->ptr); - mrb_free(mrb, l); - l = next; - } - c->mt = flat; -} - /* Creates a copy of the method table */ static mt_tbl* mt_copy(mrb_state *mrb, mt_tbl *t) @@ -339,6 +310,7 @@ mrb_mt_foreach(mrb_state *mrb, struct RClass *c, mrb_mt_foreach_func *fn, void * for (int i = 0; i < t->size; i++) { union mt_ptr *vals = mt_vals(t); mrb_sym *keys = mt_keys(t); + if (MT_REMOVED_P(keys[i], vals[i])) continue; if (fn(mrb, MT_KEY_SYM(keys[i]), create_method_value(mrb, keys[i], vals[i]), p) != 0) return; @@ -351,6 +323,7 @@ mrb_mt_foreach(mrb_state *mrb, struct RClass *c, mrb_mt_foreach_func *fn, void * mrb_sym *keys = mt_keys(layer); union mt_ptr *vals = mt_vals(layer); for (int i = 0; i < layer->size; i++) { + if (MT_REMOVED_P(keys[i], vals[i])) continue; mrb_sym sym = MT_KEY_SYM(keys[i]); /* check if shadowed by a higher layer */ if (layer != t) { @@ -3670,31 +3643,45 @@ MRB_API void mrb_remove_method(mrb_state *mrb, struct RClass *c0, mrb_sym mid) { struct RClass *c = c0; + mrb_bool found = FALSE; + MRB_CLASS_ORIGIN(c); mt_tbl *h = c->mt; - - if ((h && mt_del(mrb, h, mid)) || - (h && h->next && mt_chain_has(h->next, mid) && - (mt_flatten(mrb, c), mt_del(mrb, c->mt, mid)))) { - mrb_sym removed; - mrb_value recv; - - mc_clear_by_id(mrb, mid); - if (c0->tt == MRB_TT_SCLASS) { - removed = MRB_SYM(singleton_method_removed); - recv = mrb_iv_get(mrb, mrb_obj_value(c0), MRB_SYM(__attached__)); + if (h) { + found = mt_del(mrb, h, mid); + /* insert removed tombstone to block ROM chain lookup */ + if (h->next && mt_chain_has(h->next, mid)) { + union mt_ptr tombstone; + tombstone.func = NULL; + found = TRUE; + if (mt_readonly_p(h)) { + mt_tbl *top = mt_new(mrb); + top->next = h; + h = c->mt = top; + } + mt_put(mrb, h, mid, MT_FUNC, tombstone); + } + } + if (!found) { + mrb_name_error(mrb, mid, "method '%n' not defined in %C", mid, c); + } + mc_clear_by_id(mrb, mid); + if (c0->tt == MRB_TT_SCLASS) { + mrb_sym cb = MRB_SYM(singleton_method_removed); + mrb_value recv = mrb_iv_get(mrb, mrb_obj_value(c0), MRB_SYM(__attached__)); + if (!mrb_func_basic_p(mrb, recv, cb, mrb_do_nothing)) { + mrb_value sym = mrb_symbol_value(mid); + mrb_funcall_argv(mrb, recv, cb, 1, &sym); + } + } + else { + mrb_sym cb = MRB_SYM(method_removed); + mrb_value recv = mrb_obj_value(c0); + if (!mrb_func_basic_p(mrb, recv, cb, mrb_do_nothing)) { + mrb_value sym = mrb_symbol_value(mid); + mrb_funcall_argv(mrb, recv, cb, 1, &sym); } - else { - removed = MRB_SYM(method_removed); - recv = mrb_obj_value(c0); - } - if (!mrb_func_basic_p(mrb, recv, removed, mrb_do_nothing)) { - mrb_value sym = mrb_symbol_value(mid); - mrb_funcall_argv(mrb, recv, removed, 1, &sym); - } - return; } - mrb_name_error(mrb, mid, "method '%n' not defined in %C", mid, c); } static mrb_value