more cleanup of private FFI definitions (#6042)

* more cleanup of private FFI definitions

* newsfragment

* remove incorrect exclusions

* fixup clippy

* fixup exclusions

* fix missing x86 link name

Author: davidhewitt

newsfragments/6042.removed.md

@@ -0,0 +1 @@
+Remove private FFI definitions `_PyBytes_Resize` and `_PyErr_BadInternalCall`.

noxfile.py

@@ -1203,7 +1203,13 @@ def load_pkg_versions():
 
 @nox.session(name="ffi-check")
 def ffi_check(session: nox.Session):
-    _run_cargo(session, "run", _FFI_CHECK, "--message-format=short")
+    extra_args = []
+    # This flag can be useful for debugging ffi-check errors, but overall the
+    # short message format is easier to read
+    if "--long-message-format" not in session.posargs:
+        extra_args.append("--message-format=short")
+
+    _run_cargo(session, "run", _FFI_CHECK, *extra_args)
     _check_raw_dylib_macro(session)
 
 

pyo3-ffi-check/build.rs

@@ -10,7 +10,13 @@ fn main() {
     // write docs into the build script output directory, they will be read
     // by the proc macro to resolve what definitions exist
     let status = std::process::Command::new("cargo")
-        .args(["doc", "-p", "pyo3-ffi-check-definitions", "--no-deps"])
+        .args([
+            "doc",
+            "-p",
+            "pyo3-ffi-check-definitions",
+            "--no-deps",
+            "--document-private-items",
+        ])
         .env("CARGO_TARGET_DIR", out_dir)
         // forward target to the doc buid to ensure `--target` is honored
         .env("CARGO_BUILD_TARGET", target)

pyo3-ffi-check/macro/src/lib.rs

@@ -155,13 +155,17 @@ pub fn for_all_fields(input: proc_macro::TokenStream) -> proc_macro::TokenStream
     }
 
     let pyo3_ffi_fields = get_fields_from_file(&pyo3_ffi_struct_file);
-    let bindgen_fields = get_fields_from_file(&bindgen_struct_file);
 
-    if pyo3_ffi_fields.is_empty() {
-        // probably an opaque type on PyO3 side, skip
+    if pyo3_ffi_fields.len() == 2
+        && pyo3_ffi_fields.contains(&"_data".to_string())
+        && pyo3_ffi_fields.contains(&"_marker".to_string())
+    {
+        // looks like an opaque type on the PyO3 side, skip
         return TokenStream::new().into();
     }
 
+    let bindgen_fields = get_fields_from_file(&bindgen_struct_file);
+
     let mut all_fields: HashSet<_> = pyo3_ffi_fields.into_iter().chain(bindgen_fields).collect();
 
     if struct_name == "PyMemberDef" {
@@ -425,9 +429,11 @@ const MACRO_EXCLUSIONS: &[(&str, &str)] = &[
     ("Py_False", ""),
     ("Py_GETENV", "not(Py_3_11)"),
     ("Py_INCREF", ""),
+    ("Py_IS_TYPE", "not(Py_3_15)"), // symbol added for stable abi on 3.15
     ("Py_None", ""),
     ("Py_NotImplemented", ""),
     ("Py_REFCNT", "not(Py_3_14)"),
+    ("Py_SIZE", "not(Py_3_15)"), // symbol added for stable abi on 3.15
     ("Py_True", ""),
     ("Py_TYPE", "not(Py_3_14)"),
     ("Py_UNICODE_TODECIMAL", ""),
@@ -476,9 +482,6 @@ const EXCLUDED_SYMBOLS: &[&str] = &[
     "PyOS_BeforeFork",
     "PyOS_AfterFork_Parent",
     "PyOS_AfterFork_Child",
-    // See https://github.com/python/cpython/pull/139166/changes#r3214904694
-    "Py_IS_TYPE",
-    "Py_SIZE",
 ];
 
 // Assert at compile time that `MACRO_EXCLUSIONS` and `EXCLUDED_SYMBOLS` are disjoint
@@ -718,9 +721,11 @@ fn get_function_info(
     static FUNCTION_DECL_REGEX: LazyLock<regex::Regex> =
         LazyLock::new(|| regex::Regex::new(r"^pub\s+(.*?)\sfn\s+([^(<]*)").unwrap());
 
-    let captures = FUNCTION_DECL_REGEX
-        .captures(&text)
-        .expect("failed to parse function declaration with regex");
+    let Some(captures) = FUNCTION_DECL_REGEX.captures(&text) else {
+        panic!(
+            "failed to parse function declaration for `{function_name}` with regex, got: {text}"
+        );
+    };
 
     // find modifiers, e.g. `unsafe extern "C"`
     let modifiers = captures.get(1).unwrap().as_str().parse().unwrap();

pyo3-ffi/src/cpython/bytesobject.rs

@@ -23,7 +23,7 @@ opaque_struct!(pub PyBytesObject);
 
 extern_libpython! {
     #[cfg_attr(PyPy, link_name = "_PyPyBytes_Resize")]
-    pub fn _PyBytes_Resize(bytes: *mut *mut PyObject, newsize: Py_ssize_t) -> c_int;
+    pub(crate) fn _PyBytes_Resize(bytes: *mut *mut PyObject, newsize: Py_ssize_t) -> c_int;
 }
 
 #[cfg(not(Py_LIMITED_API))]

pyo3-ffi/src/cpython/traceback.rs

@@ -4,9 +4,9 @@ use core::ffi::c_int;
 
 #[repr(C)]
 pub struct PyTracebackObject {
-    ob_base: PyObject,
-    tb_next: *mut PyTracebackObject,
-    tb_frame: *mut PyFrameObject,
-    tb_lasti: c_int,
-    tb_lineno: c_int,
+    pub ob_base: PyObject,
+    pub tb_next: *mut PyTracebackObject,
+    pub tb_frame: *mut PyFrameObject,
+    pub tb_lasti: c_int,
+    pub tb_lineno: c_int,
 }

pyo3-ffi/src/impl_/macros.rs

@@ -158,6 +158,12 @@ macro_rules! extern_libpython_maybe_private_fn {
     ) => {
         extern_libpython_cpython_private_fn! { $(#[$attrs])* $vis $name($($args)*) $(-> $ret)? }
     };
+    (
+        [_PyErr_BadInternalCall]
+        $(#[$attrs:meta])* $vis:vis fn $name:ident($($args:tt)*) $(-> $ret:ty)?
+    ) => {
+        extern_libpython_cpython_private_fn! { $(#[$attrs])* $vis $name($($args)*) $(-> $ret)? }
+    };
     (
         [$name:ident]
         $(#[$attrs:meta])* $vis:vis fn $fn_name:ident($($args:tt)*) $(-> $ret:ty)?

pyo3-ffi/src/lib.rs

@@ -377,7 +377,7 @@
 // original CPython headers
 #![allow(unsafe_op_in_unsafe_fn)]
 
-#[cfg(all(Py_3_12, py_sys_config = "Py_REF_DEBUG"))]
+#[cfg(not(PyPy))]
 extern crate alloc;
 #[cfg(not(any(Py_3_14, target_arch = "wasm32")))]
 extern crate std;
@@ -389,7 +389,10 @@ macro_rules! opaque_struct {
     ($(#[$attrs:meta])* $pub:vis $name:ident) => {
         $(#[$attrs])*
         #[repr(C)]
-        $pub struct $name([u8; 0]);
+        $pub struct $name {
+            _data: (),
+            _marker: core::marker::PhantomData<(*mut u8, core::marker::PhantomPinned)>,
+        }
     };
 }
 

pyo3-ffi/src/pyerrors.rs

@@ -310,9 +310,24 @@ extern_libpython! {
         arg2: *mut PyObject,
         arg3: *mut PyObject,
     ) -> *mut PyObject;
+    #[cfg(PyPy)]
     #[cfg_attr(PyPy, link_name = "PyPyErr_BadInternalCall")]
     pub fn PyErr_BadInternalCall();
-    // skipped _PyErr_BadInternalCall
+
+    #[cfg(not(PyPy))]
+    fn _PyErr_BadInternalCall(filename: *const c_char, lineno: c_int);
+}
+
+#[cfg(not(PyPy))]
+#[track_caller]
+#[inline]
+pub unsafe fn PyErr_BadInternalCall() {
+    let location = core::panic::Location::caller();
+    let filename = alloc::ffi::CString::new(location.file()).unwrap();
+    unsafe { _PyErr_BadInternalCall(filename.as_ptr(), location.line() as c_int) }
+}
+
+extern_libpython! {
     #[cfg_attr(PyPy, link_name = "PyPyErr_NewException")]
     pub fn PyErr_NewException(
         name: *const c_char,