Repository navigation
fix: support implicit value conversions in native trampolines - #6204
Conversation
rwgk
left a comment
There was a problem hiding this comment.
codex GPT-6.1-Sol xhigh:
No actionable findings in PR #6204 at 6d2efbb. The lifetime handling preserves enclosing frames and excludes borrowed returns.
I also looked into "Why is the new test a C++ test?"
codex:
A typical Python test would call a bound C++ helper, and pybind11’s dispatcher would establish the lifetime-support frame before entering that helper. The trampoline conversion would then succeed even without this fix, hiding the bug.
The embedded C++ test calls the virtual method directly from native C++, with no dispatcher frame active. It still runs a Python override, but reproduces the crucial missing-frame condition.
A Python test could reproduce this by arranging the call on a fresh C++ thread, for example, but the embedded C++ test expresses the scenario more directly.
| cast_safe(object &&o) { | ||
| // A registered type returned by value is copied or moved before this frame ends. | ||
| // Preserve an enclosing frame, and never shorten the lifetime of borrowed results. | ||
| if (std::is_base_of<type_caster_generic, make_caster<T>>::value && !std::is_reference<T>::value |
There was a problem hiding this comment.
codex GPT-6.1-Sol xhigh narration for this change:
The change supplies temporary-object lifetime support when native C++ calls a Python override that returns a value requiring implicit conversion—for example, a Python list converted to an opaque C++ vector.
Normally, a Python-to-C++ bound call already has a loader_life_support frame. Implicit conversion registers its temporary Python wrapper with that frame, keeping the underlying C++ object alive during conversion. A call originating in native C++ can lack that frame, so registration throws. Existing lifetime-support code
The first two production edits add a query for that situation: loader_life_support::is_active() is declared in the header and defined in the implementation header. It simply checks whether the existing thread-local current-frame pointer is non-null. It adds no state.
The behavioral change is in detail::cast_safe<T>(), which handles trampoline return conversion. Previously it immediately called pybind11::cast<T>(). Now it first checks three conditions:
- The caster derives from
type_caster_generic, covering registered C++ types and their holder casters. Tis returned by value, rather than by reference or pointer.- No lifetime-support frame is already active.
When all three hold, it creates a local frame and performs the same cast:
loader_life_support life_support;
return pybind11::cast<T>(std::move(o));The order matters: C++ constructs the returned T before destroying life_support. Thus the implicit-conversion temporary survives until its value has been copied or moved into the result. The frame then releases its temporary objects; stack unwinding also cleans it up if conversion throws.
If an enclosing frame already exists, execution takes the original cast path, so conversion temporaries remain attached to that longer-lived frame. Reference and pointer returns also keep their previous behavior: a frame ending inside cast_safe() would expire too early to support a borrowed result. The caster restriction similarly leaves ordinary string-view conversions on their existing path.
Description
Fixes #6080.
When native C++ calls a Python override, an implicit conversion of the returned list to an opaque vector can fail because no loader frame is active. Keep conversion temporaries alive until the registered return value has been copied or moved out. Reuse an existing frame, and leave borrowed reference, pointer, and view returns unchanged.
The embedded-interpreter regression covers retained and temporary Python lists, shared ownership, temporary destruction, and rejection of borrowed results that would outlive a conversion frame. It fails on current master and passes with the fix.
Validation on Windows/MSVC and Python 3.13: all embedded C++ tests, eight CMake integration checks, packaging tests, and installed C++14 consumers in header-only and precompiled modes passed. The full Python suite had 1,311 passes, 53 skips, and eight chrono failures also reproduced on unchanged master. Debug builds with warnings as errors passed; Release builds hit C4702 on both master and this branch.
Suggested changelog entry: