Tighten specialization guards and add send_none fastpath#7359
Tighten specialization guards and add send_none fastpath#7359youknowone merged 17 commits intoRustPython:mainfrom
Conversation
📝 WalkthroughWalkthroughRefactors coroutine send logic by centralizing post-run handling into Changes
Sequence DiagramsequenceDiagram
participant Caller
participant Coroutine as Coroutine (send)
participant Frame as ExecutingFrame (run_with_context)
participant VM as VirtualMachine
participant Finalize as finalize_send_result
Caller->>Coroutine: send(value)
alt coroutine just-started
Coroutine->>Coroutine: compute value = None
else already started
Coroutine->>Coroutine: use provided value
end
Coroutine->>Frame: run_with_context(vm, computed_value)
Frame->>VM: execute frame
VM-->>Frame: ExecutionResult
Frame-->>Coroutine: ExecutionResult
Coroutine->>Finalize: finalize_send_result(jen, result, vm)
Finalize->>Finalize: map StopIteration / StopAsyncIteration
Finalize->>Coroutine: maybe_close frame if final
Finalize-->>Caller: PyIterReturn / Err
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/vm/src/frame.rs (1)
3160-3164:SendGencan usesend_nonefor the commonNonecase.
Instruction::Sendalready does this, butInstruction::SendGenstill always callssend. Aligning both paths keeps the intended fastpath consistent.Suggested patch
- match coro.send(receiver, val, vm)? { + let ret = if vm.is_none(&val) { + coro.send_none(receiver, vm)? + } else { + coro.send(receiver, val, vm)? + }; + match ret {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/frame.rs` around lines 3160 - 3164, The SendGen path currently calls coro.send(...) even when val is None; change it to use the fastpath method coro.send_none(...) (or send_none) when val equals PyNone to match Instruction::Send behavior—locate the block gated by can_fast_send in the SendGen handling (symbols: SendGen, can_fast_send, top_value, builtin_coro, send) and replace the call to coro.send(receiver, val, vm) with a conditional that invokes coro.send_none(receiver, vm) for the None case and falls back to coro.send(...) otherwise, preserving the existing PyIterReturn handling and error propagation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/vm/src/frame.rs`:
- Around line 8145-8148: The specialization_eval_frame_active method currently
returns false unconditionally, disabling frame-aware specialization guards; fix
it by reading the VM tracing flags instead of returning false: in
specialization_eval_frame_active(&self, vm: &VirtualMachine) check
vm.trace_func, vm.profile_func, and vm.use_tracing (matching the pattern used in
vm/mod.rs around the other checks) and return true when any of those indicate
tracing/profile is active so frame-aware specialization is disabled when tracing
or profiling is enabled.
---
Nitpick comments:
In `@crates/vm/src/frame.rs`:
- Around line 3160-3164: The SendGen path currently calls coro.send(...) even
when val is None; change it to use the fastpath method coro.send_none(...) (or
send_none) when val equals PyNone to match Instruction::Send behavior—locate the
block gated by can_fast_send in the SendGen handling (symbols: SendGen,
can_fast_send, top_value, builtin_coro, send) and replace the call to
coro.send(receiver, val, vm) with a conditional that invokes
coro.send_none(receiver, vm) for the None case and falls back to coro.send(...)
otherwise, preserving the existing PyIterReturn handling and error propagation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 2278b9c7-6e88-4782-9d3f-ed74f4874b53
📒 Files selected for processing (2)
crates/vm/src/coroutine.rscrates/vm/src/frame.rs
- Implement specialization_eval_frame_active to check vm.use_tracing so specializations are skipped when tracing/profiling is active - Add send_none fastpath in SendGen handler for the common None case
youknowone
force-pushed
the
specialization
branch
from
3e92a7a to
c7d095a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
crates/vm/src/coroutine.rs (1)
146-182: Deduplicatesend_none/sendshared execution path.Both functions duplicate the same closed-check / frame-prepare / resume / finalize flow and only differ in value preparation.
♻️ Proposed refactor
+ fn send_impl<F>( + &self, + jen: &PyObject, + vm: &VirtualMachine, + value_for_state: F, + ) -> PyResult<PyIterReturn> + where + F: FnOnce(bool) -> PyResult<Option<PyObjectRef>>, + { + if self.closed.load() { + return Ok(PyIterReturn::StopIteration(None)); + } + let started = self.frame.lasti() > 0; + let value = value_for_state(started)?; + self.frame.locals_to_fast(vm)?; + let result = self.run_with_context(jen, vm, |f| f.resume(value, vm)); + self.finalize_send_result(jen, result, vm) + } + pub(crate) fn send_none(&self, jen: &PyObject, vm: &VirtualMachine) -> PyResult<PyIterReturn> { - if self.closed.load() { - return Ok(PyIterReturn::StopIteration(None)); - } - self.frame.locals_to_fast(vm)?; - let value = if self.frame.lasti() > 0 { - Some(vm.ctx.none()) - } else { - None - }; - let result = self.run_with_context(jen, vm, |f| f.resume(value, vm)); - self.finalize_send_result(jen, result, vm) + self.send_impl(jen, vm, |started| { + Ok(if started { Some(vm.ctx.none()) } else { None }) + }) } pub fn send( &self, jen: &PyObject, value: PyObjectRef, vm: &VirtualMachine, ) -> PyResult<PyIterReturn> { - if self.closed.load() { - return Ok(PyIterReturn::StopIteration(None)); - } - self.frame.locals_to_fast(vm)?; - let value = if self.frame.lasti() > 0 { - Some(value) - } else if !vm.is_none(&value) { - return Err(vm.new_type_error(format!( - "can't send non-None value to a just-started {}", - gen_name(jen, vm), - ))); - } else { - None - }; - let result = self.run_with_context(jen, vm, |f| f.resume(value, vm)); - self.finalize_send_result(jen, result, vm) + self.send_impl(jen, vm, |started| { + if started { + Ok(Some(value)) + } else if !vm.is_none(&value) { + Err(vm.new_type_error(format!( + "can't send non-None value to a just-started {}", + gen_name(jen, vm), + ))) + } else { + Ok(None) + } + }) }As per coding guidelines "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/coroutine.rs` around lines 146 - 182, Both send_none and send duplicate the same closed-check, frame.locals_to_fast call, resume via run_with_context, and finalize_send_result logic; extract that shared flow into a helper (e.g., send_common or resume_with_value) that accepts the prepared Option<PyObjectRef> (or a closure producing it) and performs closed.load check, self.frame.locals_to_fast(vm)?, calls run_with_context(jen, vm, |f| f.resume(value, vm)), and returns finalize_send_result; then simplify send_none to compute its value (using frame.lasti() and vm.ctx.none()) and call the helper, and simplify send to compute its value (respecting frame.lasti() and the non-None error) and call the same helper, keeping all original calls to frame.lasti, run_with_context, finalize_send_result, and closed.load.crates/vm/src/frame.rs (1)
7837-7861: Reduce duplicated guard/backoff branches in descriptor call-conv selection.The
NOARGSandObranches duplicate the same backoff/return structure with only expected-arity differing. Extract expected arity first, then run one shared guard+backoff path.As per coding guidelines, "When branches differ only in a value but share common logic, extract the differing value first, then call the common logic once to avoid duplicate code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/vm/src/frame.rs` around lines 7837 - 7861, The NOARGS and O branches in the descriptor call-conv selection duplicate the same guard/backoff/return logic; refactor by computing the expected arity from call_conv (e.g., map PyMethodFlags::NOARGS -> 0, PyMethodFlags::O -> 1) and choose the Instruction variant after the check, then perform a single shared guard that compares nargs to expected_arity and on mismatch runs the adaptive backoff using self.code.instructions.read_adaptive_counter(cache_base) and write_adaptive_counter(...bytecode::adaptive_counter_backoff(...)) and returns; update references to new_op, call_conv, nargs, cache_base, and the Instruction variants (CallMethodDescriptorNoargs, etc.) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/derive-impl/src/pyclass.rs`:
- Around line 1018-1028: The drop_first_typed logic incorrectly ignores the
parsed raw flag, causing methods marked #[pymethod(raw)] or
#[pyclassmethod(raw)] without a receiver to drop the first typed argument;
update the match that sets drop_first_typed (which currently checks
self.inner.attr_name and has_receiver) to also require !raw before returning 1
so that raw methods do not decrement the first typed arg, then pass the
unchanged drop_first_typed into infer_native_call_flags(func.sig(),
drop_first_typed).
---
Nitpick comments:
In `@crates/vm/src/coroutine.rs`:
- Around line 146-182: Both send_none and send duplicate the same closed-check,
frame.locals_to_fast call, resume via run_with_context, and finalize_send_result
logic; extract that shared flow into a helper (e.g., send_common or
resume_with_value) that accepts the prepared Option<PyObjectRef> (or a closure
producing it) and performs closed.load check, self.frame.locals_to_fast(vm)?,
calls run_with_context(jen, vm, |f| f.resume(value, vm)), and returns
finalize_send_result; then simplify send_none to compute its value (using
frame.lasti() and vm.ctx.none()) and call the helper, and simplify send to
compute its value (respecting frame.lasti() and the non-None error) and call the
same helper, keeping all original calls to frame.lasti, run_with_context,
finalize_send_result, and closed.load.
In `@crates/vm/src/frame.rs`:
- Around line 7837-7861: The NOARGS and O branches in the descriptor call-conv
selection duplicate the same guard/backoff/return logic; refactor by computing
the expected arity from call_conv (e.g., map PyMethodFlags::NOARGS -> 0,
PyMethodFlags::O -> 1) and choose the Instruction variant after the check, then
perform a single shared guard that compares nargs to expected_arity and on
mismatch runs the adaptive backoff using
self.code.instructions.read_adaptive_counter(cache_base) and
write_adaptive_counter(...bytecode::adaptive_counter_backoff(...)) and returns;
update references to new_op, call_conv, nargs, cache_base, and the Instruction
variants (CallMethodDescriptorNoargs, etc.) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: a11e8867-e0c0-4327-8d74-a8a000cacd05
📒 Files selected for processing (6)
crates/derive-impl/src/pyclass.rscrates/derive-impl/src/pymodule.rscrates/derive-impl/src/util.rscrates/vm/src/coroutine.rscrates/vm/src/frame.rscrates/vm/src/function/method.rs
| let has_receiver = func | ||
| .sig() | ||
| .inputs | ||
| .iter() | ||
| .any(|arg| matches!(arg, syn::FnArg::Receiver(_))); | ||
| let drop_first_typed = match self.inner.attr_name { | ||
| AttrName::Method | AttrName::ClassMethod if !has_receiver => 1, | ||
| _ => 0, | ||
| }; | ||
| let call_flags = infer_native_call_flags(func.sig(), drop_first_typed); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n crates/derive-impl/src/pyclass.rs | sed -n '1010,1040p'Repository: RustPython/RustPython
Length of output: 1499
🏁 Script executed:
# Let's also search for raw method attributes in the codebase
rg "#\[pymethod.*raw\]|#\[pyclassmethod.*raw\]" -A 3 crates/Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Let's understand the infer_native_call_flags function to see what drop_first_typed affects
rg "fn infer_native_call_flags" -A 20 crates/Repository: RustPython/RustPython
Length of output: 1544
🏁 Script executed:
# Let's see how the `raw` variable is used after it's extracted at line 1016
sed -n '1016,1100p' crates/derive-impl/src/pyclass.rs | grep -n "raw"Repository: RustPython/RustPython
Length of output: 122
🏁 Script executed:
# Search for documentation or usage of the raw attribute
rg "raw.*method|method.*raw" -B 2 -A 2 crates/ --type rs | head -50Repository: RustPython/RustPython
Length of output: 91
🏁 Script executed:
# Find examples of raw method signatures in tests or examples
rg "#\[pymethod" -A 5 crates/ --type rs | grep -B 5 "raw" | head -100Repository: RustPython/RustPython
Length of output: 91
🏁 Script executed:
# Let's look at lines around where raw is used (1016 to 1050)
sed -n '1016,1050p' crates/derive-impl/src/pyclass.rsRepository: RustPython/RustPython
Length of output: 1305
🏁 Script executed:
# Search for raw method examples more broadly
grep -r "pymethod.*raw" crates/ | head -20Repository: RustPython/RustPython
Length of output: 118
🏁 Script executed:
# Search for raw method definitions in tests
find crates -name "*.rs" -exec grep -l "pymethod.*raw" {} \; | head -10Repository: RustPython/RustPython
Length of output: 97
🏁 Script executed:
# Let's look at the raw method example in object.rs
grep -B 5 -A 10 "#\[pymethod(raw)\]" crates/vm/src/builtins/object.rs | head -50Repository: RustPython/RustPython
Length of output: 500
🏁 Script executed:
# Find all raw method examples
grep -r "#\[pymethod(raw)\]" crates/ --include="*.rs" -A 5 | head -100Repository: RustPython/RustPython
Length of output: 465
🏁 Script executed:
# Let's check what MethodNurseryItem does with the raw flag
grep -A 30 "struct MethodNurseryItem" crates/derive-impl/src/pyclass.rsRepository: RustPython/RustPython
Length of output: 893
🏁 Script executed:
# Let's search for how the raw flag is used in code generation
grep -n "raw" crates/derive-impl/src/pyclass.rs | head -30Repository: RustPython/RustPython
Length of output: 392
🏁 Script executed:
# Let's look at how MethodNurseryItem's to_tokens handles raw
grep -A 50 "impl ToTokens for MethodNursery" crates/derive-impl/src/pyclass.rs | head -80Repository: RustPython/RustPython
Length of output: 2095
🏁 Script executed:
# Search for other raw method examples to understand the pattern
grep -r "#\[pymethod(raw)\]" crates/ --include="*.rs" -A 3 -B 1 | head -80Repository: RustPython/RustPython
Length of output: 445
🏁 Script executed:
# Let's understand what new_raw_const vs new_const do
grep -n "new_raw_const\|new_const" crates/ -r --include="*.rs" | grep -E "fn new_raw_const|fn new_const" | head -10Repository: RustPython/RustPython
Length of output: 202
🏁 Script executed:
# Let's look for more raw method examples to see the pattern
grep -r "#\[pymethod(raw)\]" crates/ --include="*.rs" -B 2 -A 8 | head -150Repository: RustPython/RustPython
Length of output: 774
🏁 Script executed:
# Let's check if raw methods are used differently in call_flags or elsewhere
grep -B 10 -A 10 "drop_first_typed" crates/derive-impl/src/pyclass.rsRepository: RustPython/RustPython
Length of output: 1054
🏁 Script executed:
# Let's look at the new_raw_const and new_const signatures to understand the difference
sed -n '76,110p' crates/vm/src/function/method.rsRepository: RustPython/RustPython
Length of output: 888
🏁 Script executed:
# Let's search for documentation or comments about raw methods
grep -r "raw.*method\|method.*raw" crates/derive-impl/ --include="*.rs" -B 2 -A 2 | head -80Repository: RustPython/RustPython
Length of output: 417
🏁 Script executed:
# Let's check if raw methods should have different call_flags handling
grep -B 5 -A 15 "fn infer_native_call_flags" crates/derive-impl/src/util.rsRepository: RustPython/RustPython
Length of output: 830
🏁 Script executed:
# Let's understand what drop_first_typed actually affects - search for where call_flags is used
grep -n "call_flags" crates/derive-impl/src/pyclass.rs | head -20Repository: RustPython/RustPython
Length of output: 433
🏁 Script executed:
# Let's check what the calling convention flags mean
grep -r "METH_O\|METH_NOARGS\|METH_VARARGS" crates/ --include="*.rs" -B 2 -A 2 | head -100Repository: RustPython/RustPython
Length of output: 831
🏁 Script executed:
# Let's trace how infer_native_call_flags uses drop_first_typed
sed -n '100,180p' crates/derive-impl/src/util.rsRepository: RustPython/RustPython
Length of output: 2496
🏁 Script executed:
# Let's see the complete infer_native_call_flags function
sed -n '100,200p' crates/derive-impl/src/util.rsRepository: RustPython/RustPython
Length of output: 3069
🏁 Script executed:
# Let's check if there are any raw methods without receiver that should help us understand the pattern
grep -r "fn.*raw\]\|#\[pymethod(raw)\]" crates/ --include="*.rs" -B 2 -A 8 | grep -A 8 "raw\]" | head -100Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Let's check how raw methods are different - search for any conditional logic based on raw
grep -n "\.raw" crates/derive-impl/src/pyclass.rsRepository: RustPython/RustPython
Length of output: 153
🏁 Script executed:
# Let's get the complete infer_native_call_flags function to see how it uses drop_first_typed
sed -n '75,150p' crates/derive-impl/src/util.rsRepository: RustPython/RustPython
Length of output: 2226
🏁 Script executed:
# Let's look at how the example raw method is actually used
sed -n '1730,1760p' crates/vm/src/builtins/object.rsRepository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Check if there are any tests or comments about raw method call_flags
grep -r "raw.*call_flags\|call_flags.*raw" crates/ --include="*.rs" -B 3 -A 3Repository: RustPython/RustPython
Length of output: 47
🏁 Script executed:
# Let's get the exact line numbers for infer_native_call_flags
grep -n "pub(crate) fn infer_native_call_flags" crates/derive-impl/src/util.rsRepository: RustPython/RustPython
Length of output: 165
🏁 Script executed:
# Get lines around the function
sed -n '75,125p' crates/derive-impl/src/util.rsRepository: RustPython/RustPython
Length of output: 1379
🏁 Script executed:
# Let's look for where drop_first_typed is used in the function
sed -n '75,160p' crates/derive-impl/src/util.rsRepository: RustPython/RustPython
Length of output: 2397
🏁 Script executed:
# Let's read the function starting from line 735
sed -n '735,820p' crates/derive-impl/src/util.rsRepository: RustPython/RustPython
Length of output: 2960
🏁 Script executed:
# Let's find the complete infer_native_call_flags function using a different approach
awk '/pub\(crate\) fn infer_native_call_flags/,/^}$/{print NR": "$0}' crates/derive-impl/src/util.rs | head -100Repository: RustPython/RustPython
Length of output: 2764
🏁 Script executed:
# Let's understand what the raw flag actually means - is it supposed to affect drop_first_typed?
grep -B 10 -A 10 "raw" crates/derive-impl/src/pyclass.rs | sed -n '1,80p'Repository: RustPython/RustPython
Length of output: 2442
🏁 Script executed:
# Let's check if there are any tests with raw methods to understand the expected behavior
find crates -name "*.rs" -exec grep -l "pymethod.*raw\|pyclassmethod.*raw" {} \;Repository: RustPython/RustPython
Length of output: 97
🏁 Script executed:
# Let's check if raw methods should have different drop_first_typed handling
grep -r "if.*raw\|raw.*if" crates/derive-impl/ --include="*.rs" -B 3 -A 3 | head -60Repository: RustPython/RustPython
Length of output: 543
🏁 Script executed:
# Let's check if there are any other raw method examples we haven't found
find crates -name "*.rs" -exec grep -B 3 -A 8 "pymethod(raw)\|pyclassmethod(raw)" {} + | head -200Repository: RustPython/RustPython
Length of output: 808
🏁 Script executed:
# Let's check if the raw flag should affect drop_first_typed by looking at how new_raw_const differs
grep -B 5 -A 25 "fn new_raw_const" crates/vm/src/function/method.rsRepository: RustPython/RustPython
Length of output: 908
🏁 Script executed:
# Let's understand the context better - what does raw mean in PyMethodDef?
grep -B 10 -A 10 "static_raw_func\|new_raw_const" crates/vm/src/function/ -r --include="*.rs" | head -100Repository: RustPython/RustPython
Length of output: 4761
🏁 Script executed:
# Let's trace through what happens with the raw method example
# The method is: fn __getstate__(vm: &VirtualMachine, args: FuncArgs)
# Let's verify what drop_first_typed does to FuncArgs
python3 <<'EOF'
# Simulate the infer_native_call_flags logic with the raw method example
sig_inputs = ["&VirtualMachine", "FuncArgs"]
# Filter out typed args (non-receiver)
typed_args = []
for arg in sig_inputs:
# Skip VirtualMachine
if arg.startswith('&') and "VirtualMachine" in arg:
continue
typed_args.append(arg)
print(f"typed_args after filtering: {typed_args}")
# Apply drop_first_typed = 1
drop_first_typed = 1
user_args = typed_args[drop_first_typed:]
print(f"user_args after dropping {drop_first_typed}: {user_args}")
# Now check what flags would be inferred
has_keywords = False
variable_arity = False
fixed_positional = 0
for ty in user_args:
if "FuncArgs" in ty:
has_keywords = True
variable_arity = True
continue
fixed_positional += 1
print(f"has_keywords={has_keywords}, variable_arity={variable_arity}, fixed_positional={fixed_positional}")
if has_keywords:
print("Result: FASTCALL | KEYWORDS")
elif variable_arity:
print("Result: FASTCALL")
else:
if fixed_positional == 0:
print("Result: NOARGS")
elif fixed_positional == 1:
print("Result: O")
else:
print("Result: FASTCALL")
EOFRepository: RustPython/RustPython
Length of output: 212
🏁 Script executed:
# Now let's check what SHOULD happen if drop_first_typed = 0 for raw
python3 <<'EOF'
sig_inputs = ["&VirtualMachine", "FuncArgs"]
typed_args = []
for arg in sig_inputs:
if arg.startswith('&') and "VirtualMachine" in arg:
continue
typed_args.append(arg)
print(f"typed_args after filtering: {typed_args}")
# Apply drop_first_typed = 0 (if raw methods were excluded)
drop_first_typed = 0
user_args = typed_args[drop_first_typed:]
print(f"user_args after dropping {drop_first_typed}: {user_args}")
has_keywords = False
variable_arity = False
fixed_positional = 0
for ty in user_args:
if "FuncArgs" in ty:
has_keywords = True
variable_arity = True
continue
fixed_positional += 1
print(f"has_keywords={has_keywords}, variable_arity={variable_arity}, fixed_positional={fixed_positional}")
if has_keywords:
print("Result: FASTCALL | KEYWORDS")
elif variable_arity:
print("Result: FASTCALL")
else:
if fixed_positional == 0:
print("Result: NOARGS")
elif fixed_positional == 1:
print("Result: O")
else:
print("Result: FASTCALL")
EOFRepository: RustPython/RustPython
Length of output: 233
🏁 Script executed:
# Let's verify the concern is valid by checking more raw method examples
grep -r "pymethod(raw)\|pyclassmethod(raw)" crates/vm/src/builtins/ --include="*.rs" -A 3 | head -60Repository: RustPython/RustPython
Length of output: 392
🏁 Script executed:
# Let's check if there are any other raw method patterns besides FuncArgs
grep -r "pymethod(raw)\|pyclassmethod(raw)" crates/ --include="*.rs" -A 5 | grep -E "fn |FuncArgs|args:" | head -40Repository: RustPython/RustPython
Length of output: 168
🏁 Script executed:
# Let's verify the interpretation: raw methods take FuncArgs directly as the first argument
# So dropping it would be wrong. Let's check if the raw flag should affect drop_first_typed
grep -B 20 "let drop_first_typed" crates/derive-impl/src/pyclass.rs | tail -30Repository: RustPython/RustPython
Length of output: 849
Raw methods without receiver should not drop the first typed argument.
The raw flag is extracted at line 1016 but not used in the drop_first_typed logic (lines 1023-1025). When #[pymethod(raw)] or #[pyclassmethod(raw)] is applied to a method without an explicit Rust receiver, the current code incorrectly sets drop_first_typed = 1, which causes the first typed argument to be skipped during call-flags inference.
Raw methods have different parameter semantics than normal methods: they receive FuncArgs directly as their first argument without an implicit self/class parameter. For example, the existing raw method __getstate__(vm: &VirtualMachine, args: FuncArgs) has no receiver and no implicit self. With the current logic, FuncArgs gets incorrectly dropped, resulting in NOARGS flags instead of the correct FASTCALL | KEYWORDS flags.
The fix should condition drop_first_typed on !raw:
let drop_first_typed = match self.inner.attr_name {
AttrName::Method | AttrName::ClassMethod if !has_receiver && !raw => 1,
_ => 0,
};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/derive-impl/src/pyclass.rs` around lines 1018 - 1028, The
drop_first_typed logic incorrectly ignores the parsed raw flag, causing methods
marked #[pymethod(raw)] or #[pyclassmethod(raw)] without a receiver to drop the
first typed argument; update the match that sets drop_first_typed (which
currently checks self.inner.attr_name and has_receiver) to also require !raw
before returning 1 so that raw methods do not decrement the first typed arg,
then pass the unchanged drop_first_typed into
infer_native_call_flags(func.sig(), drop_first_typed).
1 participant
Summary
send_nonefastpath inCoroto skip theis_nonecheck and type-error branch for the common case (generator iteration viaforloops)CALL_METHOD_DESCRIPTOR_*guards to verifyselftype matchesdescr.objclass, matching CPython'sCALL_METHOD_DESCRIPTORtype checkslist.appendon subclasses inCALL_LIST_APPEND(usedowncast_refinstead ofdowncast_ref_if_exact)specialization_eval_frame_activeguard to prevent specializingCALL/CALL_KW/SEND/FOR_ITER_GENwhen custom eval frames are activeFOR_ITER_GENto deoptimize on guard miss and checkrunning()state beforesend_noneSEND_GENto deoptimize on guard miss and addsend_nonefastpath forNonevaluesspecialize_sendto require exact generator/coroutine typesspecialize_for_iterto validateEND_FORshape and jump delta bounds for generator specializationCALL_METHOD_DESCRIPTOR_FASTthroughCALL_METHOD_DESCRIPTOR_FAST_WITH_KEYWORDSfor correct nargs > 1 dispatchCallBuiltinFastWithKeywordsfallback for builtin calls with effective_nargs > 1Test plan
test_bdbgenerator stepping works correctly with ForIterGen deoptimize pathtest_generatorspasses with send_none fastpath🤖 Generated with Claude Code
Summary by CodeRabbit