From 41b8e440e7f2c5d3d1c1a9644de4bdc06a343724 Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Mon, 17 Nov 2025 17:58:26 -0800 Subject: [PATCH 1/4] Support backwards compatibility for Set subclasses For subclasses from Set, require `set/subclass_compatible`, and extend the subclass and include a module in it that makes it more backwards compatible with the pure Ruby Set implementation used before Ruby 4. The module included in the subclass contains a near-copy of the previous Set implementation, with the following changes: * Accesses to `@hash` are generally replaced with `super` calls. In some cases, they are replaced with a call to another instance method. * Some methods that only accessed `@hash` and nothing else are not defined, so they inherit behavior from core Set. * The previous `Set#divide` implementation is not used, to avoid depending on tsort. This fixes the following two issues: * [Bug #21375] Set[] does not call #initialize * [Bug #21396] Set#initialize should call Set#add on items passed in It should also fix the vast majority of backwards compatibility issues in other cases where code subclassed Set and depended on implementation details (such as which methods call which other methods). This does not affect Set internals, so Set itself remains fast. For users who want to subclass Set but do not need to worry about backwards compatibility, they can subclass from Set::CoreSet, a Set subclass that does not have the backward compatibility layer included. --- lib/set/subclass_compatible.rb | 353 +++++++++++++++++++++++++++++++++ set.c | 22 ++ test/ruby/test_set.rb | 62 ++++-- 3 files changed, 421 insertions(+), 16 deletions(-) create mode 100644 lib/set/subclass_compatible.rb diff --git a/lib/set/subclass_compatible.rb b/lib/set/subclass_compatible.rb new file mode 100644 index 00000000000000..ab0aedc0e5bafd --- /dev/null +++ b/lib/set/subclass_compatible.rb @@ -0,0 +1,353 @@ +# frozen_string_literal: true + +# :markup: markdown +# +# set/subclass_compatible.rb - Provides compatibility for set subclasses +# +# Copyright (c) 2002-2024 Akinori MUSHA +# +# Documentation by Akinori MUSHA and Gavin Sinclair. +# +# All rights reserved. You can redistribute and/or modify it under the same +# terms as Ruby. + + +class Set + # This module is automatically included in subclasses of Set, to + # make them backwards compatible with the pure-Ruby set implementation + # used before Ruby 4. Users who want to use Set subclasses without + # this compatibility layer should subclass from Set::CoreSet. + # + # Note that Set subclasses that access `@hash` are not compatible even + # with this support. Such subclasses must be updated to support Ruby 4. + module SubclassCompatible + module ClassMethods + def [](*ary) + new(ary) + end + end + + # Creates a new set containing the elements of the given enumerable + # object. + # + # If a block is given, the elements of enum are preprocessed by the + # given block. + # + # Set.new([1, 2]) #=> # + # Set.new([1, 2, 1]) #=> # + # Set.new([1, 'c', :s]) #=> # + # Set.new(1..5) #=> # + # Set.new([1, 2, 3]) { |x| x * x } #=> # + def initialize(enum = nil, &block) # :yields: o + enum.nil? and return + + if block + do_with_enum(enum) { |o| add(block[o]) } + else + merge(enum) + end + end + + def do_with_enum(enum, &block) # :nodoc: + if enum.respond_to?(:each_entry) + enum.each_entry(&block) if block + elsif enum.respond_to?(:each) + enum.each(&block) if block + else + raise ArgumentError, "value must be enumerable" + end + end + private :do_with_enum + + def replace(enum) + if enum.instance_of?(self.class) + super + else + do_with_enum(enum) # make sure enum is enumerable before calling clear + clear + merge(enum) + end + end + + def to_set(*args, &block) + klass = if args.empty? + Set + else + warn "passing arguments to Enumerable#to_set is deprecated", uplevel: 1 + args.shift + end + return self if instance_of?(Set) && klass == Set && block.nil? && args.empty? + klass.new(self, *args, &block) + end + + def flatten_merge(set, seen = {}) # :nodoc: + set.each { |e| + if e.is_a?(Set) + case seen[e_id = e.object_id] + when true + raise ArgumentError, "tried to flatten recursive Set" + when false + next + end + + seen[e_id] = true + flatten_merge(e, seen) + seen[e_id] = false + else + add(e) + end + } + + self + end + protected :flatten_merge + + def flatten + self.class.new.flatten_merge(self) + end + + def flatten! + replace(flatten()) if any?(Set) + end + + def superset?(set) + case + when set.instance_of?(self.class) + super + when set.is_a?(Set) + size >= set.size && set.all?(self) + else + raise ArgumentError, "value must be a set" + end + end + alias >= superset? + + def proper_superset?(set) + case + when set.instance_of?(self.class) + super + when set.is_a?(Set) + size > set.size && set.all?(self) + else + raise ArgumentError, "value must be a set" + end + end + alias > proper_superset? + + def subset?(set) + case + when set.instance_of?(self.class) + super + when set.is_a?(Set) + size <= set.size && all?(set) + else + raise ArgumentError, "value must be a set" + end + end + alias <= subset? + + def proper_subset?(set) + case + when set.instance_of?(self.class) + super + when set.is_a?(Set) + size < set.size && all?(set) + else + raise ArgumentError, "value must be a set" + end + end + alias < proper_subset? + + def <=>(set) + return unless set.is_a?(Set) + + case size <=> set.size + when -1 then -1 if proper_subset?(set) + when +1 then +1 if proper_superset?(set) + else 0 if self.==(set) + end + end + + def intersect?(set) + case set + when Set + if size < set.size + any?(set) + else + set.any?(self) + end + when Enumerable + set.any?(self) + else + raise ArgumentError, "value must be enumerable" + end + end + + def disjoint?(set) + !intersect?(set) + end + + def add?(o) + add(o) unless include?(o) + end + + def delete?(o) + delete(o) if include?(o) + end + + def delete_if(&block) + block_given? or return enum_for(__method__) { size } + select(&block).each { |o| delete(o) } + self + end + + def keep_if(&block) + block_given? or return enum_for(__method__) { size } + reject(&block).each { |o| delete(o) } + self + end + + def collect! + block_given? or return enum_for(__method__) { size } + set = self.class.new + each { |o| set << yield(o) } + replace(set) + end + alias map! collect! + + def reject!(&block) + block_given? or return enum_for(__method__) { size } + n = size + delete_if(&block) + self if size != n + end + + def select!(&block) + block_given? or return enum_for(__method__) { size } + n = size + keep_if(&block) + self if size != n + end + + alias filter! select! + + def merge(*enums, **nil) + enums.each do |enum| + if enum.instance_of?(self.class) + super(enum) + else + do_with_enum(enum) { |o| add(o) } + end + end + + self + end + + def subtract(enum) + do_with_enum(enum) { |o| delete(o) } + self + end + + def |(enum) + dup.merge(enum) + end + alias + | + alias union | + + def -(enum) + dup.subtract(enum) + end + alias difference - + + def &(enum) + n = self.class.new + if enum.is_a?(Set) + if enum.size > size + each { |o| n.add(o) if enum.include?(o) } + else + enum.each { |o| n.add(o) if include?(o) } + end + else + do_with_enum(enum) { |o| n.add(o) if include?(o) } + end + n + end + alias intersection & + + def ^(enum) + n = self.class.new(enum) + each { |o| n.add(o) unless n.delete?(o) } + n + end + + def ==(other) + if self.equal?(other) + true + elsif other.instance_of?(self.class) + super + elsif other.is_a?(Set) && self.size == other.size + other.all? { |o| include?(o) } + else + false + end + end + + def eql?(o) # :nodoc: + return false unless o.is_a?(Set) + super + end + + def classify + block_given? or return enum_for(__method__) { size } + + h = {} + + each { |i| + (h[yield(i)] ||= self.class.new).add(i) + } + + h + end + + def join(separator=nil) + to_a.join(separator) + end + + InspectKey = :__inspect_key__ # :nodoc: + + # Returns a string containing a human-readable representation of the + # set ("#"). + def inspect + ids = (Thread.current[InspectKey] ||= []) + + if ids.include?(object_id) + return sprintf('#<%s: {...}>', self.class.name) + end + + ids << object_id + begin + return sprintf('#<%s: {%s}>', self.class, to_a.inspect[1..-2]) + ensure + ids.pop + end + end + + alias to_s inspect + + def pretty_print(pp) # :nodoc: + pp.group(1, sprintf('#<%s:', self.class.name), '>') { + pp.breakable + pp.group(1, '{', '}') { + pp.seplist(self) { |o| + pp.pp o + } + } + } + end + + def pretty_print_cycle(pp) # :nodoc: + pp.text sprintf('#<%s: {%s}>', self.class.name, empty? ? '' : '...') + end + end + private_constant :SubclassCompatible +end diff --git a/set.c b/set.c index 08b7831a7ee10c..7ef7c11d45de56 100644 --- a/set.c +++ b/set.c @@ -102,6 +102,8 @@ static ID id_any_p; static ID id_new; static ID id_i_hash; static ID id_set_iter_lev; +static ID id_subclass_compatible; +static ID id_class_methods; #define RSET_INITIALIZED FL_USER1 #define RSET_LEV_MASK (FL_USER13 | FL_USER14 | FL_USER15 | /* FL 13..19 */ \ @@ -424,6 +426,19 @@ set_s_create(int argc, VALUE *argv, VALUE klass) return set; } +static VALUE +set_s_inherited(VALUE klass, VALUE subclass) +{ + if (klass == rb_cSet) { + // When subclassing directly from Set, include the compatibility layer + rb_require("set/subclass_compatible.rb"); + VALUE subclass_compatible = rb_const_get(klass, id_subclass_compatible); + rb_include_module(subclass, subclass_compatible); + rb_extend_object(subclass, rb_const_get(subclass_compatible, id_class_methods)); + } + return Qnil; +} + static void check_set(VALUE arg) { @@ -2187,6 +2202,8 @@ Init_Set(void) id_any_p = rb_intern_const("any?"); id_new = rb_intern_const("new"); id_i_hash = rb_intern_const("@hash"); + id_subclass_compatible = rb_intern_const("SubclassCompatible"); + id_class_methods = rb_intern_const("ClassMethods"); id_set_iter_lev = rb_make_internal_id(); rb_define_alloc_func(rb_cSet, set_s_alloc); @@ -2257,5 +2274,10 @@ Init_Set(void) VALUE compat = rb_define_class_under(rb_cSet, "compatible", rb_cObject); rb_marshal_define_compat(rb_cSet, compat, compat_dumper, compat_loader); + // Create Set::CoreSet before defining inherited, so it does not include + // the backwards compatibility layer. + rb_define_class_under(rb_cSet, "CoreSet", rb_cSet); + rb_define_private_method(rb_singleton_class(rb_cSet), "inherited", set_s_inherited, 1); + rb_provide("set.rb"); } diff --git a/test/ruby/test_set.rb b/test/ruby/test_set.rb index 73103a1a123a0b..6dec0d41ae9a48 100644 --- a/test/ruby/test_set.rb +++ b/test/ruby/test_set.rb @@ -3,8 +3,11 @@ require 'set' class TC_Set < Test::Unit::TestCase - class Set2 < Set + class SetSubclass < Set end + class CoreSetSubclass < Set::CoreSet + end + ALL_SET_CLASSES = [Set, SetSubclass, CoreSetSubclass].freeze def test_marshal set = Set[1, 2, 3] @@ -264,7 +267,7 @@ def test_superset? set.superset?([2]) } - [Set, Set2].each { |klass| + ALL_SET_CLASSES.each { |klass| assert_equal(true, set.superset?(klass[]), klass.name) assert_equal(true, set.superset?(klass[1,2]), klass.name) assert_equal(true, set.superset?(klass[1,2,3]), klass.name) @@ -293,7 +296,7 @@ def test_proper_superset? set.proper_superset?([2]) } - [Set, Set2].each { |klass| + ALL_SET_CLASSES.each { |klass| assert_equal(true, set.proper_superset?(klass[]), klass.name) assert_equal(true, set.proper_superset?(klass[1,2]), klass.name) assert_equal(false, set.proper_superset?(klass[1,2,3]), klass.name) @@ -322,7 +325,7 @@ def test_subset? set.subset?([2]) } - [Set, Set2].each { |klass| + ALL_SET_CLASSES.each { |klass| assert_equal(true, set.subset?(klass[1,2,3,4]), klass.name) assert_equal(true, set.subset?(klass[1,2,3]), klass.name) assert_equal(false, set.subset?(klass[1,2]), klass.name) @@ -351,7 +354,7 @@ def test_proper_subset? set.proper_subset?([2]) } - [Set, Set2].each { |klass| + ALL_SET_CLASSES.each { |klass| assert_equal(true, set.proper_subset?(klass[1,2,3,4]), klass.name) assert_equal(false, set.proper_subset?(klass[1,2,3]), klass.name) assert_equal(false, set.proper_subset?(klass[1,2]), klass.name) @@ -371,7 +374,7 @@ def test_spacecraft_operator assert_nil(set <=> set.to_a) - [Set, Set2].each { |klass| + ALL_SET_CLASSES.each { |klass| assert_equal(-1, set <=> klass[1,2,3,4], klass.name) assert_equal( 0, set <=> klass[3,2,1] , klass.name) assert_equal(nil, set <=> klass[1,2,4] , klass.name) @@ -687,15 +690,17 @@ def test_and end def test_xor - set = Set[1,2,3,4] - ret = set ^ [2,4,5,5] - assert_not_same(set, ret) - assert_equal(Set[1,3,5], ret) - - set2 = Set2[1,2,3,4] - ret2 = set2 ^ [2,4,5,5] - assert_instance_of(Set2, ret2) - assert_equal(Set2[1,3,5], ret2) + ALL_SET_CLASSES.each { |klass| + set = klass[1,2,3,4] + ret = set ^ [2,4,5,5] + assert_not_same(set, ret) + assert_equal(klass[1,3,5], ret) + + set2 = klass[1,2,3,4] + ret2 = set2 ^ [2,4,5,5] + assert_instance_of(klass, ret2) + assert_equal(klass[1,3,5], ret2) + } end def test_eq @@ -847,9 +852,13 @@ def test_inspect set1.add(set2) assert_equal('Set[Set[0], 1, 2, Set[1, 2, Set[...]]]', set2.inspect) - c = Class.new(Set) + c = Class.new(Set::CoreSet) c.set_temporary_name("_MySet") assert_equal('_MySet[1, 2]', c[1, 2].inspect) + + c = Class.new(Set) + c.set_temporary_name("_MySet") + assert_equal('#<_MySet: {1, 2}>', c[1, 2].inspect) end def test_to_s @@ -922,6 +931,27 @@ def test_larger_sets assert_includes set, i end end + + def test_subclass_new_calls_add + c = Class.new(Set) do + def add(o) + super + super(o+1) + end + end + assert_equal([1, 2], c.new([1]).to_a) + end + + def test_subclass_aref_calls_initialize + c = Class.new(Set) do + def initialize(enum) + super + add(1) + end + end + assert_equal([2, 1], c[2].to_a) + end + end class TC_Enumerable < Test::Unit::TestCase From 0b6daad624e36d755f7a6919af2c2eee11353da1 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Thu, 20 Nov 2025 10:13:04 -0500 Subject: [PATCH 2/4] ZJIT: Fix pointer types for SetInstanceVariable --- zjit/src/codegen.rs | 2 +- zjit/src/hir.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 18266b46933e6c..84b3bbd13673ef 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -895,7 +895,7 @@ fn gen_setivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, val: } /// Emit an uncached instance variable store using the interpreter inline cache -fn gen_set_instance_variable(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_constant_cache, val: Opnd, state: &FrameState) { +fn gen_set_instance_variable(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_iv_cache_entry, val: Opnd, state: &FrameState) { gen_incr_counter(asm, Counter::dynamic_setivar_count); // Setting an ivar can raise FrozenError, so we need proper frame state for exception handling. gen_prepare_non_leaf_call(jit, asm, state); diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 2640507e33fab5..5db44025b9cf07 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -712,7 +712,7 @@ pub enum Insn { /// Set `self_val`'s instance variable `id` to `val` SetIvar { self_val: InsnId, id: ID, val: InsnId, state: InsnId }, /// Set `self_val`'s instance variable `id` to `val` using the interpreter inline cache - SetInstanceVariable { self_val: InsnId, id: ID, ic: *const iseq_inline_constant_cache, val: InsnId, state: InsnId }, + SetInstanceVariable { self_val: InsnId, id: ID, ic: *const iseq_inline_iv_cache_entry, val: InsnId, state: InsnId }, /// Check whether an instance variable exists on `self_val` DefinedIvar { self_val: InsnId, id: ID, pushval: VALUE, state: InsnId }, From f8cb9f320ceddbfe6feffb6410bb6212ed27673c Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Thu, 20 Nov 2025 10:28:23 -0500 Subject: [PATCH 3/4] ZJIT: Put optional interpreter cache on both GetIvar and SetIvar --- vm_insnhelper.c | 6 ++++++ zjit/src/codegen.rs | 32 ++++++++++++++++---------------- zjit/src/cruby.rs | 1 + zjit/src/hir.rs | 35 +++++++++++++++-------------------- zjit/src/hir/opt_tests.rs | 2 +- zjit/src/hir/tests.rs | 2 +- 6 files changed, 40 insertions(+), 38 deletions(-) diff --git a/vm_insnhelper.c b/vm_insnhelper.c index 7626d461352c8d..ff686d047a163d 100644 --- a/vm_insnhelper.c +++ b/vm_insnhelper.c @@ -1723,6 +1723,12 @@ rb_vm_setinstancevariable(const rb_iseq_t *iseq, VALUE obj, ID id, VALUE val, IV vm_setinstancevariable(iseq, obj, id, val, ic); } +VALUE +rb_vm_getinstancevariable(const rb_iseq_t *iseq, VALUE obj, ID id, IVC ic) +{ + return vm_getinstancevariable(iseq, obj, id, ic); +} + static VALUE vm_throw_continue(const rb_execution_context_t *ec, VALUE err) { diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 84b3bbd13673ef..8838a72fc887bd 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -428,7 +428,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::CCallVariadic { cfunc, recv, args, name, cme, state, return_type: _, elidable: _ } => { gen_ccall_variadic(jit, asm, *cfunc, *name, opnd!(recv), opnds!(args), *cme, &function.frame_state(*state)) } - Insn::GetIvar { self_val, id, state: _ } => gen_getivar(asm, opnd!(self_val), *id), + Insn::GetIvar { self_val, id, ic, state: _ } => gen_getivar(jit, asm, opnd!(self_val), *id, *ic), Insn::SetGlobal { id, val, state } => no_output!(gen_setglobal(jit, asm, *id, opnd!(val), &function.frame_state(*state))), Insn::GetGlobal { id, state } => gen_getglobal(jit, asm, *id, &function.frame_state(*state)), &Insn::GetLocal { ep_offset, level, use_sp, .. } => gen_getlocal(asm, ep_offset, level, use_sp), @@ -436,8 +436,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::GetConstantPath { ic, state } => gen_get_constant_path(jit, asm, *ic, &function.frame_state(*state)), Insn::GetClassVar { id, ic, state } => gen_getclassvar(jit, asm, *id, *ic, &function.frame_state(*state)), Insn::SetClassVar { id, val, ic, state } => no_output!(gen_setclassvar(jit, asm, *id, opnd!(val), *ic, &function.frame_state(*state))), - Insn::SetIvar { self_val, id, val, state } => no_output!(gen_setivar(jit, asm, opnd!(self_val), *id, opnd!(val), &function.frame_state(*state))), - Insn::SetInstanceVariable { self_val, id, ic, val, state } => no_output!(gen_set_instance_variable(jit, asm, opnd!(self_val), *id, *ic, opnd!(val), &function.frame_state(*state))), + Insn::SetIvar { self_val, id, ic, val, state } => no_output!(gen_setivar(jit, asm, opnd!(self_val), *id, *ic, opnd!(val), &function.frame_state(*state))), Insn::FixnumBitCheck { val, index } => gen_fixnum_bit_check(asm, opnd!(val), *index), Insn::SideExit { state, reason } => no_output!(gen_side_exit(jit, asm, reason, &function.frame_state(*state))), Insn::PutSpecialObject { value_type } => gen_putspecialobject(asm, *value_type), @@ -881,26 +880,27 @@ fn gen_ccall_variadic( } /// Emit an uncached instance variable lookup -fn gen_getivar(asm: &mut Assembler, recv: Opnd, id: ID) -> Opnd { +fn gen_getivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_iv_cache_entry) -> Opnd { gen_incr_counter(asm, Counter::dynamic_getivar_count); - asm_ccall!(asm, rb_ivar_get, recv, id.0.into()) + if ic.is_null() { + asm_ccall!(asm, rb_ivar_get, recv, id.0.into()) + } else { + let iseq = Opnd::Value(jit.iseq.into()); + asm_ccall!(asm, rb_vm_getinstancevariable, iseq, recv, id.0.into(), Opnd::const_ptr(ic)) + } } /// Emit an uncached instance variable store -fn gen_setivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, val: Opnd, state: &FrameState) { +fn gen_setivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_iv_cache_entry, val: Opnd, state: &FrameState) { gen_incr_counter(asm, Counter::dynamic_setivar_count); // Setting an ivar can raise FrozenError, so we need proper frame state for exception handling. gen_prepare_non_leaf_call(jit, asm, state); - asm_ccall!(asm, rb_ivar_set, recv, id.0.into(), val); -} - -/// Emit an uncached instance variable store using the interpreter inline cache -fn gen_set_instance_variable(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_iv_cache_entry, val: Opnd, state: &FrameState) { - gen_incr_counter(asm, Counter::dynamic_setivar_count); - // Setting an ivar can raise FrozenError, so we need proper frame state for exception handling. - gen_prepare_non_leaf_call(jit, asm, state); - let iseq = Opnd::Value(jit.iseq.into()); - asm_ccall!(asm, rb_vm_setinstancevariable, iseq, recv, id.0.into(), val, Opnd::const_ptr(ic)); + if ic.is_null() { + asm_ccall!(asm, rb_ivar_set, recv, id.0.into(), val); + } else { + let iseq = Opnd::Value(jit.iseq.into()); + asm_ccall!(asm, rb_vm_setinstancevariable, iseq, recv, id.0.into(), val, Opnd::const_ptr(ic)); + } } fn gen_getclassvar(jit: &mut JITState, asm: &mut Assembler, id: ID, ic: *const iseq_inline_cvar_cache_entry, state: &FrameState) -> Opnd { diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 61c25a4092bdc4..a854f2e07c0667 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -147,6 +147,7 @@ unsafe extern "C" { ) -> bool; pub fn rb_vm_set_ivar_id(obj: VALUE, idx: u32, val: VALUE) -> VALUE; pub fn rb_vm_setinstancevariable(iseq: IseqPtr, obj: VALUE, id: ID, val: VALUE, ic: IVC); + pub fn rb_vm_getinstancevariable(iseq: IseqPtr, obj: VALUE, id: ID, ic: IVC) -> VALUE; pub fn rb_aliased_callable_method_entry( me: *const rb_callable_method_entry_t, ) -> *const rb_callable_method_entry_t; diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 5db44025b9cf07..bbe5dd3435b75b 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -707,12 +707,10 @@ pub enum Insn { SetGlobal { id: ID, val: InsnId, state: InsnId }, //NewObject? - /// Get an instance variable `id` from `self_val` - GetIvar { self_val: InsnId, id: ID, state: InsnId }, - /// Set `self_val`'s instance variable `id` to `val` - SetIvar { self_val: InsnId, id: ID, val: InsnId, state: InsnId }, - /// Set `self_val`'s instance variable `id` to `val` using the interpreter inline cache - SetInstanceVariable { self_val: InsnId, id: ID, ic: *const iseq_inline_iv_cache_entry, val: InsnId, state: InsnId }, + /// Get an instance variable `id` from `self_val`, using the inline cache `ic` if present + GetIvar { self_val: InsnId, id: ID, ic: *const iseq_inline_iv_cache_entry, state: InsnId }, + /// Set `self_val`'s instance variable `id` to `val`, using the inline cache `ic` if present + SetIvar { self_val: InsnId, id: ID, val: InsnId, ic: *const iseq_inline_iv_cache_entry, state: InsnId }, /// Check whether an instance variable exists on `self_val` DefinedIvar { self_val: InsnId, id: ID, pushval: VALUE, state: InsnId }, @@ -912,7 +910,7 @@ impl Insn { | Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. } | Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetGlobal { .. } | Insn::SetLocal { .. } | Insn::Throw { .. } | Insn::IncrCounter(_) | Insn::IncrCounterPtr { .. } - | Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::SetInstanceVariable { .. } | Insn::StoreField { .. } | Insn::WriteBarrier { .. } => false, + | Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::StoreField { .. } | Insn::WriteBarrier { .. } => false, _ => true, } } @@ -1248,7 +1246,6 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { &Insn::StoreField { recv, id, offset, val } => write!(f, "StoreField {recv}, :{}@{:p}, {val}", id.contents_lossy(), self.ptr_map.map_offset(offset)), &Insn::WriteBarrier { recv, val } => write!(f, "WriteBarrier {recv}, {val}"), Insn::SetIvar { self_val, id, val, .. } => write!(f, "SetIvar {self_val}, :{}, {val}", id.contents_lossy()), - Insn::SetInstanceVariable { self_val, id, val, .. } => write!(f, "SetInstanceVariable {self_val}, :{}, {val}", id.contents_lossy()), Insn::GetGlobal { id, .. } => write!(f, "GetGlobal :{}", id.contents_lossy()), Insn::SetGlobal { id, val, .. } => write!(f, "SetGlobal :{}, {val}", id.contents_lossy()), &Insn::GetLocal { level, ep_offset, use_sp: true, rest_param } => write!(f, "GetLocal l{level}, SP@{}{}", ep_offset + 1, if rest_param { ", *" } else { "" }), @@ -1891,12 +1888,11 @@ impl Function { &ArrayInclude { ref elements, target, state } => ArrayInclude { elements: find_vec!(elements), target: find!(target), state: find!(state) }, &DupArrayInclude { ary, target, state } => DupArrayInclude { ary, target: find!(target), state: find!(state) }, &SetGlobal { id, val, state } => SetGlobal { id, val: find!(val), state }, - &GetIvar { self_val, id, state } => GetIvar { self_val: find!(self_val), id, state }, + &GetIvar { self_val, id, ic, state } => GetIvar { self_val: find!(self_val), id, ic, state }, &LoadField { recv, id, offset, return_type } => LoadField { recv: find!(recv), id, offset, return_type }, &StoreField { recv, id, offset, val } => StoreField { recv: find!(recv), id, offset, val: find!(val) }, &WriteBarrier { recv, val } => WriteBarrier { recv: find!(recv), val: find!(val) }, - &SetIvar { self_val, id, val, state } => SetIvar { self_val: find!(self_val), id, val: find!(val), state }, - &SetInstanceVariable { self_val, id, ic, val, state } => SetInstanceVariable { self_val: find!(self_val), id, ic, val: find!(val), state }, + &SetIvar { self_val, id, ic, val, state } => SetIvar { self_val: find!(self_val), id, ic, val: find!(val), state }, &GetClassVar { id, ic, state } => GetClassVar { id, ic, state }, &SetClassVar { id, val, ic, state } => SetClassVar { id, val: find!(val), ic, state }, &SetLocal { val, ep_offset, level } => SetLocal { val: find!(val), ep_offset, level }, @@ -1951,7 +1947,7 @@ impl Function { | Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. } | Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetLocal { .. } | Insn::IncrCounter(_) | Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::IncrCounterPtr { .. } - | Insn::SetInstanceVariable { .. } | Insn::StoreField { .. } | Insn::WriteBarrier { .. } => + | Insn::StoreField { .. } | Insn::WriteBarrier { .. } => panic!("Cannot infer type of instruction with no output: {}. See Insn::has_output().", self.insns[insn.0]), Insn::Const { val: Const::Value(val) } => Type::from_value(*val), Insn::Const { val: Const::CBool(val) } => Type::from_cbool(*val), @@ -2450,7 +2446,7 @@ impl Function { self.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state }); } } - let getivar = self.push_insn(block, Insn::GetIvar { self_val: recv, id, state }); + let getivar = self.push_insn(block, Insn::GetIvar { self_val: recv, id, ic: std::ptr::null(), state }); self.make_equal_to(insn_id, getivar); } else if let (VM_METHOD_TYPE_ATTRSET, &[val]) = (def_type, args.as_slice()) { self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state }); @@ -2467,7 +2463,7 @@ impl Function { self.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state }); } } - self.push_insn(block, Insn::SetIvar { self_val: recv, id, val, state }); + self.push_insn(block, Insn::SetIvar { self_val: recv, id, ic: std::ptr::null(), val, state }); self.make_equal_to(insn_id, val); } else if def_type == VM_METHOD_TYPE_OPTIMIZED { let opt_type: OptimizedMethodType = unsafe { get_cme_def_body_optimized_type(cme) }.into(); @@ -2750,7 +2746,7 @@ impl Function { assert!(self.blocks[block.0].insns.is_empty()); for insn_id in old_insns { match self.find(insn_id) { - Insn::GetIvar { self_val, id, state } => { + Insn::GetIvar { self_val, id, ic: _, state } => { let frame_state = self.frame_state(state); let Some(recv_type) = self.profiled_type_of_at(self_val, frame_state.insn_idx) else { // No (monomorphic/skewed polymorphic) profile info @@ -3503,8 +3499,7 @@ impl Function { worklist.push_back(self_val); worklist.push_back(state); } - &Insn::SetIvar { self_val, val, state, .. } - | &Insn::SetInstanceVariable { self_val, val, state, .. } => { + &Insn::SetIvar { self_val, val, state, .. } => { worklist.push_back(self_val); worklist.push_back(val); worklist.push_back(state); @@ -4082,7 +4077,6 @@ impl Function { } // Instructions with 2 Ruby object operands Insn::SetIvar { self_val: left, val: right, .. } - | Insn::SetInstanceVariable { self_val: left, val: right, .. } | Insn::NewRange { low: left, high: right, .. } | Insn::AnyToString { val: left, str: right, .. } | Insn::WriteBarrier { recv: left, val: right } => { @@ -5450,11 +5444,12 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { } YARVINSN_getinstancevariable => { let id = ID(get_arg(pc, 0).as_u64()); + let ic = get_arg(pc, 1).as_ptr(); // ic is in arg 1 // Assume single-Ractor mode to omit gen_prepare_non_leaf_call on gen_getivar // TODO: We only really need this if self_val is a class/module fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state: exit_id }); - let result = fun.push_insn(block, Insn::GetIvar { self_val: self_param, id, state: exit_id }); + let result = fun.push_insn(block, Insn::GetIvar { self_val: self_param, id, ic, state: exit_id }); state.stack_push(result); } YARVINSN_setinstancevariable => { @@ -5464,7 +5459,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { // TODO: We only really need this if self_val is a class/module fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state: exit_id }); let val = state.stack_pop()?; - fun.push_insn(block, Insn::SetInstanceVariable { self_val: self_param, id, ic, val, state: exit_id }); + fun.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id }); } YARVINSN_getclassvariable => { let id = ID(get_arg(pc, 0).as_u64()); diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 19f0e91b47e82b..70efa000761acf 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -3367,7 +3367,7 @@ mod hir_opt_tests { bb2(v6:BasicObject): v10:Fixnum[1] = Const Value(1) PatchPoint SingleRactorMode - SetInstanceVariable v6, :@foo, v10 + SetIvar v6, :@foo, v10 CheckInterrupts Return v10 "); diff --git a/zjit/src/hir/tests.rs b/zjit/src/hir/tests.rs index a00ca97e85a8b0..5e6ec118922b02 100644 --- a/zjit/src/hir/tests.rs +++ b/zjit/src/hir/tests.rs @@ -2241,7 +2241,7 @@ pub mod hir_build_tests { bb2(v6:BasicObject): v10:Fixnum[1] = Const Value(1) PatchPoint SingleRactorMode - SetInstanceVariable v6, :@foo, v10 + SetIvar v6, :@foo, v10 CheckInterrupts Return v10 "); From 48027256cf9d3e3bf12603184448cf88406b92d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20Barri=C3=A9?= Date: Thu, 20 Nov 2025 16:41:45 +0100 Subject: [PATCH 4/4] [ruby/json] Remove unused symbols https://github.com/ruby/json/commit/9364d0c761 --- ext/json/generator/generator.c | 6 +----- ext/json/parser/parser.c | 7 +------ 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/ext/json/generator/generator.c b/ext/json/generator/generator.c index 8930213b939a0d..fb8424dd86d736 100644 --- a/ext/json/generator/generator.c +++ b/ext/json/generator/generator.c @@ -38,7 +38,7 @@ typedef struct JSON_Generator_StateStruct { static VALUE mJSON, cState, cFragment, eGeneratorError, eNestingError, Encoding_UTF_8; -static ID i_to_s, i_to_json, i_new, i_pack, i_unpack, i_create_id, i_extend, i_encode; +static ID i_to_s, i_to_json, i_new, i_encode; static VALUE sym_indent, sym_space, sym_space_before, sym_object_nl, sym_array_nl, sym_max_nesting, sym_allow_nan, sym_allow_duplicate_key, sym_ascii_only, sym_depth, sym_buffer_initial_length, sym_script_safe, sym_escape_slash, sym_strict, sym_as_json; @@ -2173,10 +2173,6 @@ void Init_generator(void) i_to_s = rb_intern("to_s"); i_to_json = rb_intern("to_json"); i_new = rb_intern("new"); - i_pack = rb_intern("pack"); - i_unpack = rb_intern("unpack"); - i_create_id = rb_intern("create_id"); - i_extend = rb_intern("extend"); i_encode = rb_intern("encode"); sym_indent = ID2SYM(rb_intern("indent")); diff --git a/ext/json/parser/parser.c b/ext/json/parser/parser.c index 62ee1f24e71c40..a1d0265a919370 100644 --- a/ext/json/parser/parser.c +++ b/ext/json/parser/parser.c @@ -5,8 +5,7 @@ static VALUE mJSON, eNestingError, Encoding_UTF_8; static VALUE CNaN, CInfinity, CMinusInfinity; -static ID i_chr, i_aset, i_aref, - i_leftshift, i_new, i_try_convert, i_uminus, i_encode; +static ID i_new, i_try_convert, i_uminus, i_encode; static VALUE sym_max_nesting, sym_allow_nan, sym_allow_trailing_comma, sym_symbolize_names, sym_freeze, sym_decimal_class, sym_on_load, sym_allow_duplicate_key; @@ -1612,10 +1611,6 @@ void Init_parser(void) sym_decimal_class = ID2SYM(rb_intern("decimal_class")); sym_allow_duplicate_key = ID2SYM(rb_intern("allow_duplicate_key")); - i_chr = rb_intern("chr"); - i_aset = rb_intern("[]="); - i_aref = rb_intern("[]"); - i_leftshift = rb_intern("<<"); i_new = rb_intern("new"); i_try_convert = rb_intern("try_convert"); i_uminus = rb_intern("-@");