Skip to content

Commit 7683ab7

Browse files
authored
ZJIT: Side-exit sends with blocks to non-block methods (ruby#16624)
When the caller passes a block to a method that either rejects blocks (`&nil` parameter, `accepts_no_block`) or doesn't use them (`use_block` is false), ZJIT now falls back to the interpreter instead of compiling a direct send. This ensures: 1. Methods defined with `&nil` properly raise ArgumentError 2. Unused block warnings are properly emitted Previously, ZJIT would compile these sends directly, skipping the block validation and warning logic that the interpreter handles.
1 parent 88efe89 commit 7683ab7

File tree

4 files changed

+62
-16
lines changed

4 files changed

+62
-16
lines changed

zjit/src/codegen_tests.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5419,3 +5419,37 @@ fn test_ep_escape_preserves_keyword_default() {
54195419
target("x")
54205420
"#), @"[]");
54215421
}
5422+
5423+
#[test]
5424+
fn test_send_block_to_accepts_no_block() {
5425+
// Methods with &nil should raise ArgumentError when called with a block
5426+
assert_snapshot!(inspect("
5427+
def m(a, &nil); a end
5428+
5429+
def test
5430+
m(1) {}
5431+
rescue ArgumentError => e
5432+
e.message
5433+
end
5434+
5435+
test
5436+
test
5437+
"), @r#""no block accepted""#);
5438+
}
5439+
5440+
#[test]
5441+
fn test_send_block_to_method_not_using_block() {
5442+
// Passing a block to a method that doesn't use it should still work correctly.
5443+
// ZJIT falls back to the interpreter for this case so that unused block
5444+
// warnings are properly emitted.
5445+
assert_snapshot!(inspect("
5446+
def m_no_block = 42
5447+
5448+
def test
5449+
m_no_block {}
5450+
end
5451+
5452+
test
5453+
test
5454+
"), @"42");
5455+
}

zjit/src/hir.rs

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2357,7 +2357,7 @@ pub enum ValidationError {
23572357
}
23582358

23592359
/// Check if we can do a direct send to the given iseq with the given args.
2360-
fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq_t, ci: *const rb_callinfo, send_insn: InsnId, args: &[InsnId]) -> bool {
2360+
fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq_t, ci: *const rb_callinfo, send_insn: InsnId, args: &[InsnId], has_block: bool) -> bool {
23612361
let mut can_send = true;
23622362
let mut count_failure = |counter| {
23632363
can_send = false;
@@ -2376,6 +2376,16 @@ fn can_direct_send(function: &mut Function, block: BlockId, iseq: *const rb_iseq
23762376
{ count_failure(complex_arg_pass_param_block) }
23772377
if 0 != params.flags.has_kwrest() { count_failure(complex_arg_pass_param_kwrest) }
23782378

2379+
// If the caller passes a block (literal or &block), we need to fall back to the
2380+
// interpreter for two cases it handles that we don't:
2381+
// 1. Methods with &nil reject blocks with ArgumentError
2382+
// 2. Methods that don't use blocks emit "unused block" warnings
2383+
let caller_passes_block = has_block || caller_passes_block_arg;
2384+
if caller_passes_block && 0 != params.flags.accepts_no_block()
2385+
{ count_failure(complex_arg_pass_accepts_no_block) }
2386+
if caller_passes_block && 0 == params.flags.use_block()
2387+
{ count_failure(complex_arg_pass_does_not_use_block) }
2388+
23792389
if !can_send {
23802390
function.set_dynamic_send_reason(send_insn, ComplexArgPass);
23812391
return false;
@@ -3723,7 +3733,7 @@ impl Function {
37233733
// Only specialize positional-positional calls
37243734
// TODO(max): Handle other kinds of parameter passing
37253735
let iseq = unsafe { get_def_iseq_ptr((*cme).def) };
3726-
if !can_direct_send(self, block, iseq, ci, insn_id, args.as_slice()) {
3736+
if !can_direct_send(self, block, iseq, ci, insn_id, args.as_slice(), has_block) {
37273737
self.push_insn_id(block, insn_id); continue;
37283738
}
37293739

@@ -3762,7 +3772,7 @@ impl Function {
37623772
let capture = unsafe { proc_block.as_.captured.as_ref() };
37633773
let iseq = unsafe { *capture.code.iseq.as_ref() };
37643774

3765-
if !can_direct_send(self, block, iseq, ci, insn_id, args.as_slice()) {
3775+
if !can_direct_send(self, block, iseq, ci, insn_id, args.as_slice(), has_block) {
37663776
self.push_insn_id(block, insn_id); continue;
37673777
}
37683778

@@ -4135,7 +4145,7 @@ impl Function {
41354145
// If not, we can't do direct dispatch.
41364146
let super_iseq = unsafe { get_def_iseq_ptr((*super_cme).def) };
41374147
// TODO: pass Option<blockiseq> to can_direct_send when we start specializing `super { ... }`.
4138-
if !can_direct_send(self, block, super_iseq, ci, insn_id, args.as_slice()) {
4148+
if !can_direct_send(self, block, super_iseq, ci, insn_id, args.as_slice(), false) {
41394149
self.push_insn_id(block, insn_id);
41404150
self.set_dynamic_send_reason(insn_id, SuperTargetComplexArgsPass);
41414151
continue;

zjit/src/hir/opt_tests.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11433,6 +11433,8 @@ mod hir_opt_tests {
1143311433

1143411434
#[test]
1143511435
fn test_inline_send_with_block_with_no_params() {
11436+
// Passing a block to a method that doesn't use it falls back to the
11437+
// interpreter so that unused block warnings are properly emitted.
1143611438
eval(r#"
1143711439
def callee = 123
1143811440
def test
@@ -11452,16 +11454,16 @@ mod hir_opt_tests {
1145211454
v4:BasicObject = LoadArg :self@0
1145311455
Jump bb3(v4)
1145411456
bb3(v6:BasicObject):
11455-
PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010)
11456-
v19:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)]
11457-
v21:Fixnum[123] = Const Value(123)
11457+
v11:BasicObject = Send v6, 0x1000, :callee # SendFallbackReason: Complex argument passing
1145811458
CheckInterrupts
11459-
Return v21
11459+
Return v11
1146011460
");
1146111461
}
1146211462

1146311463
#[test]
1146411464
fn test_inline_send_with_block_with_one_param() {
11465+
// Passing a block to a method that doesn't use it falls back to the
11466+
// interpreter so that unused block warnings are properly emitted.
1146511467
eval(r#"
1146611468
def callee = 123
1146711469
def test
@@ -11481,16 +11483,16 @@ mod hir_opt_tests {
1148111483
v4:BasicObject = LoadArg :self@0
1148211484
Jump bb3(v4)
1148311485
bb3(v6:BasicObject):
11484-
PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010)
11485-
v19:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)]
11486-
v21:Fixnum[123] = Const Value(123)
11486+
v11:BasicObject = Send v6, 0x1000, :callee # SendFallbackReason: Complex argument passing
1148711487
CheckInterrupts
11488-
Return v21
11488+
Return v11
1148911489
");
1149011490
}
1149111491

1149211492
#[test]
1149311493
fn test_inline_send_with_block_with_multiple_params() {
11494+
// Passing a block to a method that doesn't use it falls back to the
11495+
// interpreter so that unused block warnings are properly emitted.
1149411496
eval(r#"
1149511497
def callee = 123
1149611498
def test
@@ -11510,11 +11512,9 @@ mod hir_opt_tests {
1151011512
v4:BasicObject = LoadArg :self@0
1151111513
Jump bb3(v4)
1151211514
bb3(v6:BasicObject):
11513-
PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010)
11514-
v19:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)]
11515-
v21:Fixnum[123] = Const Value(123)
11515+
v11:BasicObject = Send v6, 0x1000, :callee # SendFallbackReason: Complex argument passing
1151611516
CheckInterrupts
11517-
Return v21
11517+
Return v11
1151811518
");
1151911519
}
1152011520

zjit/src/stats.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,8 @@ make_counters! {
424424
complex_arg_pass_param_kwrest,
425425
complex_arg_pass_param_block,
426426
complex_arg_pass_param_forwardable,
427+
complex_arg_pass_accepts_no_block,
428+
complex_arg_pass_does_not_use_block,
427429

428430
// Unsupported caller side features
429431
complex_arg_pass_caller_splat,

0 commit comments

Comments
 (0)