check for void return FFI functions, fix incorrect cases

newsfragments/6043.fixed.md

@@ -0,0 +1 @@
+Fix FFI definition `PyErr_SetInterruptEx` missing `c_int` return type.

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,34 +572,40 @@ 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" => {
                 FunctionInfo {
                     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("->"),
     })
 }
 

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)*)?;
         }};
     }
 

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

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")]