diff --git a/box.c b/box.c index e7065c1c298815..0ce8d0aee02f0f 100644 --- a/box.c +++ b/box.c @@ -62,6 +62,7 @@ bool ruby_box_crashed = false; // extern, changed only in vm.c VALUE rb_resolve_feature_path(VALUE klass, VALUE fname); static VALUE rb_box_inspect(VALUE obj); +static void cleanup_all_local_extensions(VALUE libmap); void rb_box_init_done(void) @@ -274,6 +275,8 @@ box_entry_free(void *ptr) st_foreach(box->classext_cow_classes, free_classext_for_box, (st_data_t)box); } + cleanup_all_local_extensions(box->ruby_dln_libmap); + box_root_free(ptr); xfree(ptr); } @@ -724,8 +727,57 @@ escaped_basename(const char *path, const char *fname, char *rvalue, size_t rsize } } +static void +box_ext_cleanup_mark(void *p) +{ + rb_gc_mark((VALUE)p); +} + +static void +box_ext_cleanup_free(void *p) +{ + VALUE path = (VALUE)p; + unlink(RSTRING_PTR(path)); +} + +static const rb_data_type_t box_ext_cleanup_type = { + "box_ext_cleanup", + {box_ext_cleanup_mark, box_ext_cleanup_free}, + .flags = RUBY_TYPED_FREE_IMMEDIATELY, +}; + +void +rb_box_cleanup_local_extension(VALUE cleanup) +{ + void *p = DATA_PTR(cleanup); + DATA_PTR(cleanup) = NULL; +#ifndef _WIN32 + if (p) box_ext_cleanup_free(p); +#endif +} + +static int +cleanup_local_extension_i(VALUE key, VALUE value, VALUE arg) +{ +#if defined(_WIN32) + HMODULE h = (HMODULE)NUM2SVALUE(value); + WCHAR module_path[MAXPATHLEN]; + DWORD len = GetModuleFileNameW(h, module_path, numberof(module_path)); + + FreeLibrary(h); + if (len > 0 && len < numberof(module_path)) DeleteFileW(module_path); +#endif + return ST_DELETE; +} + +static void +cleanup_all_local_extensions(VALUE libmap) +{ + rb_hash_foreach(libmap, cleanup_local_extension_i, 0); +} + VALUE -rb_box_local_extension(VALUE box_value, VALUE fname, VALUE path) +rb_box_local_extension(VALUE box_value, VALUE fname, VALUE path, VALUE *cleanup) { char ext_path[MAXPATHLEN], fname2[MAXPATHLEN], basename[MAXPATHLEN]; int wrote; @@ -739,14 +791,16 @@ rb_box_local_extension(VALUE box_value, VALUE fname, VALUE path) if (wrote >= (int)sizeof(ext_path)) { rb_bug("Extension file path in the box was too long"); } + VALUE new_path = rb_str_new_cstr(ext_path); + *cleanup = TypedData_Wrap_Struct(0, &box_ext_cleanup_type, NULL); enum copy_error_type copy_error = copy_ext_file(src_path, ext_path); if (copy_error) { char message[1024]; copy_ext_file_error(message, sizeof(message), copy_error); rb_raise(rb_eLoadError, "can't prepare the extension file for Ruby Box (%s from %"PRIsVALUE"): %s", ext_path, path, message); } - // TODO: register the path to be clean-uped - return rb_str_new_cstr(ext_path); + DATA_PTR(*cleanup) = (void *)new_path; + return new_path; } // TODO: delete it just after dln_load? or delay it? diff --git a/doc/language/box.md b/doc/language/box.md index 928c98eb3c32bb..aebce7188b3179 100644 --- a/doc/language/box.md +++ b/doc/language/box.md @@ -5,14 +5,13 @@ Ruby Box is designed to provide separated spaces in a Ruby process, to isolate a ## Known issues * Experimental warning is shown when ruby starts with `RUBY_BOX=1` (specify `-W:no-experimental` option to hide it) -* `bundle install` may fail -* `require 'active_support'` may fail -* A wrong current namespace detection happens sometimes in the root namespace +* Installing native extensions may fail under `RUBY_BOX=1` because of stack level too deep in extconf.rb +* `require 'active_support/core_ext'` may fail under `RUBY_BOX=1` +* Defined methods in a box may not be referred by built-in methods written in Ruby ## TODOs * Add the loaded namespace on iseq to check if another namespace tries running the iseq (add a field only when VM_CHECK_MODE?) -* Delete per-box extension files (.so) lazily or process exit (on Windows) * Assign its own TOPLEVEL_BINDING in boxes * Fix calling `warn` in boxes to refer `$VERBOSE` and `Warning.warn` in the box * Make an internal data container class `Ruby::Box::Entry` invisible @@ -258,6 +257,16 @@ Ruby Box works in file scope. One `.rb` file runs in a single box. Once a file is loaded in a box `box`, all methods/procs defined/created in the file run in `box`. +### Utility methods + +Several methods are available for trying/testing Ruby Box. + +* `Ruby::Box.current` returns the current box +* `Ruby::Box.enabled?` returns true/false to represent `RUBY_BOX=1` is specified or not +* `Ruby::Box.root` returns the root box +* `Ruby::Box.main` returns the main box +* `Ruby::Box#eval` evaluates a Ruby code (String) in the receiver box, just like calling `#load` with a file + ## Implementation details #### ISeq inline method/constant cache @@ -294,6 +303,10 @@ It is a breaking change. Users can define methods using `Ruby::Box.root.eval(...)`, but it's clearly not ideal API. +#### Assigning values to global variables used by builtin methods + +Similar to monkey patching methods, global variables assigned in a box is separated from the root box. Methods defined in the root box referring a global variable can't find the re-assigned one. + #### Context of `$LOAD_PATH` and `$LOADED_FEATURES` Global variables `$LOAD_PATH` and `$LOADED_FEATURES` control `require` method behaviors. So those variables are determined by the loading box instead of the current box. diff --git a/ext/win32/lib/win32/resolv.rb b/ext/win32/lib/win32/resolv.rb index 9a74a753517ab3..8d631a21403f4c 100644 --- a/ext/win32/lib/win32/resolv.rb +++ b/ext/win32/lib/win32/resolv.rb @@ -63,13 +63,13 @@ def get_info if add_search = search.nil? search = [] - nvdom = params.value('NV Domain') + domain = params.value('Domain') - if nvdom and !nvdom.empty? - search = [ nvdom ] + if domain and !domain.empty? + search = [ domain ] udmnd = params.value('UseDomainNameDevolution') if udmnd&.nonzero? - if /^\w+\./ =~ nvdom + if /^\w+\./ =~ domain devo = $' end end diff --git a/include/ruby/internal/core/rtypeddata.h b/include/ruby/internal/core/rtypeddata.h index ebcac2c8469db6..aed4bd89b893f6 100644 --- a/include/ruby/internal/core/rtypeddata.h +++ b/include/ruby/internal/core/rtypeddata.h @@ -607,7 +607,6 @@ RTYPEDDATA_TYPE(VALUE obj) return (const struct rb_data_type_struct *)(RTYPEDDATA(obj)->type & TYPED_DATA_PTR_MASK); } -RBIMPL_ATTR_PURE_UNLESS_DEBUG() RBIMPL_ATTR_ARTIFICIAL() /** * @private diff --git a/internal/box.h b/internal/box.h index c341f046d3e147..72263cc9dc5e5d 100644 --- a/internal/box.h +++ b/internal/box.h @@ -3,6 +3,16 @@ #include "ruby/ruby.h" /* for VALUE */ +#if SIZEOF_VALUE <= SIZEOF_LONG +# define SVALUE2NUM(x) LONG2NUM((long)(x)) +# define NUM2SVALUE(x) (SIGNED_VALUE)NUM2LONG(x) +#elif SIZEOF_VALUE <= SIZEOF_LONG_LONG +# define SVALUE2NUM(x) LL2NUM((LONG_LONG)(x)) +# define NUM2SVALUE(x) (SIGNED_VALUE)NUM2LL(x) +#else +# error Need integer for VALUE +#endif + /** * @author Ruby developers * @copyright This file is a part of the programming language Ruby. @@ -75,7 +85,8 @@ void rb_box_gc_update_references(void *ptr); rb_box_t * rb_get_box_t(VALUE ns); VALUE rb_get_box_object(rb_box_t *ns); -VALUE rb_box_local_extension(VALUE box, VALUE fname, VALUE path); +VALUE rb_box_local_extension(VALUE box, VALUE fname, VALUE path, VALUE *cleanup); +void rb_box_cleanup_local_extension(VALUE cleanup); void rb_initialize_main_box(void); void rb_box_init_done(void); diff --git a/load.c b/load.c index 5a0697a2626707..466517f465b098 100644 --- a/load.c +++ b/load.c @@ -27,16 +27,6 @@ #define IS_SOEXT(e) (strcmp((e), ".so") == 0 || strcmp((e), ".o") == 0) #define IS_DLEXT(e) (strcmp((e), DLEXT) == 0) -#if SIZEOF_VALUE <= SIZEOF_LONG -# define SVALUE2NUM(x) LONG2NUM((long)(x)) -# define NUM2SVALUE(x) (SIGNED_VALUE)NUM2LONG(x) -#elif SIZEOF_VALUE <= SIZEOF_LONG_LONG -# define SVALUE2NUM(x) LL2NUM((LONG_LONG)(x)) -# define NUM2SVALUE(x) (SIGNED_VALUE)NUM2LL(x) -#else -# error Need integer for VALUE -#endif - enum { loadable_ext_rb = (0+ /* .rb extension is the first in both tables */ 1) /* offset by rb_find_file_ext() */ @@ -1203,11 +1193,15 @@ load_ext(VALUE path, VALUE fname) { VALUE loaded = path; const rb_box_t *box = rb_loading_box(); + VALUE cleanup = 0; if (BOX_USER_P(box)) { - loaded = rb_box_local_extension(box->box_object, fname, path); + loaded = rb_box_local_extension(box->box_object, fname, path, &cleanup); } rb_scope_visibility_set(METHOD_VISI_PUBLIC); void *handle = dln_load_feature(RSTRING_PTR(loaded), RSTRING_PTR(fname)); + if (cleanup) { + rb_box_cleanup_local_extension(cleanup); + } RB_GC_GUARD(loaded); RB_GC_GUARD(fname); return (VALUE)handle; diff --git a/test/ruby/test_box.rb b/test/ruby/test_box.rb index c52c7564ae682e..5d06b60cd77005 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -697,10 +697,6 @@ def test_root_and_main_methods assert !$LOADED_FEATURES.include?("/tmp/barbaz") assert !Object.const_defined?(:FooClass) end; - ensure - tmp = ENV["TMPDIR"] || ENV["TMP"] || Etc.systmpdir || "/tmp" - pat = "_ruby_ns_*."+RbConfig::CONFIG["DLEXT"] - File.unlink(*Dir.glob(pat, base: tmp).map {|so| "#{tmp}/#{so}"}) end def test_basic_box_detections @@ -827,4 +823,16 @@ def test_mark_box_object_referred_only_from_binding assert_equal 42, b.eval('1+2') end; end + + def test_loaded_extension_deleted_in_user_box + require 'tmpdir' + Dir.mktmpdir do |tmpdir| + env = ENV_ENABLE_BOX.merge({'TMPDIR'=>tmpdir}) + assert_ruby_status([env], "#{<<~"begin;"}\n#{<<~'end;'}") + begin; + require "json" + end; + assert_empty(Dir.children(tmpdir)) + end + end end diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index cc5143aee52b0b..81d940407063f1 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -555,7 +555,8 @@ def test_send_kwarg def test(a:, b:) = [a, b] def entry = test(b: 2, a: 1) # change order entry - } + entry + }, call_threshold: 2 end def test_send_kwarg_optional @@ -567,6 +568,15 @@ def entry = test }, call_threshold: 2 end + def test_send_kwarg_optional_too_many + assert_compiles '[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]', %q{ + def test(a: 1, b: 2, c: 3, d: 4, e: 5, f: 6, g: 7, h: 8, i: 9, j: 10) = [a, b, c, d, e, f, g, h, i, j] + def entry = test + entry + entry + }, call_threshold: 2 + end + def test_send_kwarg_required_and_optional assert_compiles '[3, 2]', %q{ def test(a:, b: 2) = [a, b] @@ -585,6 +595,37 @@ def entry = test(a: 3) }, call_threshold: 2 end + def test_send_kwarg_to_ccall + assert_compiles '["a", "b", "c"]', %q{ + def test(s) = s.each_line(chomp: true).to_a + def entry = test(%(a\nb\nc)) + entry + entry + }, call_threshold: 2 + end + + def test_send_kwarg_and_block_to_ccall + assert_compiles '["a", "b", "c"]', %q{ + def test(s) + a = [] + s.each_line(chomp: true) { |l| a << l } + a + end + def entry = test(%(a\nb\nc)) + entry + entry + }, call_threshold: 2 + end + + def test_send_kwarg_with_too_many_args_to_c_call + assert_compiles '"a b c d {kwargs: :e}"', %q{ + def test(a:, b:, c:, d:, e:) = sprintf("%s %s %s %s %s", a, b, c, d, kwargs: e) + def entry = test(e: :e, d: :d, c: :c, a: :a, b: :b) + entry + entry + }, call_threshold: 2 + end + def test_send_kwrest assert_compiles '{a: 3}', %q{ def test(**kwargs) = kwargs @@ -594,6 +635,56 @@ def entry = test(a: 3) }, call_threshold: 2 end + def test_send_req_kwreq + assert_compiles '[1, 3]', %q{ + def test(a, c:) = [a, c] + def entry = test(1, c: 3) + entry + entry + }, call_threshold: 2 + end + + def test_send_req_opt_kwreq + assert_compiles '[[1, 2, 3], [-1, -2, -3]]', %q{ + def test(a, b = 2, c:) = [a, b, c] + def entry = [test(1, c: 3), test(-1, -2, c: -3)] # specify all, change kw order + entry + entry + }, call_threshold: 2 + end + + def test_send_req_opt_kwreq_kwopt + assert_compiles '[[1, 2, 3, 4], [-1, -2, -3, -4]]', %q{ + def test(a, b = 2, c:, d: 4) = [a, b, c, d] + def entry = [test(1, c: 3), test(-1, -2, d: -4, c: -3)] # specify all, change kw order + entry + entry + }, call_threshold: 2 + end + + def test_send_unexpected_keyword + assert_compiles ':error', %q{ + def test(a: 1) = a*2 + def entry + test(z: 2) + rescue ArgumentError + :error + end + + entry + entry + }, call_threshold: 2 + end + + def test_send_all_arg_types + assert_compiles '[:req, :opt, :post, :kwr, :kwo, true]', %q{ + def test(a, b = :opt, c, d:, e: :kwo) = [a, b, c, d, e, block_given?] + def entry = test(:req, :post, d: :kwr) {} + entry + entry + }, call_threshold: 2 + end + def test_send_ccall_variadic_with_different_receiver_classes assert_compiles '[true, true]', %q{ def test(obj) = obj.start_with?("a") @@ -1129,6 +1220,34 @@ def test(x) }, insns: [:opt_duparray_send], call_threshold: 1 end + def test_opt_newarray_send_pack_buffer + assert_compiles '["ABC", "ABC", "ABC", "ABC"]', %q{ + def test(num, buffer) + [num].pack('C', buffer:) + end + buf = "" + [test(65, buf), test(66, buf), test(67, buf), buf] + }, insns: [:opt_newarray_send], call_threshold: 1 + end + + def test_opt_newarray_send_pack_buffer_redefined + assert_compiles '["b", "A"]', %q{ + class Array + alias_method :old_pack, :pack + def pack(fmt, buffer: nil) + old_pack(fmt, buffer: buffer) + "b" + end + end + + def test(num, buffer) + [num].pack('C', buffer:) + end + buf = "" + [test(65, buf), buf] + }, insns: [:opt_newarray_send], call_threshold: 1 + end + def test_opt_newarray_send_hash assert_compiles 'Integer', %q{ def test(x) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 642991a4381265..05b704d66a4bb4 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -481,6 +481,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio &Insn::WriteBarrier { recv, val } => no_output!(gen_write_barrier(asm, opnd!(recv), opnd!(val), function.type_of(val))), &Insn::IsBlockGiven => gen_is_block_given(jit, asm), Insn::ArrayInclude { elements, target, state } => gen_array_include(jit, asm, opnds!(elements), opnd!(target), &function.frame_state(*state)), + Insn::ArrayPackBuffer { elements, fmt, buffer, state } => gen_array_pack_buffer(jit, asm, opnds!(elements), opnd!(fmt), opnd!(buffer), &function.frame_state(*state)), &Insn::DupArrayInclude { ary, target, state } => gen_dup_array_include(jit, asm, ary, opnd!(target), &function.frame_state(state)), Insn::ArrayHash { elements, state } => gen_opt_newarray_hash(jit, asm, opnds!(elements), &function.frame_state(*state)), &Insn::IsA { val, class } => gen_is_a(asm, opnd!(val), opnd!(class)), @@ -1336,6 +1337,20 @@ fn gen_send_without_block_direct( specval, }); + // Write "keyword_bits" to the callee's frame if the callee accepts keywords. + // This is a synthetic local/parameter that the callee reads via checkkeyword to determine + // which optional keyword arguments need their defaults evaluated. + if unsafe { rb_get_iseq_flags_has_kw(iseq) } { + let keyword = unsafe { rb_get_iseq_body_param_keyword(iseq) }; + let bits_start = unsafe { (*keyword).bits_start } as usize; + // Currently we only support required keywords, so all bits are 0 (all keywords specified). + // TODO: When supporting optional keywords, calculate actual unspecified_bits here. + let unspecified_bits = VALUE::fixnum_from_usize(0); + let bits_offset = (state.stack().len() - args.len() + bits_start) * SIZEOF_VALUE; + asm_comment!(asm, "write keyword bits to callee frame"); + asm.store(Opnd::mem(64, SP, bits_offset as i32), unspecified_bits.into()); + } + asm_comment!(asm, "switch to new SP register"); let sp_offset = (state.stack().len() + local_size - args.len() + VM_ENV_DATA_SIZE.to_usize()) * SIZEOF_VALUE; let new_sp = asm.add(SP, sp_offset.into()); @@ -1355,8 +1370,11 @@ fn gen_send_without_block_direct( // See vm_call_iseq_setup_normal_opt_start in vm_inshelper.c let lead_num = params.lead_num as u32; let opt_num = params.opt_num as u32; - assert!(args.len() as u32 <= lead_num + opt_num); - let num_optionals_passed = args.len() as u32 - lead_num; + let keyword = params.keyword; + let kw_req_num = if keyword.is_null() { 0 } else { unsafe { (*keyword).required_num } } as u32; + let req_num = lead_num + kw_req_num; + assert!(args.len() as u32 <= req_num + opt_num); + let num_optionals_passed = args.len() as u32 - req_num; num_optionals_passed } else { 0 @@ -1548,6 +1566,34 @@ fn gen_array_include( ) } +fn gen_array_pack_buffer( + jit: &JITState, + asm: &mut Assembler, + elements: Vec, + fmt: Opnd, + buffer: Opnd, + state: &FrameState, +) -> lir::Opnd { + gen_prepare_non_leaf_call(jit, asm, state); + + let array_len: c_long = elements.len().try_into().expect("Unable to fit length of elements into c_long"); + + // After gen_prepare_non_leaf_call, the elements are spilled to the Ruby stack. + // The elements are at the bottom of the virtual stack, followed by the fmt, followed by the buffer. + // Get a pointer to the first element on the Ruby stack. + let stack_bottom = state.stack().len() - elements.len() - 2; + let elements_ptr = asm.lea(Opnd::mem(64, SP, stack_bottom as i32 * SIZEOF_VALUE_I32)); + + unsafe extern "C" { + fn rb_vm_opt_newarray_pack_buffer(ec: EcPtr, num: c_long, elts: *const VALUE, fmt: VALUE, buffer: VALUE) -> VALUE; + } + asm_ccall!( + asm, + rb_vm_opt_newarray_pack_buffer, + EC, array_len.into(), elements_ptr, fmt, buffer + ) +} + fn gen_dup_array_include( jit: &JITState, asm: &mut Assembler, diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index abf48e04d64be6..6d1fe70e271869 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -616,6 +616,10 @@ pub enum SendFallbackReason { SendWithoutBlockNotOptimizedMethodTypeOptimized(OptimizedMethodType), SendWithoutBlockBopRedefined, SendWithoutBlockOperandsNotFixnum, + SendWithoutBlockDirectKeywordMismatch, + SendWithoutBlockDirectOptionalKeywords, + SendWithoutBlockDirectKeywordCountMismatch, + SendWithoutBlockDirectMissingKeyword, SendPolymorphic, SendMegamorphic, SendNoProfiles, @@ -632,6 +636,8 @@ pub enum SendFallbackReason { /// The call has at least one feature on the caller or callee side that the optimizer does not /// support. ComplexArgPass, + /// Caller has keyword arguments but callee doesn't expect them; need to convert to hash. + UnexpectedKeywordArgs, /// Initial fallback reason for every instruction, which should be mutated to /// a more actionable reason when an attempt to specialize the instruction fails. Uncategorized(ruby_vminsn_type), @@ -675,6 +681,7 @@ pub enum Insn { ArrayHash { elements: Vec, state: InsnId }, ArrayMax { elements: Vec, state: InsnId }, ArrayInclude { elements: Vec, target: InsnId, state: InsnId }, + ArrayPackBuffer { elements: Vec, fmt: InsnId, buffer: InsnId, state: InsnId }, DupArrayInclude { ary: VALUE, target: InsnId, state: InsnId }, /// Extend `left` with the elements from `right`. `left` and `right` must both be `Array`. ArrayExtend { left: InsnId, right: InsnId, state: InsnId }, @@ -1122,6 +1129,13 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { } write!(f, " | {target}") } + Insn::ArrayPackBuffer { elements, fmt, buffer, .. } => { + write!(f, "ArrayPackBuffer ")?; + for element in elements { + write!(f, "{element}, ")?; + } + write!(f, "fmt: {fmt}, buf: {buffer}") + } Insn::DupArrayInclude { ary, target, .. } => { write!(f, "DupArrayInclude {} | {}", ary.print(self.ptr_map), target) } @@ -1562,11 +1576,22 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq use Counter::*; if 0 != params.flags.has_rest() { count_failure(complex_arg_pass_param_rest) } if 0 != params.flags.has_post() { count_failure(complex_arg_pass_param_post) } - if 0 != params.flags.has_kw() { count_failure(complex_arg_pass_param_kw) } - if 0 != params.flags.has_kwrest() { count_failure(complex_arg_pass_param_kwrest) } if 0 != params.flags.has_block() { count_failure(complex_arg_pass_param_block) } if 0 != params.flags.forwardable() { count_failure(complex_arg_pass_param_forwardable) } + if 0 != params.flags.has_kwrest() { count_failure(complex_arg_pass_param_kwrest) } + if 0 != params.flags.has_kw() { + let keyword = params.keyword; + if !keyword.is_null() { + let num = unsafe { (*keyword).num }; + let required_num = unsafe { (*keyword).required_num }; + // Only support required keywords for now (no optional keywords) + if num != required_num { + count_failure(complex_arg_pass_param_kw_opt) + } + } + } + if !can_send { function.set_dynamic_send_reason(send_insn, ComplexArgPass); return false; @@ -1575,9 +1600,12 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq // Because we exclude e.g. post parameters above, they are also excluded from the sum below. let lead_num = params.lead_num; let opt_num = params.opt_num; + let keyword = params.keyword; + let kw_req_num = if keyword.is_null() { 0 } else { unsafe { (*keyword).required_num } }; + let req_num = lead_num + kw_req_num; can_send = c_int::try_from(args.len()) .as_ref() - .map(|argc| (lead_num..=lead_num + opt_num).contains(argc)) + .map(|argc| (req_num..=req_num + opt_num).contains(argc)) .unwrap_or(false); if !can_send { function.set_dynamic_send_reason(send_insn, ArgcParamMismatch); @@ -1990,6 +2018,7 @@ impl Function { &ArrayLength { array } => ArrayLength { array: find!(array) }, &ArrayMax { ref elements, state } => ArrayMax { elements: find_vec!(elements), state: find!(state) }, &ArrayInclude { ref elements, target, state } => ArrayInclude { elements: find_vec!(elements), target: find!(target), state: find!(state) }, + &ArrayPackBuffer { ref elements, fmt, buffer, state } => ArrayPackBuffer { elements: find_vec!(elements), fmt: find!(fmt), buffer: find!(buffer), state: find!(state) }, &DupArrayInclude { ary, target, state } => DupArrayInclude { ary, target: find!(target), state: find!(state) }, &ArrayHash { ref elements, state } => ArrayHash { elements: find_vec!(elements), state }, &SetGlobal { id, val, state } => SetGlobal { id, val: find!(val), state }, @@ -2144,6 +2173,7 @@ impl Function { Insn::FixnumBitCheck { .. } => types::BoolExact, Insn::ArrayMax { .. } => types::BasicObject, Insn::ArrayInclude { .. } => types::BoolExact, + Insn::ArrayPackBuffer { .. } => types::String, Insn::DupArrayInclude { .. } => types::BoolExact, Insn::ArrayHash { .. } => types::Fixnum, Insn::GetGlobal { .. } => types::BasicObject, @@ -2289,6 +2319,72 @@ impl Function { } } + /// Reorder keyword arguments to match the callee's expectation. + /// + /// Returns Ok with reordered arguments if successful, or Err with the fallback reason if not. + fn reorder_keyword_arguments( + &self, + args: &[InsnId], + kwarg: *const rb_callinfo_kwarg, + iseq: IseqPtr, + ) -> Result, SendFallbackReason> { + let callee_keyword = unsafe { rb_get_iseq_body_param_keyword(iseq) }; + if callee_keyword.is_null() { + // Caller is passing kwargs but callee doesn't expect them. + return Err(SendWithoutBlockDirectKeywordMismatch); + } + + let caller_kw_count = unsafe { get_cikw_keyword_len(kwarg) } as usize; + let callee_kw_count = unsafe { (*callee_keyword).num } as usize; + let callee_kw_required = unsafe { (*callee_keyword).required_num } as usize; + let callee_kw_table = unsafe { (*callee_keyword).table }; + + // For now, only handle the case where all keywords are required. + if callee_kw_count != callee_kw_required { + return Err(SendWithoutBlockDirectOptionalKeywords); + } + if caller_kw_count != callee_kw_count { + return Err(SendWithoutBlockDirectKeywordCountMismatch); + } + + // The keyword arguments are the last arguments in the args vector. + let kw_args_start = args.len() - caller_kw_count; + + // Build a mapping from caller keywords to their positions. + let mut caller_kw_order: Vec = Vec::with_capacity(caller_kw_count); + for i in 0..caller_kw_count { + let sym = unsafe { get_cikw_keywords_idx(kwarg, i as i32) }; + let id = unsafe { rb_sym2id(sym) }; + caller_kw_order.push(id); + } + + // Reorder keyword arguments to match callee expectation. + let mut reordered_kw_args: Vec = Vec::with_capacity(callee_kw_count); + for i in 0..callee_kw_count { + let expected_id = unsafe { *callee_kw_table.add(i) }; + + // Find where this keyword is in the caller's order + let mut found = false; + for (j, &caller_id) in caller_kw_order.iter().enumerate() { + if caller_id == expected_id { + reordered_kw_args.push(args[kw_args_start + j]); + found = true; + break; + } + } + + if !found { + // Required keyword not provided by caller which will raise an ArgumentError. + return Err(SendWithoutBlockDirectMissingKeyword); + } + } + + // Replace the keyword arguments with the reordered ones. + let mut processed_args = args[..kw_args_start].to_vec(); + processed_args.extend(reordered_kw_args); + Ok(processed_args) + } + /// Resolve the receiver type for method dispatch optimization. /// /// Takes the receiver's Type, receiver HIR instruction, and ISEQ instruction index. @@ -2504,6 +2600,7 @@ impl Function { cme = unsafe { rb_aliased_callable_method_entry(cme) }; def_type = unsafe { get_cme_def_type(cme) }; } + if def_type == VM_METHOD_TYPE_ISEQ { // TODO(max): Allow non-iseq; cache cme // Only specialize positional-positional calls @@ -2519,7 +2616,21 @@ impl Function { if let Some(profiled_type) = profiled_type { recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state }); } - let send_direct = self.push_insn(block, Insn::SendWithoutBlockDirect { recv, cd, cme, iseq, args, state }); + + let kwarg = unsafe { rb_vm_ci_kwarg(ci) }; + let processed_args = if !kwarg.is_null() { + match self.reorder_keyword_arguments(&args, kwarg, iseq) { + Ok(reordered) => reordered, + Err(reason) => { + self.set_dynamic_send_reason(insn_id, reason); + self.push_insn_id(block, insn_id); continue; + } + } + } else { + args.clone() + }; + + let send_direct = self.push_insn(block, Insn::SendWithoutBlockDirect { recv, cd, cme, iseq, args: processed_args, state }); self.make_equal_to(insn_id, send_direct); } else if def_type == VM_METHOD_TYPE_BMETHOD { let procv = unsafe { rb_get_def_bmethod_proc((*cme).def) }; @@ -2554,7 +2665,21 @@ impl Function { if let Some(profiled_type) = profiled_type { recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state }); } - let send_direct = self.push_insn(block, Insn::SendWithoutBlockDirect { recv, cd, cme, iseq, args, state }); + + let kwarg = unsafe { rb_vm_ci_kwarg(ci) }; + let processed_args = if !kwarg.is_null() { + match self.reorder_keyword_arguments(&args, kwarg, iseq) { + Ok(reordered) => reordered, + Err(reason) => { + self.set_dynamic_send_reason(insn_id, reason); + self.push_insn_id(block, insn_id); continue; + } + } + } else { + args.clone() + }; + + let send_direct = self.push_insn(block, Insn::SendWithoutBlockDirect { recv, cd, cme, iseq, args: processed_args, state }); self.make_equal_to(insn_id, send_direct); } else if def_type == VM_METHOD_TYPE_IVAR && args.is_empty() { // Check if we're accessing ivars of a Class or Module object as they require single-ractor mode. @@ -3096,7 +3221,7 @@ impl Function { // When seeing &block argument, fall back to dynamic dispatch for now // TODO: Support block forwarding - if unspecializable_call_type(ci_flags) { + if unspecializable_c_call_type(ci_flags) { fun.count_complex_call_features(block, ci_flags); fun.set_dynamic_send_reason(send_insn_id, ComplexArgPass); return Err(()); @@ -3658,6 +3783,12 @@ impl Function { worklist.push_back(target); worklist.push_back(state); } + &Insn::ArrayPackBuffer { ref elements, fmt, buffer, state } => { + worklist.extend(elements); + worklist.push_back(fmt); + worklist.push_back(buffer); + worklist.push_back(state); + } &Insn::DupArrayInclude { target, state, .. } => { worklist.push_back(target); worklist.push_back(state); @@ -4417,6 +4548,14 @@ impl Function { } Ok(()) } + Insn::ArrayPackBuffer { ref elements, fmt, buffer, .. } => { + self.assert_subtype(insn_id, fmt, types::BasicObject)?; + self.assert_subtype(insn_id, buffer, types::BasicObject)?; + for &element in elements { + self.assert_subtype(insn_id, element, types::BasicObject)?; + } + Ok(()) + } // Instructions with a Vec of Ruby objects Insn::InvokeBlock { ref args, .. } | Insn::NewArray { elements: ref args, .. } @@ -4994,9 +5133,14 @@ fn unhandled_call_type(flags: u32) -> Result<(), CallType> { Ok(()) } +/// If a given call to a c func uses overly complex arguments, then we won't specialize. +fn unspecializable_c_call_type(flags: u32) -> bool { + ((flags & VM_CALL_KWARG) != 0) || + unspecializable_call_type(flags) +} + /// If a given call uses overly complex arguments, then we won't specialize. fn unspecializable_call_type(flags: u32) -> bool { - ((flags & VM_CALL_KWARG) != 0) || ((flags & VM_CALL_ARGS_SPLAT) != 0) || ((flags & VM_CALL_ARGS_BLOCKARG) != 0) } @@ -5252,20 +5396,31 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { YARVINSN_opt_newarray_send => { let count = get_arg(pc, 0).as_usize(); let method = get_arg(pc, 1).as_u32(); - let elements = state.stack_pop_n(count)?; let (bop, insn) = match method { - VM_OPT_NEWARRAY_SEND_MAX => (BOP_MAX, Insn::ArrayMax { elements, state: exit_id }), - VM_OPT_NEWARRAY_SEND_HASH => (BOP_HASH, Insn::ArrayHash { elements, state: exit_id }), + VM_OPT_NEWARRAY_SEND_MAX => { + let elements = state.stack_pop_n(count)?; + (BOP_MAX, Insn::ArrayMax { elements, state: exit_id }) + } + VM_OPT_NEWARRAY_SEND_HASH => { + let elements = state.stack_pop_n(count)?; + (BOP_HASH, Insn::ArrayHash { elements, state: exit_id }) + } VM_OPT_NEWARRAY_SEND_INCLUDE_P => { - let target = elements[elements.len() - 1]; - let array_elements = elements[..elements.len() - 1].to_vec(); - (BOP_INCLUDE_P, Insn::ArrayInclude { elements: array_elements, target, state: exit_id }) - }, + let target = state.stack_pop()?; + let elements = state.stack_pop_n(count - 1)?; + (BOP_INCLUDE_P, Insn::ArrayInclude { elements, target, state: exit_id }) + } + VM_OPT_NEWARRAY_SEND_PACK_BUFFER => { + let buffer = state.stack_pop()?; + let fmt = state.stack_pop()?; + let elements = state.stack_pop_n(count - 2)?; + (BOP_PACK, Insn::ArrayPackBuffer { elements, fmt, buffer, state: exit_id }) + } _ => { // Unknown opcode; side-exit into the interpreter fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnhandledNewarraySend(method) }); break; // End the block - }, + } }; if !unsafe { rb_BASIC_OP_UNREDEFINED_P(bop, ARRAY_REDEFINED_OP_FLAG) } { // If the basic operation is already redefined, we cannot optimize it. @@ -6084,12 +6239,29 @@ fn compile_jit_entry_state(fun: &mut Function, jit_entry_block: BlockId, jit_ent let lead_num: usize = params.lead_num.try_into().expect("iseq param lead_num >= 0"); let passed_opt_num = jit_entry_idx; + // If the iseq has keyword parameters, the keyword bits local will be appended to the local table. + let kw_bits_idx: Option = if unsafe { rb_get_iseq_flags_has_kw(iseq) } { + let keyword = unsafe { rb_get_iseq_body_param_keyword(iseq) }; + if !keyword.is_null() { + Some(unsafe { (*keyword).bits_start } as usize) + } else { + None + } + } else { + None + }; + let self_param = fun.push_insn(jit_entry_block, Insn::Param); let mut entry_state = FrameState::new(iseq); for local_idx in 0..num_locals(iseq) { if (lead_num + passed_opt_num..lead_num + opt_num).contains(&local_idx) { // Omitted optionals are locals, so they start as nils before their code run entry_state.locals.push(fun.push_insn(jit_entry_block, Insn::Const { val: Const::Value(Qnil) })); + } else if Some(local_idx) == kw_bits_idx { + // We currently only support required keywords so the unspecified bits will always be zero. + // TODO: Make this a parameter when we start writing anything other than zero. + let unspecified_bits = VALUE::fixnum_from_usize(0); + entry_state.locals.push(fun.push_insn(jit_entry_block, Insn::Const { val: Const::Value(unspecified_bits) })); } else if local_idx < param_size { entry_state.locals.push(fun.push_insn(jit_entry_block, Insn::Param)); } else { diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 5646af804a2d0f..0a0737469371d7 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -626,8 +626,9 @@ mod hir_opt_tests { v2:BasicObject = GetLocal :k, l0, SP@5 v3:BasicObject = GetLocal , l0, SP@4 Jump bb2(v1, v2, v3) - bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject): + bb1(v6:BasicObject, v7:BasicObject): EntryPoint JIT(0) + v8:Fixnum[0] = Const Value(0) Jump bb2(v6, v7, v8) bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject): CheckInterrupts @@ -2761,10 +2762,10 @@ mod hir_opt_tests { } #[test] - fn dont_specialize_call_to_iseq_with_kw() { + fn specialize_call_to_iseq_with_multiple_required_kw() { eval(" - def foo(a:) = 1 - def test = foo(a: 1) + def foo(a:, b:) = [a, b] + def test = foo(a: 1, b: 2) test test "); @@ -2779,7 +2780,162 @@ mod hir_opt_tests { Jump bb2(v4) bb2(v6:BasicObject): v11:Fixnum[1] = Const Value(1) - IncrCounter complex_arg_pass_caller_kwarg + v13:Fixnum[2] = Const Value(2) + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v22:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + v23:BasicObject = SendWithoutBlockDirect v22, :foo (0x1038), v11, v13 + CheckInterrupts + Return v23 + "); + } + + #[test] + fn specialize_call_to_iseq_with_required_kw_reorder() { + eval(" + def foo(a:, b:, c:) = [a, b, c] + def test = foo(c: 3, a: 1, b: 2) + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + v11:Fixnum[3] = Const Value(3) + v13:Fixnum[1] = Const Value(1) + v15:Fixnum[2] = Const Value(2) + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v24:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + v25:BasicObject = SendWithoutBlockDirect v24, :foo (0x1038), v13, v15, v11 + CheckInterrupts + Return v25 + "); + } + + #[test] + fn specialize_call_to_iseq_with_positional_and_required_kw_reorder() { + eval(" + def foo(x, a:, b:) = [x, a, b] + def test = foo(0, b: 2, a: 1) + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + v11:Fixnum[0] = Const Value(0) + v13:Fixnum[2] = Const Value(2) + v15:Fixnum[1] = Const Value(1) + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v24:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + v25:BasicObject = SendWithoutBlockDirect v24, :foo (0x1038), v11, v15, v13 + CheckInterrupts + Return v25 + "); + } + + #[test] + fn dont_specialize_call_with_positional_and_optional_kw() { + eval(" + def foo(x, a: 1) = [x, a] + def test = foo(0, a: 2) + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + v11:Fixnum[0] = Const Value(0) + v13:Fixnum[2] = Const Value(2) + IncrCounter complex_arg_pass_param_kw_opt + v15:BasicObject = SendWithoutBlock v6, :foo, v11, v13 + CheckInterrupts + Return v15 + "); + } + + #[test] + fn specialize_call_with_pos_optional_and_req_kw() { + eval(" + def foo(r, x = 2, a:, b:) = [x, a] + def test = [foo(1, a: 3, b: 4), foo(1, 2, b: 4, a: 3)] # with and without the optional, change kw order + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:Fixnum[3] = Const Value(3) + v15:Fixnum[4] = Const Value(4) + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v37:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + v38:BasicObject = SendWithoutBlockDirect v37, :foo (0x1038), v11, v13, v15 + v20:Fixnum[1] = Const Value(1) + v22:Fixnum[2] = Const Value(2) + v24:Fixnum[4] = Const Value(4) + v26:Fixnum[3] = Const Value(3) + PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v41:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + v42:BasicObject = SendWithoutBlockDirect v41, :foo (0x1038), v20, v22, v26, v24 + v30:ArrayExact = NewArray v38, v42 + CheckInterrupts + Return v30 + "); + } + + #[test] + fn test_send_call_to_iseq_with_optional_kw() { + eval(" + def foo(a: 1) = a + def test = foo(a: 2) + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + v11:Fixnum[2] = Const Value(2) + IncrCounter complex_arg_pass_param_kw_opt v13:BasicObject = SendWithoutBlock v6, :foo, v11 CheckInterrupts Return v13 @@ -2805,7 +2961,7 @@ mod hir_opt_tests { Jump bb2(v4) bb2(v6:BasicObject): v11:Fixnum[1] = Const Value(1) - IncrCounter complex_arg_pass_caller_kwarg + IncrCounter complex_arg_pass_param_kwrest v13:BasicObject = SendWithoutBlock v6, :foo, v11 CheckInterrupts Return v13 @@ -2813,7 +2969,7 @@ mod hir_opt_tests { } #[test] - fn dont_specialize_call_to_iseq_with_param_kw() { + fn dont_specialize_call_to_iseq_with_optional_param_kw() { eval(" def foo(int: 1) = int + 1 def test = foo @@ -2830,7 +2986,7 @@ mod hir_opt_tests { EntryPoint JIT(0) Jump bb2(v4) bb2(v6:BasicObject): - IncrCounter complex_arg_pass_param_kw + IncrCounter complex_arg_pass_param_kw_opt v11:BasicObject = SendWithoutBlock v6, :foo CheckInterrupts Return v11 @@ -2862,6 +3018,69 @@ mod hir_opt_tests { "); } + #[test] + fn dont_optimize_ccall_with_kwarg() { + eval(" + def test = sprintf('%s', a: 1) + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + v11:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) + v12:StringExact = StringCopy v11 + v14:Fixnum[1] = Const Value(1) + v16:BasicObject = SendWithoutBlock v6, :sprintf, v12, v14 + CheckInterrupts + Return v16 + "); + } + + #[test] + fn dont_optimize_ccall_with_block_and_kwarg() { + eval(" + def test(s) + a = [] + s.each_line(chomp: true) { |l| a << l } + a + end + test %(a\nb\nc) + test %() + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal :s, l0, SP@5 + v3:NilClass = Const Value(nil) + Jump bb2(v1, v2, v3) + bb1(v6:BasicObject, v7:BasicObject): + EntryPoint JIT(0) + v8:NilClass = Const Value(nil) + Jump bb2(v6, v7, v8) + bb2(v10:BasicObject, v11:BasicObject, v12:NilClass): + v16:ArrayExact = NewArray + SetLocal :a, l0, EP@3, v16 + v22:TrueClass = Const Value(true) + IncrCounter complex_arg_pass_caller_kwarg + v24:BasicObject = Send v11, 0x1000, :each_line, v22 + v25:BasicObject = GetLocal :s, l0, EP@4 + v26:BasicObject = GetLocal :a, l0, EP@3 + v30:BasicObject = GetLocal :a, l0, EP@3 + CheckInterrupts + Return v30 + "); + } + #[test] fn dont_replace_get_constant_path_with_empty_ic() { eval(" @@ -3116,8 +3335,8 @@ mod hir_opt_tests { v13:NilClass = Const Value(nil) PatchPoint MethodRedefined(Hash@0x1008, new@0x1009, cme:0x1010) v46:HashExact = ObjectAllocClass Hash:VALUE(0x1008) - IncrCounter complex_arg_pass_param_kw IncrCounter complex_arg_pass_param_block + IncrCounter complex_arg_pass_param_kw_opt v20:BasicObject = SendWithoutBlock v46, :initialize CheckInterrupts CheckInterrupts @@ -8964,9 +9183,9 @@ mod hir_opt_tests { bb2(v6:BasicObject): v11:Fixnum[1] = Const Value(1) IncrCounter complex_arg_pass_param_rest - IncrCounter complex_arg_pass_param_kw - IncrCounter complex_arg_pass_param_kwrest IncrCounter complex_arg_pass_param_block + IncrCounter complex_arg_pass_param_kwrest + IncrCounter complex_arg_pass_param_kw_opt v13:BasicObject = SendWithoutBlock v6, :fancy, v11 CheckInterrupts Return v13 diff --git a/zjit/src/hir/tests.rs b/zjit/src/hir/tests.rs index 76be941fe529f9..7ef7671fa444f6 100644 --- a/zjit/src/hir/tests.rs +++ b/zjit/src/hir/tests.rs @@ -2154,7 +2154,51 @@ pub mod hir_build_tests { v36:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008)) v37:StringExact = StringCopy v36 v39:BasicObject = GetLocal :buf, l0, EP@3 - SideExit UnhandledNewarraySend(PACK_BUFFER) + PatchPoint BOPRedefined(ARRAY_REDEFINED_OP_FLAG, BOP_PACK) + v42:String = ArrayPackBuffer v15, v16, fmt: v37, buf: v39 + PatchPoint NoEPEscape(test) + CheckInterrupts + Return v30 + "); + } + + #[test] + fn test_opt_newarray_send_pack_buffer_redefined() { + eval(r#" + class Array + def pack(fmt, buffer: nil) = 5 + end + def test(a,b) + sum = a+b + buf = "" + [a,b].pack 'C', buffer: buf + buf + end + "#); + assert_contains_opcode("test", YARVINSN_opt_newarray_send); + assert_snapshot!(hir_string("test"), @r" + fn test@:6: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal :a, l0, SP@7 + v3:BasicObject = GetLocal :b, l0, SP@6 + v4:NilClass = Const Value(nil) + v5:NilClass = Const Value(nil) + Jump bb2(v1, v2, v3, v4, v5) + bb1(v8:BasicObject, v9:BasicObject, v10:BasicObject): + EntryPoint JIT(0) + v11:NilClass = Const Value(nil) + v12:NilClass = Const Value(nil) + Jump bb2(v8, v9, v10, v11, v12) + bb2(v14:BasicObject, v15:BasicObject, v16:BasicObject, v17:NilClass, v18:NilClass): + v25:BasicObject = SendWithoutBlock v15, :+, v16 + v29:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000)) + v30:StringExact = StringCopy v29 + v36:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + v37:StringExact = StringCopy v36 + v39:BasicObject = GetLocal :buf, l0, EP@3 + SideExit PatchPoint(BOPRedefined(ARRAY_REDEFINED_OP_FLAG, BOP_PACK)) "); } @@ -2956,8 +3000,9 @@ pub mod hir_build_tests { v3:BasicObject = GetLocal :exception, l0, SP@5 v4:BasicObject = GetLocal , l0, SP@4 Jump bb2(v1, v2, v3, v4) - bb1(v7:BasicObject, v8:BasicObject, v9:BasicObject, v10:BasicObject): + bb1(v7:BasicObject, v8:BasicObject, v9:BasicObject): EntryPoint JIT(0) + v10:Fixnum[0] = Const Value(0) Jump bb2(v7, v8, v9, v10) bb2(v12:BasicObject, v13:BasicObject, v14:BasicObject, v15:BasicObject): v19:Float = InvokeBuiltin rb_f_float, v12, v13, v14 @@ -3006,8 +3051,9 @@ pub mod hir_build_tests { v5:BasicObject = GetLocal :block, l0, SP@5 v6:NilClass = Const Value(nil) Jump bb2(v1, v2, v3, v4, v5, v6) - bb1(v9:BasicObject, v10:BasicObject, v11:BasicObject, v12:BasicObject, v13:BasicObject): + bb1(v9:BasicObject, v10:BasicObject, v11:BasicObject, v13:BasicObject): EntryPoint JIT(0) + v12:Fixnum[0] = Const Value(0) v14:NilClass = Const Value(nil) Jump bb2(v9, v10, v11, v12, v13, v14) bb2(v16:BasicObject, v17:BasicObject, v18:BasicObject, v19:BasicObject, v20:BasicObject, v21:NilClass): @@ -3068,8 +3114,9 @@ pub mod hir_build_tests { v4:BasicObject = GetLocal :immediate_sweep, l0, SP@5 v5:BasicObject = GetLocal , l0, SP@4 Jump bb2(v1, v2, v3, v4, v5) - bb1(v8:BasicObject, v9:BasicObject, v10:BasicObject, v11:BasicObject, v12:BasicObject): + bb1(v8:BasicObject, v9:BasicObject, v10:BasicObject, v11:BasicObject): EntryPoint JIT(0) + v12:Fixnum[0] = Const Value(0) Jump bb2(v8, v9, v10, v11, v12) bb2(v14:BasicObject, v15:BasicObject, v16:BasicObject, v17:BasicObject, v18:BasicObject): v25:FalseClass = Const Value(false) @@ -3488,8 +3535,9 @@ pub mod hir_build_tests { v2:BasicObject = GetLocal :kw, l0, SP@5 v3:BasicObject = GetLocal , l0, SP@4 Jump bb2(v1, v2, v3) - bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject): + bb1(v6:BasicObject, v7:BasicObject): EntryPoint JIT(0) + v8:Fixnum[0] = Const Value(0) Jump bb2(v6, v7, v8) bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject): v15:BasicObject = GetLocal , l0, EP@3 @@ -3561,8 +3609,9 @@ pub mod hir_build_tests { v34:BasicObject = GetLocal :k33, l0, SP@5 v35:BasicObject = GetLocal , l0, SP@4 Jump bb2(v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11, v12, v13, v14, v15, v16, v17, v18, v19, v20, v21, v22, v23, v24, v25, v26, v27, v28, v29, v30, v31, v32, v33, v34, v35) - bb1(v38:BasicObject, v39:BasicObject, v40:BasicObject, v41:BasicObject, v42:BasicObject, v43:BasicObject, v44:BasicObject, v45:BasicObject, v46:BasicObject, v47:BasicObject, v48:BasicObject, v49:BasicObject, v50:BasicObject, v51:BasicObject, v52:BasicObject, v53:BasicObject, v54:BasicObject, v55:BasicObject, v56:BasicObject, v57:BasicObject, v58:BasicObject, v59:BasicObject, v60:BasicObject, v61:BasicObject, v62:BasicObject, v63:BasicObject, v64:BasicObject, v65:BasicObject, v66:BasicObject, v67:BasicObject, v68:BasicObject, v69:BasicObject, v70:BasicObject, v71:BasicObject, v72:BasicObject): + bb1(v38:BasicObject, v39:BasicObject, v40:BasicObject, v41:BasicObject, v42:BasicObject, v43:BasicObject, v44:BasicObject, v45:BasicObject, v46:BasicObject, v47:BasicObject, v48:BasicObject, v49:BasicObject, v50:BasicObject, v51:BasicObject, v52:BasicObject, v53:BasicObject, v54:BasicObject, v55:BasicObject, v56:BasicObject, v57:BasicObject, v58:BasicObject, v59:BasicObject, v60:BasicObject, v61:BasicObject, v62:BasicObject, v63:BasicObject, v64:BasicObject, v65:BasicObject, v66:BasicObject, v67:BasicObject, v68:BasicObject, v69:BasicObject, v70:BasicObject, v71:BasicObject): EntryPoint JIT(0) + v72:Fixnum[0] = Const Value(0) Jump bb2(v38, v39, v40, v41, v42, v43, v44, v45, v46, v47, v48, v49, v50, v51, v52, v53, v54, v55, v56, v57, v58, v59, v60, v61, v62, v63, v64, v65, v66, v67, v68, v69, v70, v71, v72) bb2(v74:BasicObject, v75:BasicObject, v76:BasicObject, v77:BasicObject, v78:BasicObject, v79:BasicObject, v80:BasicObject, v81:BasicObject, v82:BasicObject, v83:BasicObject, v84:BasicObject, v85:BasicObject, v86:BasicObject, v87:BasicObject, v88:BasicObject, v89:BasicObject, v90:BasicObject, v91:BasicObject, v92:BasicObject, v93:BasicObject, v94:BasicObject, v95:BasicObject, v96:BasicObject, v97:BasicObject, v98:BasicObject, v99:BasicObject, v100:BasicObject, v101:BasicObject, v102:BasicObject, v103:BasicObject, v104:BasicObject, v105:BasicObject, v106:BasicObject, v107:BasicObject, v108:BasicObject): SideExit TooManyKeywordParameters diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 2272404b690b37..7d54f40c8d5dbd 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -222,6 +222,10 @@ make_counters! { send_fallback_too_many_args_for_lir, send_fallback_send_without_block_bop_redefined, send_fallback_send_without_block_operands_not_fixnum, + send_fallback_send_without_block_direct_keyword_mismatch, + send_fallback_send_without_block_direct_optional_keywords, + send_fallback_send_without_block_direct_keyword_count_mismatch, + send_fallback_send_without_block_direct_missing_keyword, send_fallback_send_polymorphic, send_fallback_send_megamorphic, send_fallback_send_no_profiles, @@ -231,6 +235,8 @@ make_counters! { // The call has at least one feature on the caller or callee side // that the optimizer does not support. send_fallback_one_or_more_complex_arg_pass, + // Caller has keyword arguments but callee doesn't expect them. + send_fallback_unexpected_keyword_args, send_fallback_bmethod_non_iseq_proc, send_fallback_obj_to_string_not_string, send_fallback_send_cfunc_variadic, @@ -347,7 +353,7 @@ make_counters! { // Unsupported parameter features complex_arg_pass_param_rest, complex_arg_pass_param_post, - complex_arg_pass_param_kw, + complex_arg_pass_param_kw_opt, complex_arg_pass_param_kwrest, complex_arg_pass_param_block, complex_arg_pass_param_forwardable, @@ -546,12 +552,17 @@ pub fn send_fallback_counter(reason: crate::hir::SendFallbackReason) -> Counter TooManyArgsForLir => send_fallback_too_many_args_for_lir, SendWithoutBlockBopRedefined => send_fallback_send_without_block_bop_redefined, SendWithoutBlockOperandsNotFixnum => send_fallback_send_without_block_operands_not_fixnum, + SendWithoutBlockDirectKeywordMismatch => send_fallback_send_without_block_direct_keyword_mismatch, + SendWithoutBlockDirectOptionalKeywords => send_fallback_send_without_block_direct_optional_keywords, + SendWithoutBlockDirectKeywordCountMismatch=> send_fallback_send_without_block_direct_keyword_count_mismatch, + SendWithoutBlockDirectMissingKeyword => send_fallback_send_without_block_direct_missing_keyword, SendPolymorphic => send_fallback_send_polymorphic, SendMegamorphic => send_fallback_send_megamorphic, SendNoProfiles => send_fallback_send_no_profiles, SendCfuncVariadic => send_fallback_send_cfunc_variadic, SendCfuncArrayVariadic => send_fallback_send_cfunc_array_variadic, ComplexArgPass => send_fallback_one_or_more_complex_arg_pass, + UnexpectedKeywordArgs => send_fallback_unexpected_keyword_args, ArgcParamMismatch => send_fallback_argc_param_mismatch, BmethodNonIseqProc => send_fallback_bmethod_non_iseq_proc, SendNotOptimizedMethodType(_) => send_fallback_send_not_optimized_method_type,