Skip to content

Conversation

@cyyever
Copy link
Contributor

@cyyever cyyever commented Dec 9, 2025

Description

This PR replaces some C-style casts with C++ ones.

Suggested changelog entry:

  • Many C-style casts were replaced with C++-style casts.

@rwgk
Copy link
Collaborator

rwgk commented Dec 13, 2025

I used Cursor like this:


Prompt

Could you please look at git diff master?

This is pybind11 PR #5930

The changes look great to me. Could you please review them to see if the C++-style casts are the best choices in all cases?

(I forgot to add: GHA testing passes, except for known issues)

Review

Overall

All of the C++-style casts introduced in this diff look appropriate, and I don’t see any cases where a different cast (static_cast, const_cast, dynamic_cast) would be safer or clearer given the CPython C-API patterns. The changes are mostly mechanical replacements of C‑style casts, with reinterpret_cast used for Python C-API struct-pointer conversions and static_cast used for numeric and void * conversions, which is exactly what you want.

Per-file notes (brief)

  • attr.h: reinterpret_cast<PyObject *>(base_info->type) is correct: PyTypeObject * and PyObject * are unrelated C structs in C++, so reinterpret_cast is the only “honest” cast. The static_cast<size_t>(has_args / has_kwargs) is fine (even optional, but explicit is clearer).

  • buffer_info.h: static_cast<ssize_t>(shape.size()/strides.size()) and static_cast<size_t>(ndim) are the right direction of cast and just make the previous intent explicit; same for static_cast<size_t>(b.itemsize).

  • cast.h: Passing reinterpret_cast<PyTypeObject *>(type::handle_of(h).ptr()) into all_type_info is appropriate; there is no safer cast for that C-API struct conversion. The static_cast<size_t>(PYBIND11_BYTES_SIZE(...)) is the right numeric cast (it was already doing that conceptually with a C-style cast).

  • detail/argument_vector.h: Using static_cast<std::size_t>(-1) and static_cast<std::size_t>(1) << bit is the correct typed way to express the existing “all bits set” / “bit mask” idioms.

  • detail/class.h: All the conversions between PyObject *, PyTypeObject *, and PyHeapTypeObject * now use reinterpret_cast, which is exactly what’s needed for CPython’s “struct inheritance” in C++; static_cast would not be valid here. static_cast<char *>(PyMem_MALLOC(size)) is the right choice for void *char *.

  • detail/cpp_conduit.h: The bool-casts were redundant; returning the boolean expression directly is fine and keeps the function signatures clear.

  • detail/function_record_pyobject.h: reinterpret_cast<detail::function_record_PyObject *>(obj) and reinterpret_steal<object>(reinterpret_cast<PyObject *>(py_func_rec)) are appropriate: these are known layout reinterprets for a custom Python heap type.

  • detail/init.h: Casting v_h.inst to PyObject * via reinterpret_cast when interacting with getattr/setattr is correct: instance is a CPython heap object struct, so again this is the right kind of cast.

  • detail/internals.h: Just removing the redundant null-check before delete; no cast issues here.

  • detail/type_caster_base.h: All new reinterpret_cast<PyTypeObject *> and reinterpret_cast<PyObject *> uses are in the same pattern as elsewhere and are necessary. static_cast<void **>(PyMem_Calloc(...)) is the right cast from void * for an allocation.

  • detail/value_and_holder.h: The bitmask updates now use static_cast<std::uint8_t>(~instance::status_...); this is a faithful, type-safe replacement for the old C-style casts and still correctly truncates the complemented mask to 8 bits.

  • pybind11.h: Pointer casts between PyObject *, PyTypeObject *, PyHeapTypeObject *, and the various internal structs (function_record_PyObject, PyCFunctionObject, instance) are all correctly expressed with reinterpret_cast. static_cast<std::uint16_t>(args) and static_cast<size_t>(PyTuple_GET_SIZE(...)) accurately document the narrowing/widening already happening.

  • pytypes.h: handle(reinterpret_cast<PyObject *>(Py_TYPE(h.ptr()))) is the right way to turn a PyTypeObject * into a handle. All the size/length conversions now using static_cast<size_t> and the float conversions using static_cast<double> / static_cast<float> are straightforward and correct; the as_unsigned changes preserve the established “(Unsigned)-1 sentinel” behaviour.

Bottom line

I don’t see any misused C++-style casts in this PR; the choices between reinterpret_cast and static_cast are consistent with CPython’s C-API conventions and with pybind11’s existing style, and there are no places where a safer cast type is obviously available. Any further tweaks would be purely stylistic, not correctness or safety fixes.

Copy link
Collaborator

@rwgk rwgk left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for doing this.

@rwgk rwgk merged commit 032e73d into pybind:master Dec 13, 2025
81 of 85 checks passed
@github-actions github-actions bot added the needs changelog Possibly needs a changelog entry label Dec 13, 2025
@cyyever cyyever deleted the c_cast branch December 13, 2025 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs changelog Possibly needs a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants