Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions newsfragments/6042.removed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove private FFI definitions `_PyBytes_Resize` and `_PyErr_BadInternalCall`.
8 changes: 7 additions & 1 deletion noxfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -1184,7 +1184,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)


Expand Down
8 changes: 7 additions & 1 deletion pyo3-ffi-check/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
23 changes: 14 additions & 9 deletions pyo3-ffi-check/macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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" {
Expand Down Expand Up @@ -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_XDECREF", ""),
Expand Down Expand Up @@ -489,9 +495,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
Expand Down Expand Up @@ -704,9 +707,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();
Expand Down
2 changes: 1 addition & 1 deletion pyo3-ffi/src/cpython/bytesobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down
10 changes: 5 additions & 5 deletions pyo3-ffi/src/cpython/traceback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
7 changes: 5 additions & 2 deletions pyo3-ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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)>,
}
Comment on lines +392 to +395
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This change was motivated by looking at the nomicon https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs - which documents this _data / _marker combination as the current way to do opaque types.

};
}

Expand Down
17 changes: 16 additions & 1 deletion pyo3-ffi/src/pyerrors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
pub fn _PyErr_BadInternalCall(filename: *const c_char, lineno: c_int);

#[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,
Expand Down
Loading