From 25b165adbddc1f9eb5fca69da741c0795a1487af Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Wed, 13 May 2026 19:14:40 +0100 Subject: [PATCH 1/2] check for void return FFI functions, fix incorrect cases --- pyo3-ffi-check/macro/src/lib.rs | 38 +++++++++++++++++++++++++++++++-- pyo3-ffi-check/src/main.rs | 12 +++++------ pyo3-ffi/src/cpython/object.rs | 2 +- pyo3-ffi/src/pyerrors.rs | 2 +- 4 files changed, 44 insertions(+), 10 deletions(-) diff --git a/pyo3-ffi-check/macro/src/lib.rs b/pyo3-ffi-check/macro/src/lib.rs index 399e87b9270..3b8d1caa781 100644 --- a/pyo3-ffi-check/macro/src/lib.rs +++ b/pyo3-ffi-check/macro/src/lib.rs @@ -560,6 +560,7 @@ pub fn for_all_functions(_input: proc_macro::TokenStream) -> proc_macro::TokenSt modifiers, arg_count, variadic, + void_return, } = match (function_name, get_function_info(function_name, &entry)) { (_, Ok(info)) => info, // In some cases symbols and macros differ only by case, which is a problem for case-insensitive filesystems. @@ -571,27 +572,32 @@ pub fn for_all_functions(_input: proc_macro::TokenStream) -> proc_macro::TokenSt modifiers: quote!(), arg_count: 1, variadic: false, + void_return: true, }, ("Py_IncRef", Err(FunctionNameMismatch(e))) if e == "Py_INCREF" => FunctionInfo { modifiers: quote!(extern "C"), arg_count: 1, variadic: false, + void_return: true, }, ("Py_DECREF", Err(FunctionNameMismatch(e))) if e == "Py_DecRef" => FunctionInfo { modifiers: quote!(), arg_count: 1, variadic: false, + void_return: true, }, ("Py_DecRef", Err(FunctionNameMismatch(e))) if e == "Py_DECREF" => FunctionInfo { modifiers: quote!(extern "C"), arg_count: 1, variadic: false, + void_return: true, }, ("PyThreadState_GET", Err(FunctionNameMismatch(e))) if e == "PyThreadState_Get" => { FunctionInfo { modifiers: quote!(), arg_count: 0, variadic: false, + void_return: false, } } ("PyThreadState_Get", Err(FunctionNameMismatch(e))) if e == "PyThreadState_GET" => { @@ -599,6 +605,7 @@ pub fn for_all_functions(_input: proc_macro::TokenStream) -> proc_macro::TokenSt modifiers: quote!(extern "C"), arg_count: 0, variadic: false, + void_return: false, } } (function_name, Err(FunctionNameMismatch(unexpected))) => { @@ -614,6 +621,8 @@ pub fn for_all_functions(_input: proc_macro::TokenStream) -> proc_macro::TokenSt let arg_types = std::iter::repeat_n(quote!(_), arg_count); + let retval = if void_return { quote!() } else { quote!(-> _) }; + let vararg = if variadic { Some(quote!(, ...)) } else { None }; // if the function is not extern "C": @@ -639,6 +648,23 @@ pub fn for_all_functions(_input: proc_macro::TokenStream) -> proc_macro::TokenSt let has_symbol = BINDGEN_FUNCTION_NAMES.contains(function_name); + if has_symbol { + if let Ok(FunctionInfo { + void_return: bindgen_void_return, + .. + }) = get_function_info( + function_name, + &DOC_DIR.join(format!("bindgen/fn.{}.html", function_name)), + ) { + if void_return != bindgen_void_return { + let error_message = format!( + "void return mismatch between pyo3-ffi and bindgen for `{function_name}`: pyo3-ffi has void return {void_return}, but bindgen has void return {bindgen_void_return}", + ); + output.extend(quote!(compile_error!(#error_message);)); + } + } + } + match (macro_exclusion_cfg, has_symbol) { (Some(cfg), true) => { // emit an error if checking within the cfgs where a macro is expected @@ -648,7 +674,7 @@ pub fn for_all_functions(_input: proc_macro::TokenStream) -> proc_macro::TokenSt output.extend(quote!(#[cfg(#cfg)] compile_error!(#error_message);)); // if not within the macro range, we found a symbol, this should be good output.extend( - quote!(#[cfg(not(#cfg))] #macro_name!(#inline #function_ident, #modifiers (#(#arg_types),* #vararg));), + quote!(#[cfg(not(#cfg))] #macro_name!(#inline #function_ident, #modifiers (#(#arg_types),* #vararg) #retval);), ); } (Some(cfg), false) => { @@ -662,7 +688,7 @@ pub fn for_all_functions(_input: proc_macro::TokenStream) -> proc_macro::TokenSt (None, true) => { // emit the comparison macro to check that the argument count matches output.extend( - quote!(#macro_name!(#inline #function_ident, #modifiers (#(#arg_types),* #vararg));), + quote!(#macro_name!(#inline #function_ident, #modifiers (#(#arg_types),* #vararg) #retval);), ); } (None, false) => { @@ -682,6 +708,7 @@ struct FunctionInfo { modifiers: TokenStream, // e.g. `unsafe extern "C"`, empty for no modifiers arg_count: usize, // not including the "..." for variadic functions variadic: bool, + void_return: bool, // whether the function returns void (i.e. has no return type in C) } // Error returned when the function definition does not match the expected name of the file @@ -758,10 +785,17 @@ fn get_function_info( arg_count += 1; } + let end_paren = args_begin[end..] + .find(')') + .expect("function declaration should have closing paren after arguments"); + + let after_parens = args_begin[end + end_paren + 1..].trim_start(); + Ok(FunctionInfo { modifiers, arg_count, variadic, + void_return: !after_parens.contains("->"), }) } diff --git a/pyo3-ffi-check/src/main.rs b/pyo3-ffi-check/src/main.rs index 0a88cfaf3b3..56d6bd37ffa 100644 --- a/pyo3-ffi-check/src/main.rs +++ b/pyo3-ffi-check/src/main.rs @@ -74,17 +74,17 @@ fn main() { // This macro attempts to check that both functions exist and have the same number of arguments, it is // difficult to check the argument types match. macro_rules! check_function { - ($name:ident, [$($modifiers:tt)*] ($($arg_types:tt)*)) => {{ + ($name:ident, [$($modifiers:tt)*] ($($arg_types:tt)*) $(-> $($retval:tt)*)?) => {{ #[allow(deprecated)] - { pyo3_ffi::$name as $($modifiers)* fn($($arg_types)*) -> _ }; - bindings::$name as $($modifiers)* fn($($arg_types)*) -> _; + { pyo3_ffi::$name as $($modifiers)* fn($($arg_types)*) $(-> $($retval)*)? }; + bindings::$name as $($modifiers)* fn($($arg_types)*) $(-> $($retval)*)?; }}; // case when the function is an inline function in the headers, in which case pyo3-ffi will use the // Rust abi and the extern symbol uses the C abi - (@inline $name:ident, ($($arg_types:tt)*)) => {{ + (@inline $name:ident, ($($arg_types:tt)*) $(-> $($retval:tt)*)?) => {{ #[allow(deprecated)] - { pyo3_ffi::$name as unsafe fn($($arg_types)*) -> _ }; - bindings::$name as unsafe extern "C" fn($($arg_types)*) -> _; + { pyo3_ffi::$name as unsafe fn($($arg_types)*) $(-> $($retval)*)? }; + bindings::$name as unsafe extern "C" fn($($arg_types)*) $(-> $($retval)*)?; }}; } diff --git a/pyo3-ffi/src/cpython/object.rs b/pyo3-ffi/src/cpython/object.rs index 8a7ce069cc8..60f85727994 100644 --- a/pyo3-ffi/src/cpython/object.rs +++ b/pyo3-ffi/src/cpython/object.rs @@ -409,7 +409,7 @@ extern_libpython! { pub fn PyUnstable_TryIncRef(obj: *mut PyObject) -> c_int; - pub fn PyUnstable_EnableTryIncRef(obj: *mut PyObject) -> c_void; + pub fn PyUnstable_EnableTryIncRef(obj: *mut PyObject); pub fn PyUnstable_Object_IsUniquelyReferenced(op: *mut PyObject) -> c_int; } diff --git a/pyo3-ffi/src/pyerrors.rs b/pyo3-ffi/src/pyerrors.rs index 505c4806a35..a49b15f0da7 100644 --- a/pyo3-ffi/src/pyerrors.rs +++ b/pyo3-ffi/src/pyerrors.rs @@ -334,7 +334,7 @@ extern_libpython! { pub fn PyErr_SetInterrupt(); #[cfg(Py_3_10)] #[cfg_attr(PyPy, link_name = "PyPyErr_SetInterruptEx")] - pub fn PyErr_SetInterruptEx(signum: c_int); + pub fn PyErr_SetInterruptEx(signum: c_int) -> c_int; #[cfg_attr(PyPy, link_name = "PyPyErr_SyntaxLocation")] pub fn PyErr_SyntaxLocation(filename: *const c_char, lineno: c_int); #[cfg_attr(PyPy, link_name = "PyPyErr_SyntaxLocationEx")] From 6c136d0967717433706631f0033950f8e74eee55 Mon Sep 17 00:00:00 2001 From: David Hewitt Date: Wed, 13 May 2026 19:18:37 +0100 Subject: [PATCH 2/2] newsfragment --- newsfragments/6043.fixed.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/6043.fixed.md diff --git a/newsfragments/6043.fixed.md b/newsfragments/6043.fixed.md new file mode 100644 index 00000000000..901e9ccec4f --- /dev/null +++ b/newsfragments/6043.fixed.md @@ -0,0 +1 @@ +Fix FFI definition `PyErr_SetInterruptEx` missing `c_int` return type.