X Tutup
Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 21 additions & 3 deletions crates/derive-impl/src/pyclass.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use super::Diagnostic;
use crate::util::{
ALL_ALLOWED_NAMES, ClassItemMeta, ContentItem, ContentItemInner, ErrorVec, ExceptionItemMeta,
ItemMeta, ItemMetaInner, ItemNursery, SimpleItemMeta, format_doc, pyclass_ident_and_attrs,
pyexception_ident_and_attrs, text_signature,
ItemMeta, ItemMetaInner, ItemNursery, SimpleItemMeta, format_doc, infer_native_call_flags,
pyclass_ident_and_attrs, pyexception_ident_and_attrs, text_signature,
};
use core::str::FromStr;
use proc_macro2::{Delimiter, Group, Span, TokenStream, TokenTree};
Expand Down Expand Up @@ -1015,6 +1015,16 @@ where

let raw = item_meta.raw()?;
let sig_doc = text_signature(func.sig(), &py_name);
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);

Comment on lines +1018 to 1028
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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 -50

Repository: 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 -100

Repository: 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.rs

Repository: RustPython/RustPython

Length of output: 1305


🏁 Script executed:

# Search for raw method examples more broadly
grep -r "pymethod.*raw" crates/ | head -20

Repository: 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 -10

Repository: 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 -50

Repository: RustPython/RustPython

Length of output: 500


🏁 Script executed:

# Find all raw method examples
grep -r "#\[pymethod(raw)\]" crates/ --include="*.rs" -A 5 | head -100

Repository: 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.rs

Repository: 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 -30

Repository: 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 -80

Repository: 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 -80

Repository: 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 -10

Repository: 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 -150

Repository: 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.rs

Repository: 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.rs

Repository: 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 -80

Repository: 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.rs

Repository: 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 -20

Repository: 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 -100

Repository: 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.rs

Repository: 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.rs

Repository: 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 -100

Repository: 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.rs

Repository: 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.rs

Repository: 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.rs

Repository: 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 3

Repository: 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.rs

Repository: RustPython/RustPython

Length of output: 165


🏁 Script executed:

# Get lines around the function
sed -n '75,125p' crates/derive-impl/src/util.rs

Repository: 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.rs

Repository: 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.rs

Repository: 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 -100

Repository: 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 -60

Repository: 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 -200

Repository: 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.rs

Repository: 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 -100

Repository: 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")
EOF

Repository: 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")
EOF

Repository: 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 -60

Repository: 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 -40

Repository: 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 -30

Repository: 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).

// Add #[allow(non_snake_case)] for setter methods like set___name__
let method_name = ident.to_string();
Expand All @@ -1031,6 +1041,7 @@ where
doc,
raw,
attr_name: self.inner.attr_name,
call_flags,
});
Ok(())
}
Expand Down Expand Up @@ -1248,6 +1259,7 @@ struct MethodNurseryItem {
raw: bool,
doc: Option<String>,
attr_name: AttrName,
call_flags: TokenStream,
}

impl MethodNursery {
Expand Down Expand Up @@ -1278,7 +1290,7 @@ impl ToTokens for MethodNursery {
} else {
quote! { None }
};
let flags = match &item.attr_name {
let binding_flags = match &item.attr_name {
AttrName::Method => {
quote! { rustpython_vm::function::PyMethodFlags::METHOD }
}
Expand All @@ -1290,6 +1302,12 @@ impl ToTokens for MethodNursery {
}
_ => unreachable!(),
};
let call_flags = &item.call_flags;
let flags = quote! {
rustpython_vm::function::PyMethodFlags::from_bits_retain(
(#binding_flags).bits() | (#call_flags).bits()
)
};
// TODO: intern
// let py_name = if py_name.starts_with("__") && py_name.ends_with("__") {
// let name_ident = Ident::new(&py_name, ident.span());
Expand Down
9 changes: 6 additions & 3 deletions crates/derive-impl/src/pymodule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ use crate::error::Diagnostic;
use crate::pystructseq::PyStructSequenceMeta;
use crate::util::{
ALL_ALLOWED_NAMES, AttrItemMeta, AttributeExt, ClassItemMeta, ContentItem, ContentItemInner,
ErrorVec, ItemMeta, ItemNursery, ModuleItemMeta, SimpleItemMeta, format_doc, iter_use_idents,
pyclass_ident_and_attrs, text_signature,
ErrorVec, ItemMeta, ItemNursery, ModuleItemMeta, SimpleItemMeta, format_doc,
infer_native_call_flags, iter_use_idents, pyclass_ident_and_attrs, text_signature,
};
use core::str::FromStr;
use proc_macro2::{Delimiter, Group, TokenStream, TokenTree};
Expand Down Expand Up @@ -525,6 +525,7 @@ struct FunctionNurseryItem {
cfgs: Vec<Attribute>,
ident: Ident,
doc: String,
call_flags: TokenStream,
}

impl FunctionNursery {
Expand All @@ -550,14 +551,14 @@ struct ValidatedFunctionNursery(FunctionNursery);
impl ToTokens for ValidatedFunctionNursery {
fn to_tokens(&self, tokens: &mut TokenStream) {
let mut inner_tokens = TokenStream::new();
let flags = quote! { rustpython_vm::function::PyMethodFlags::empty() };
for item in &self.0.items {
let ident = &item.ident;
let cfgs = &item.cfgs;
let cfgs = quote!(#(#cfgs)*);
let py_names = &item.py_names;
let doc = &item.doc;
let doc = quote!(Some(#doc));
let flags = &item.call_flags;

inner_tokens.extend(quote![
#(
Expand Down Expand Up @@ -706,12 +707,14 @@ impl ModuleItem for FunctionItem {
py_names
}
};
let call_flags = infer_native_call_flags(func.sig(), 0);

args.context.function_items.add_item(FunctionNurseryItem {
ident: ident.to_owned(),
py_names,
cfgs: args.cfgs.to_vec(),
doc,
call_flags,
});
Ok(())
}
Expand Down
71 changes: 71 additions & 0 deletions crates/derive-impl/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,77 @@ pub(crate) fn text_signature(sig: &Signature, name: &str) -> String {
}
}

pub(crate) fn infer_native_call_flags(sig: &Signature, drop_first_typed: usize) -> TokenStream {
// Best-effort mapping of Rust function signatures to CPython-style
// METH_* calling convention flags used by CALL specialization.
let mut typed_args = Vec::new();
for arg in &sig.inputs {
let syn::FnArg::Typed(typed) = arg else {
continue;
};
let ty_tokens = &typed.ty;
let ty = quote!(#ty_tokens).to_string().replace(' ', "");
// `vm: &VirtualMachine` is not a Python-level argument.
if ty.starts_with('&') && ty.ends_with("VirtualMachine") {
continue;
}
typed_args.push(ty);
}

let mut user_args = typed_args.into_iter();
for _ in 0..drop_first_typed {
if user_args.next().is_none() {
break;
}
}

let mut has_keywords = false;
let mut variable_arity = false;
let mut fixed_positional = 0usize;

for ty in user_args {
let is_named = |name: &str| {
ty == name
|| ty.starts_with(&format!("{name}<"))
|| ty.contains(&format!("::{name}<"))
|| ty.ends_with(&format!("::{name}"))
};

if is_named("FuncArgs") {
has_keywords = true;
variable_arity = true;
continue;
}
if is_named("KwArgs") {
has_keywords = true;
variable_arity = true;
continue;
}
if is_named("PosArgs") || is_named("OptionalArg") || is_named("OptionalOption") {
variable_arity = true;
continue;
}
fixed_positional += 1;
}

if has_keywords {
quote! {
rustpython_vm::function::PyMethodFlags::from_bits_retain(
rustpython_vm::function::PyMethodFlags::FASTCALL.bits()
| rustpython_vm::function::PyMethodFlags::KEYWORDS.bits()
)
}
} else if variable_arity {
quote! { rustpython_vm::function::PyMethodFlags::FASTCALL }
} else {
match fixed_positional {
0 => quote! { rustpython_vm::function::PyMethodFlags::NOARGS },
1 => quote! { rustpython_vm::function::PyMethodFlags::O },
_ => quote! { rustpython_vm::function::PyMethodFlags::FASTCALL },
}
}
}

fn func_sig(sig: &Signature) -> String {
sig.inputs
.iter()
Expand Down
57 changes: 40 additions & 17 deletions crates/vm/src/coroutine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,27 +115,12 @@ impl Coro {
result
}

pub fn send(
fn finalize_send_result(
&self,
jen: &PyObject,
value: PyObjectRef,
result: PyResult<ExecutionResult>,
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.maybe_close(&result);
match result {
Ok(exec_res) => Ok(exec_res.into_iter_return(vm)),
Expand All @@ -158,6 +143,44 @@ impl Coro {
}
}

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)
}

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)
}

pub fn throw(
&self,
jen: &PyObject,
Expand Down
Loading
Loading
X Tutup