Skip to content

fix(stl): rework array_caster element loading (compile time, bounds) - #6276

Open
rwgk wants to merge 7 commits into
pybind:masterfrom
rwgk:rwgk/array-caster-rework
Open

rwgk wants to merge 7 commits into
pybind:masterfrom
rwgk:rwgk/array-caster-rework

Conversation

@rwgk

@rwgk rwgk commented Nov 14, 2026 •

Copy link
Copy Markdown
Collaborator

— Claude Code (Fable 5.2) on behalf of Ralf —

Description

Fixes #5765. Alternative to #6258, which this PR grew out of (see "Compared with #6258").

array_caster (the caster behind std::array and std::valarray in pybind11/stl.h) now loads elements in one of two ways:

  • Constructing: collect the elements in a temporary std::vector, then aggregate-initialize the std::array through an index_sequence of Size elements, which copy- or move-constructs them. Since feat: change array_caster in pybind11/stl.h to support value types that are not default-constructible. #5305 all std::array types go through this code, and most still do.
  • Assigning: default-construct the array, then assign the elements. std::valarray has always been loaded this way. A std::array is now loaded this way only for element types for which that cannot make a difference: scalar types (int, double, pointers, enums), pybind11's numeric types (is_fmt_numeric, which include std::complex), and std::arrays of those. In addition, the caster has to yield the element type itself (not something that converts to it), and the elements have to be assignable (an array of const elements is not). Everything else is constructed, as on master.

What this fixes

1. Very slow compilation for large std::array of scalar and numeric element types (#5765). Assigning needs no index_sequence. Compiling the example from the issue (std::array<int, 102400> with def_readwrite, -O1, C++17):

Compiler master this PR
g++ 13.3 134 s 4.3 s
clang++ 18.1 stopped after 400 s 4.6 s

The numeric types include std::complex (with pybind11/complex.h), whose default constructor is not trivial although all it does is zero-initialize. A function taking a std::array<std::complex<double>, 102400> compiles in 3.5 s (g++) and 4.2 s (clang++) with this PR; with g++ on master it takes 130 s. Numeric types added to is_fmt_numeric later are covered without further changes, and so are nested arrays such as std::array<std::array<double, 3>, N>.

2. Out-of-bounds accesses when the length of the sequence changes during the conversion. On master the length is queried more than once: for the size check, and again by the end() of the iteration (the std::array path also queries it for reserve). If the answers differ, the loops can run past the end of the buffer. Both of these are reproducible on master with AddressSanitizer, using a sequence whose __len__ returns different values:

  • std::valarray: heap-buffer-overflow write if the length grows.
  • std::array: heap-buffer-overflow read if the length shrinks (elements are moved out of a std::vector that has fewer than Size elements).

A user-defined __len__ is the deterministic way to trigger this. In a free-threaded build, another thread resizing a list between two of the queries is the realistic way (not reproduced; the window is small).

All element loops now query the length once and index up to it. As before, each element caster is constructed before its item is fetched, and the item is kept alive until its value has been copied out.

3. No partially-loaded value. The std::valarray path used to resize and fill the caster's value directly. The result is now built in a local and stored only if all elements converted.

What changes in behavior

  • The length is queried once, instead of two or three times. If a sequence reports different lengths, the first answer decides: a three-item sequence reporting length 3 and then 2 loads into a std::valarray as [1, 2, 0] on master and as [1, 2, 3] with this PR. A __len__ with side effects can also tell the difference.
  • Nothing else is meant to change observably. In particular, elements are assigned only for element types that cannot tell that apart from being constructed.

The first revision of this PR (04e6c39) assigned whenever the element type was default constructible and assignable. That is not equivalent in general: with boost::multi_array elements, whose assignment does not resize, it loaded empty arrays where master loads them correctly. This comment has the reproducer and the options that were considered. The selection above is narrower than option 1 from there, in that trivial class types are constructed rather than assigned, and wider in that it includes the numeric types.

A requirement that follows from is_fmt_numeric

The specializations of is_fmt_numeric must be visible consistently in all translation units of a module that convert such arrays. In practice, pybind11/complex.h has to be included in all of them or in none, also where std::complex is opaque or has a custom caster. Otherwise array_caster is instantiated differently in different translation units, which violates the ODR, even though both instantiations give the same results.

The documentation of the STL conversions gets a note about this. The general part of that note (optional caster headers must be visible consistently wherever the casters or traits they provide affect a conversion) already applies on master, but was only documented for custom type casters.

Where the explosion remains

Element types that are constructed still pay for the index_sequence expansion, exactly as on master. That is every class type except the numeric types and std::arrays of scalar or numeric types: std::string, registered classes, and other structs and classes, including trivial ones. (The exceptions are constructed as well if their caster yields something other than the element type, or if they cannot be assigned.) With g++ 13.3 at -O1, a function taking a std::array of 102400 elements of a registered type without a default constructor takes 95 s to compile with this PR and 98 s on master.

Two ways to extend the fast compile to more element types, neither pursued here:

  • Assign class types as well, where every element operation involved is trivial. Two attempts at that during review each left a case in which master runs a user-defined constructor that assigning skips: a constructor template can take over from the copy or move constructor without affecting the triviality traits, and the traits test direct-initialization, whereas the elements of the array are copy-initialized (which ignores explicit constructors).
  • Construct the elements in place with placement-new in raw storage, then treat the storage as a std::array. That would cover every element type, but it depends on std::array layout details (including the special cases for Size == 0) and on implicit-lifetime rules, which seems too fragile for pybind11's compiler matrix.

Compared with #6258

#6258 has the right idea, which this PR keeps: fill the array in place where that is possible, instead of going through index_sequence. The differences:

  • When to fill in place. fix: avoid slow compile for large std::array in array_caster #6258 does it for element types that are default constructible and move assignable, which changes how the elements of class types are created. With boost::multi_array elements it loads empty arrays (g++ 13), or does not compile (clang++ 18, see the third item). Here it is limited to scalar and numeric element types, and arrays of those.
  • Two paths instead of three. fix: avoid slow compile for large std::array in array_caster #6258 adds a third convert_elements overload next to the std::valarray one, which does the same thing. Here they are one function; make_array() covers the only difference (resize vs. size check).
  • new ArrayType() instead of new ArrayType{}. The latter copy-list-initializes the elements, which is ill-formed for element types with an explicit default constructor (clang++ 18 rejects it; g++ 13 accepts it). boost::multi_array is such a type.
  • Bounds. The in-place overload in fix: avoid slow compile for large std::array in array_caster #6258 has the std::valarray problem described above: (*result)[ctr++] is bounded by a second length query, so a growing sequence overflows the heap-allocated array (confirmed with AddressSanitizer at 25e2172).
  • Assignability. fix: avoid slow compile for large std::array in array_caster #6258 tests is_move_assignable<Value>, but the assignment gets what cast_op<Value &&> yields, which is Value & for types registered with py::class_. A registered type that is copy constructible and move assignable, but not copy assignable, compiles on master and is a hard error with fix: avoid slow compile for large std::array in array_caster #6258. Here such a type is constructed.

The large-array test is adapted from #6258.

Testing

New and extended tests in tests/test_stl.py (bindings in tests/test_stl.cpp):

  • test_array_large: the [BUG]: slow compile if very long std::array being used with clang-18 #5765 case, and the same size with std::complex<double> elements. Building pybind11_tests.cpp + test_stl.cpp with these bindings (g++ 13.3, -O0) takes 671 s against master's headers and 20 s with this PR.
  • test_array_caster_len_changes, test_array_caster_len_exceeds_items: a sequence whose reported length shrinks, grows, or exceeds its items, for an assigned std::array, a std::valarray and a constructed std::array. The first asserts on the indices requested through __getitem__, so it fails on master without depending on undefined behavior being detected.
  • test_array_caster_items_created_on_the_fly: items that the sequence does not retain (meaningful under Valgrind/ASan).
  • test_array_element_types: five class types that must be constructed, four of them trivially copyable. Three were loaded differently from master by an earlier revision of the selection: one whose assignment does nothing (the boost::multi_array behavior in miniature), one whose default constructor throws, and one with a constructor template that takes over from the copy constructor. static_asserts pin which element types are assigned.
  • test_array_nested: constructed and assigned nested arrays, and an opaque array of const elements inside another array.
  • test_array_cast_only_element: an element caster that only implements cast() still works for returning a std::array.

Local validation, Linux x86-64, g++ 13.3, C++20, CPython 3.14.4:

  • Full build and test run with default and free-threaded CPython: Catch2 31 test cases pass in both; pytest 1481 passed, 14 skipped, 2 xfailed (default) and 1479 passed, 16 skipped, 2 xfailed (free-threaded).
  • test_stl.py under AddressSanitizer: 67 passed, 2 skipped, no reports.
  • Five reproducers from review give the same results as on master: boost::multi_array elements, a constructor template that takes over from the copy constructor, an explicit move constructor, an opaque array of const elements, and a caster whose constructor has side effects that __getitem__ observes.
  • Against master's headers (with the static_asserts on the new trait removed, which master does not have), the six test_array_caster_len_changes cases fail and the other tests pass.
  • tests/test_stl.cpp compiles without warnings (-Werror) with g++ 13.3 and clang++ 18.1 as C++11, 14, 17, 20 and 23. pre-commit passes on the changed files.

Not tested locally: MSVC, PyPy, GraalPy. CI was green for the first three commits.

AI assistance

This PR was written by Claude Code (Fable 5.2) at Ralf's direction. It started as a review of #6258, followed by the question of how array_caster would look if written from scratch. The claims in this description were checked by building and running: AddressSanitizer reproducers for the out-of-bounds cases (on master, on #6258 and with this PR), the compile-time measurements, and the test runs above.

The code and this description were reviewed independently with Codex (gpt-6.1-sol, reasoning effort "ultra") in several rounds, before and after posting, in addition to the review on this PR. What those reviews found, and what became of it:

  • The selection trait, then in class scope, broke element casters that only implement cast(). It is now evaluated inside a member function, with a test.
  • Assigning calls default constructors and assignment operators that master does not call. Following that up turned up the boost::multi_array case, which the review on this PR confirmed independently.
  • Triviality traits cannot select class types soundly (see "Where the explosion remains"). Hence the fixed families of element types.
  • Keying on is_fmt_numeric creates the ODR requirement above. The design is kept, because it is what makes future numeric types work without further changes, and the requirement is documented.
  • An opaque array of const elements, nested in another array, no longer compiled. The assignability condition and a test cover that.
  • The indexed loops fetched each item before constructing its caster, unlike master. The order is restored.

A last review of the final state, in a fresh session, ended with no findings for the code, the documentation note and this description.

How array_caster got here, and how that informed this PR
When Change Element loading
2016-02 std::array caster added (10c74c6, fixes #97) Value held in the caster. Input must be a list. Size check, then value[ctr++] = ... while iterating.
2016-12 std::valarray support, #545 Generalized into array_caster<ArrayType, Value, Resizable, Size>. An SFINAE pair require_size() either resizes or compares. Still default-construct and assign, shared by both containers.
2018-11 Arbitrary sequences accepted, #1602 Same loop, but the length is now whatever __len__ returns: once for require_size(), again for the iteration.
2024-08 Generators and other iterables accepted, #4686 Converted with tuple() first; convert_elements() split out of load().
2024-08 Element types that are not default constructible, #5305 (first released in v3.0.0) Storage becomes std::unique_ptr<ArrayType>. std::array elements go into a temporary std::vector, then vector_to_array() with an index_sequence of Size. Deliberately used for all std::array element types, to have one code path. std::valarray keeps resize-and-assign.
2025-07 Issue #5765 The index_sequence expansion makes std::array<int, 102400> take minutes to compile.
2026-11 #6258 Adds an in-place overload for default-constructible, move-assignable element types, next to the other two.
2026-11 This PR, first revision One assigning loop shared by std::valarray and std::array, for all default-constructible, assignable element types.

How this shaped the implementation:

Also not pursued: holding small arrays by value in the caster, as before #5305, which would need a second storage strategy; and sharing load() and cast() with list_caster, which has near-identical copies (worth doing, but separately).

Suggested changelog entry:

  • Fixed very slow compilation when binding large std::array types with pybind11/stl.h, for scalar and numeric element types, such as int, double or std::complex, and nested arrays of those.

  • Fixed potential out-of-bounds accesses in the std::array and std::valarray casters when the length of the Python sequence changes while it is being converted.

🤖 Generated with Claude Code


📚 Documentation preview 📚: https://pybind11--6276.org.readthedocs.build/

rwgk added 2 commits November 13, 2026 23:24
Load `std::array` and `std::valarray` elements in one of two ways:

* General path: default-construct the array, then assign the elements.
* Fallback (`std::array` only): temporary `std::vector` plus an
  `index_sequence` of `Size` elements, for element types that are not
  default constructible, or not assignable from what their caster yields.

Since pybind#5305 all `std::array` types went through the `index_sequence`,
which is very slow to compile for large `Size` (pybind#5765).

All element loops now determine the length of the sequence only once and
index up to it. Before, a sequence reporting different lengths could
cause out-of-bounds accesses (a write for `std::valarray`, a read for
`std::array`).

Assisted-by: ClaudeCode:claude-fable-5-2
Select the loading path inside a member function instead of in class
scope, so that an element caster that only implements `cast()` keeps
working for returning a `std::array`, as on master.

Assisted-by: ClaudeCode:claude-fable-5-2
@rwgk
rwgk marked this pull request as ready for review November 14, 2026 08:05
@rwgk

rwgk commented Nov 14, 2026

Copy link
Copy Markdown
Collaborator Author

— Claude Code (Fable 5.2) on behalf of Ralf —

The general path changes element semantics: a real case, and a proposed fix

Summary: On the general path, this PR default-constructs the elements of a std::array and then assigns them, where master copy- or move-constructs them. The description lists that under "What changes in behavior", with a contrived example (a default constructor that throws). There is a real type for which the two are not equivalent, with silently wrong results, so I think the selection trait should be narrowed. #6258 is affected in the same way. Opinions on the options below are welcome.

The real case

boost::multi_array does not resize on assignment, so assigning to a default-constructed (0×0) array copies nothing. Loading one 2×3 element into a std::array<boost::multi_array<double, 2>, 1> argument (Boost 1.83, -DNDEBUG), and returning the shape of the loaded element:

Headers g++ 13 clang++ 18
master (c68d56a) (2, 3) (2, 3)
#6258 (25e2172) (0, 0) compile error
this PR (04e6c39) (0, 0) (0, 0)
this PR + narrower trait (option 1) (2, 3) (2, 3)

Without NDEBUG, the (0, 0) cases abort on a Boost assertion instead. The clang error for #6258 is new ArrayType{} meeting the explicit default constructor of boost::multi_array.

Reproducer
#include <pybind11/pybind11.h>
#include <pybind11/stl.h>

#include <boost/multi_array.hpp>

#include <array>

namespace py = pybind11;
using MA = boost::multi_array<double, 2>;

PYBIND11_MODULE(ma_demo, m) {
    py::class_<MA>(m, "MA");
    m.def("make", []() { return MA(boost::extents[2][3]); });
    m.def("shape_of_first", [](const std::array<MA, 1> &a) {
        return py::make_tuple(a[0].shape()[0], a[0].shape()[1]);
    });
}
import ma_demo as m
print(m.shape_of_first([m.make()]))

For the description's own example, a copyable type whose default constructor throws in normal use, I did not find a real instance. The general point stands without it: any type whose assignment is not equivalent to copy construction, or whose default constructor has side effects, can tell the two approaches apart.

Options

1. Narrow the trait (proposed). Take the general path only where the switch cannot be observed: the element type is trivially default constructible, trivially copyable, and trivially assignable from what its caster yields.

-                            || (std::is_default_constructible<ArrayType>::value
-                                && std::is_assignable<Value &, value_cast_op_type>::value)>;
+                            || (std::is_trivially_default_constructible<Value>::value
+                                && std::is_trivially_copyable<Value>::value
+                                && std::is_trivially_assignable<Value &,
+                                                                value_cast_op_type>::value)>;

Everything else takes the fallback, where it behaves and compiles exactly as on master:

Element type PR as posted Narrower trait
int, double, bool, pointers general general
POD struct, std::array<double, 3> general general
std::complex<double> general fallback
class with a user-provided default constructor general fallback
std::string, std::vector<int>, std::shared_ptr<T> general fallback
boost::multi_array<double, 2> general fallback

The cost is that fewer element types get the fast compile. std::complex<double> is the one I would expect someone to miss: a large std::array of it would stay slow to compile, as on master. The #5765 case itself is unaffected (4.3 s with g++ 13.3, 4.6 s with clang++ 18.1, as before).

Checked so far with this change applied (plus making one test type trivial): the table above, and test_stl.py under AddressSanitizer (63 passed, 2 skipped). The full test suites and CI have not been run with it.

2. A looser trait. Require "default constructible" instead of "trivially default constructible", and keep the two trivial-copy conditions. That keeps std::complex and simple math structs with a user-provided default constructor on the general path, and still sends boost::multi_array to the fallback. What it cannot do is exclude a trivially copyable type whose default constructor throws or has side effects: with libstdc++, such a type has the same trait profile as std::complex<double> (trivially copyable, neither trivially nor nothrow default constructible). I know of no real type like that.

3. Construct each element in place in raw storage with placement-new. That keeps master's element operations for every type, and would also remove the compile-time explosion for the types that otherwise stay on the fallback. It is the approach the description calls too fragile under "Where the explosion remains" (it leans on implicit-lifetime rules and std::array layout details). I still think that belongs in a separate PR, if anywhere.

My preference is option 1: it makes this PR a pure optimization plus the bounds fix, with no change in element semantics.

Status

The PR head (04e6c39) is unchanged and still has this problem. With option 1, the description's "What changes in behavior" section shrinks to the changing-length case, and "Where the explosion remains" widens to all element types that are not trivial.

@henryiii henryiii left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI-generated review — OpenCode:GLM-5.3, at Henry Schreiner's direction. All claims below were verified by building and running locally (Apple clang arm64, free-threaded CPython 3.15, PYBIND11_TEST_OVERRIDE=test_stl), not only by reading the code.

Code review: the diff itself is solid

  • Bounds fix: length is queried once and l[i] goes through PySequence_GetItem (pytypes.h:1111), which is bounds-checked and throws — a shrinking sequence now raises instead of overflowing the std::valarray write / OOB-reading the fallback's std::vector. Confirmed the tests fail on master's behavior without relying on UB being caught: test_array_caster_len_changes asserts on the __getitem__ indices, and test_array_caster_len_exceeds_items expects the IndexError to propagate (on master it is silently swallowed by old-style iteration and reads out of bounds).
  • The selection trait is exact. movable_cast_op_type<Value&&> resolves to Value&& for macro casters and Value& for class_ casters (detail/type_caster_base.h:967), which is exactly what cast_op<Value &&>(std::move(conv)) yields, so is_assignable<Value &, value_cast_op_type> precisely predicts the assignment it gates. This is what saves the copy-constructible-but-not-copy-assignable registered types that #6258 broke (covered by NotCopyAssignable).
  • Cast-only element casters: keeping the trait inside the function body means cast_op_type is only required when load() is instantiated. test_array_cast_only_element passes locally.
  • ArrayType() vs ArrayType{} is the right call (copy-list-init would reject explicit default constructors; covered by test_array_element_types).
  • Item lifetime (const object item = l[i]) and no partial value (result stored only after all elements convert) are correct improvements, and tested.

Local run: full test_stl.py passes (55 passed, 10 skipped, skips unrelated to this PR); CI is green across the matrix.

The multi_array finding: confirmed independently

I built the reproducer from the comment above locally (Homebrew Boost, -DNDEBUG, Apple clang 17 / C++17):

Headers result of shape_of_first([m.make()])
this PR as posted (04e6c39) (0, 0) — silently wrong
this PR + option 1 trait (2, 3) — matches master

So the concern is real and reproducible, not just theoretical.

Recommendation: option 1

I applied option 1's trait locally and re-ran the full test_stl.py: everything still passes, including:

  • ExplicitDefaultCtor — its = default constructor is trivial, so it stays on the general path and keeps covering the () vs {} distinction;
  • the #5765 case — std::array<int, 102400> is trivial, so the compile-time fix is unaffected (the whole test_stl test module still builds in seconds);
  • nested POD arrays (std::array<std::array<double, 3>, N> style) remain trivially constructible/copyable/assignable and stay on the fast path.

Rationale: option 1 makes this PR a pure optimization plus the bounds fix, with no observable element-semantics change for any type. The cost (non-trivial element types like std::complex keep today's slow fallback compile) is the status quo, not a regression, and can be revisited (option 2) separately if a real std::complex-heavy case shows up.

Two small notes if option 1 is adopted:

  1. The proposed diff switches the default-constructibility test from ArrayType to Value. Both are correct; Value sends std::array<T, 0> to the fallback, which is cheap (empty index_sequence). Just worth making that a deliberate, commented choice.
  2. Consider adding a test that pins the semantics: an element type whose copy assignment is a no-op but whose copy constructor works (the boost::multi_array behavior in miniature — correct on the fallback, all-zeros on the general path). It would fail if the trait is ever loosened again without revisiting this discussion.

The PR description's "What changes in behavior" section will need the matching update.

Default-constructing the elements and then assigning them is not
equivalent to copy constructing them for every element type. With
`boost::multi_array` elements, whose assignment does not resize, the
previous revision loaded empty arrays where master loads them correctly.

Assign only if that cannot be told apart from constructing: the element
type is trivially default constructible, trivially copyable, and
trivially assignable from what its caster yields. All other element
types are constructed, exactly as on master.

Assisted-by: ClaudeCode:claude-fable-5-2
@rwgk

rwgk commented Nov 14, 2026

Copy link
Copy Markdown
Collaborator Author

Please don't merge: I'm still validating a change that'll resolve this issue (see comment above):

The cost is that fewer element types get the fast compile. std::complex is the one I would expect someone to miss: a large std::array of it would stay slow to compile, as on master.

Shorten the array_caster comments, get each item as an owned object
directly (one less incref/decref pair per element), and bind
array_element_vals for NoDefaultCtor so the tests share one parameter
list.

Assisted-by: ClaudeCode:claude-fable-5-2
@henryiii

Copy link
Copy Markdown
Collaborator

🤖 AI text below 🤖

I pushed 5d687c7, a cleanup pass (reuse, simplification, efficiency). It does not change behavior. test_stl.py passes locally and pre-commit is clean.

Changed

  • stl.h comments: shortened the rationale block above convert_elements, removed the resize comment in make_array, and stated the keep-alive reason one time instead of on both loops.
  • stl.h item access: both loops now call accessor_policies::sequence_item::get(l, i) instead of l[i]. This removes one incref/decref pair per element and keeps the same ownership and errors.
  • Test helper: the Python helper load_array_no_default_ctor is replaced by a C++ binding of the existing array_element_vals<NoDefaultCtor>.
  • Test parameters: NoDefaultCtor is now a case of test_array_element_types, so the duplicate wrong-length check is gone from test_array_no_default_ctor. test_array_caster_items_created_on_the_fly now uses ARRAY_CASTER_LOADS instead of its own list.

One coverage change: the array_assign entry of ARRAY_CASTER_LOADS is now std::array<ExplicitDefaultCtor, 2> instead of std::array<int, 3>. It takes the same overload, but the length-change tests no longer run on an int array.

Considered and not changed

  • Merge the two convert_elements loops into a helper with a lambda sink: it saves about 4 lines and adds an indirection.
  • Write directly into value instead of the local result: a failed load would leave a partly filled array.
  • new ArrayType without (): it saves a memset but leaves indeterminate storage.
  • Smaller test edits: getattr-based parametrization, removal of the <array> and <valarray> includes, and removal of the third sum_large_array call are matters of taste or cost almost nothing.

Scope limit

  • The [BUG]: slow compile if very long std::array being used with clang-18 #5765 compile-time fix covers only trivial element types. In a local measurement (clang, -std=c++17 -O0 -g), std::array<std::string, 102400> took more than 180 s to compile, against 0.8 s for int. I found no C++11-safe alternative. The PR description or changelog entry should state this limit.
  • tuple_caster in cast.h has a similar item-lifetime pattern (seq[Is] temporaries are released before the subcasters are used). It is outside this PR.

rwgk added 3 commits November 14, 2026 12:23
Assign the elements of a `std::array`, instead of constructing them, for
the element types for which that cannot make a difference: scalar types,
the numeric types (`is_fmt_numeric`), and `std::array`s of those. The
caster has to yield the element type itself, and the assignment has to
be possible. All other class types are constructed, as on master.

The numeric types are what makes this more than "trivial types": they
include `std::complex` (with pybind11/complex.h), whose default
constructor is not trivial, although all it does is zero-initialize. A
large `std::array` of `std::complex` now compiles as quickly as one of
`double`, and numeric types added to `is_fmt_numeric` later are covered
without further changes.

Trivial class types are no longer assigned. Identifying the ones for
which assigning is equivalent to constructing is not feasible with type
traits: review found a constructor template that takes over from the
copy constructor, and an explicit move constructor, which
direct-initialization uses but the copy-initialization of the array
elements does not. In both cases master runs a user-defined constructor
that assigning would skip.

Assisted-by: ClaudeCode:claude-fable-5-2
Master iterates over the sequence with a lazy accessor, so that an item
is fetched when its caster loads it, after the caster was constructed.
The indexed loops fetched the item first. A caster whose constructor has
side effects that `__getitem__` can observe saw the difference, also for
a sequence of constant length.

Go back to an accessor, which fetches the item when it is first used,
and which keeps it alive while the caster is in use. Pass `item.ptr()`
to the caster instead of the accessor itself: that keeps the saving of
the cleanup commit, which avoided the temporary `object` that master
creates for each item.

Assisted-by: ClaudeCode:claude-fable-5-2
Add a note to the documentation of the STL container conversions.

The general rule in the note already applies on master, but was only
documented for custom type casters: the optional caster headers (stl.h,
complex.h, ...) must be visible consistently wherever the casters or
traits they provide affect a conversion. If one translation unit of an
extension module converts a type with such a header taking effect, every
translation unit that converts that type has to include it, or the ODR
is violated.

The last sentence of the note is new with this PR. complex.h now also
matters for a `std::array` of `std::complex` if `std::complex` is opaque
or has a custom caster, because `array_caster` selects how it loads the
elements with `is_fmt_numeric`, a trait that complex.h specializes.

Assisted-by: ClaudeCode:claude-fable-5-2
@rwgk

rwgk commented Nov 14, 2026

Copy link
Copy Markdown
Collaborator Author

— Claude Code (Fable 5.2) on behalf of Ralf —

What changed since bf5c31a

Three commits are pushed on top of the cleanup commit 5d687c7, and the PR description is rewritten to match. The elements of a std::array are now assigned (the way that compiles quickly) only for scalar types, pybind11's numeric types (is_fmt_numeric, which brings in std::complex), and std::arrays of those; every other class type is constructed, exactly as on master. Compared with "option 1" from the discussion above, that is wider in one respect (a large array of std::complex, or of small numeric arrays, now compiles in seconds) and narrower in another (trivial class types such as plain structs are constructed again). The narrowing has a reason: two more review rounds each produced a class type that passed every triviality condition and was still loaded differently from master, so type traits were given up as a way to select class types. Review also found a compile regression for nested arrays of const elements and a change in the order of caster construction and item access, both fixed, and an ODR requirement that follows from keying on is_fmt_numeric, which is kept and now documented. The final state went through an independent review that ended without findings.

The three new commits
  • 051ee96, fix(stl): assign std::array elements for scalar and numeric types. Replaces the trivial-types selection of bf5c31a.
  • 10ddb9a, fix(stl): construct each element caster before its item is fetched.
  • 7dbed0d, docs: caster headers must be visible consistently.

The selection now reads:

template <typename T>
struct is_plain_array_element : any_of<std::is_scalar<T>, is_fmt_numeric<T>> {};
template <typename T, size_t N>
struct is_plain_array_element<std::array<T, N>> : is_plain_array_element<T> {};

// in array_caster::convert_elements():
using assign_elements = bool_constant<
    Resizable
    || (is_plain_array_element<Value>::value
        && std::is_same<remove_cv_t<remove_reference_t<value_cast_op_type>>,
                        remove_cv_t<Value>>::value
        && std::is_assignable<Value &, value_cast_op_type>::value)>;

The second condition requires the caster to yield the element type itself, rather than something that converts to it. The third is explained under "Two smaller fixes".

The cleanup commit 5d687c7 is kept. Two things in it changed again:

  • The item access in the two loops (see "Two smaller fixes"). Its saving of one incref/decref pair per element is preserved.
  • ExplicitDefaultCtor, which its ARRAY_CASTER_LOADS used as the assigned case, is a class type and therefore constructed now. A registered enum takes its place.
Which element types are assigned now
Element type bf5c31a (option 1) now
int, double, enums, pointers assigned assigned
std::array<double, 3> assigned assigned
struct { double x, y, z; } assigned constructed
std::chrono::seconds assigned constructed
trivial class that is not standard-layout assigned constructed
std::complex<double> constructed assigned
std::array<std::complex<double>, 2> constructed assigned
std::pair<int, int>, std::string, class with a user-provided default constructor constructed constructed

The table was produced by evaluating both selections for these types. "POD" is not the criterion in either direction: scalars and arrays of scalars are PODs and are assigned, POD structs are not, and std::complex is not a POD and is. The rule is: every class type is constructed, as on master, except the numeric types and std::arrays of scalar or numeric types.

Why std::complex, and why through is_fmt_numeric

The earlier comment named the cost of option 1: a large std::array of std::complex<double> would stay slow to compile. Measured with g++ 13.3 at -O1 for 102400 elements, that was 130 s on master and at bf5c31a. It is now 3.5 s (4.2 s with clang++ 18.1).

std::complex is not trivially default constructible, but all its default constructor does is zero-initialize. Rather than special-casing it, the selection uses is_fmt_numeric, the existing trait for the numeric types pybind11 knows. It is true for the arithmetic types, and pybind11/complex.h specializes it for std::complex. Numeric types added to it later, such as short float types, are then covered without touching stl.h. The assumption that a numeric type is a plain value is recorded at the trait.

stl.h does not include <complex> for this. complex.h carries an #undef I workaround for a glibc macro, and stl.h uses I as a template parameter name.

Why class types are no longer selected by triviality traits

Both types below are registered with py::class_ and loaded into a std::array<T, 1> from [T(7)]. Master returns 107 for both, because it runs the constructor template. A selection by triviality returns 7, because assigning skips it.

A constructor template that takes over from the copy constructor for a non-const lvalue, which is what the caster for a registered type yields. This type is trivially default constructible, trivially copyable and trivially assignable, so it passed the conditions of bf5c31a:

struct T {
    T() = default;
    explicit T(int v) : value(v) {}
    template <class U>
    T(U &&x) : value(x.value + 100) {}
    int value;
};

Testing each element operation separately (is_trivially_constructible<T, T &> and so on) catches that one, but not this one, which has an explicit move constructor. The traits test direct-initialization, which uses it. The elements of the array are copy-initialized, which ignores it and selects the template:

struct T {
    T() = default;
    explicit T(int n) : val(n) {}
    T(const T &) = default;
    explicit T(T &&) = default;
    T &operator=(const T &) = default;
    template <class U>
    T(const U &&x) : val(x.val + 100) {}
    int val;
};

Both are contrived. But after boost::multi_array and these two, the pattern was clear enough: each set of conditions held until someone looked for the next type. Scalars have no constructors or assignment operators to disagree about, the numeric types are assumed to be plain values, and std::array only passes its elements' operations through. So the selection is by those families.

The ODR requirement, and the documentation note

Keying on is_fmt_numeric means that its specializations must be visible consistently in all translation units of a module that convert such arrays. In practice, pybind11/complex.h has to be included in all of them or in none. That was already required wherever the caster from that header is used. What is new is that it also holds if std::complex is opaque or has a custom caster: those casters are the same in every translation unit, but array_caster would be instantiated differently depending on the header. Both instantiations give the same results, but it is an ODR violation.

The alternatives were to include <complex> in stl.h (see above), or to tie the numeric condition to the caster type, which adds a condition and leaves the same case open for custom casters. The design is kept and the requirement is stated.

7dbed0d adds a note to the documentation of the STL conversions. Its general part (optional caster headers must be visible consistently wherever the casters or traits they provide affect a conversion) already applies on master, but was only documented for custom type casters. Its last sentence, about pybind11/complex.h and arrays of std::complex, is new with this PR.

Two smaller fixes from review

Arrays of const elements. With

using Inner = std::array<const int, 2>;
PYBIND11_MAKE_OPAQUE(Inner)

a std::array<Inner, 1> compiles on master, where the elements are copy constructed. Inner is an array of scalars, so the family rule alone selects assigning, which does not compile. Hence the std::is_assignable condition, with a test.

Order of caster construction and item access. Master iterates with a lazy accessor: the element caster is constructed first, and the item is fetched when the caster loads it. The indexed loops, in my version and in the cleanup's, fetched the item first. Only a caster whose constructor has side effects that __getitem__ can observe sees the difference, but it does so for a perfectly ordinary sequence. The loops use an accessor again, declared before the caster so that it also outlives it, as on master:

const auto item = l[i];
value_conv conv;
if (!conv.load(item.ptr(), convert)) {

Passing item.ptr() instead of item avoids the temporary object that master creates for each item. That keeps the saving the cleanup commit was after.

Differences from master

For ordinary inputs there are none that I know of. What does differ:

  • The length of the sequence is queried once instead of two or three times. If the answers would differ, the first one decides (master ran out of bounds). A __len__ with side effects can tell.
  • The ODR requirement above. It is formal: there is no behavioral effect.
  • Scalar and numeric elements are zero-initialized and then assigned, which is not observable.
  • Numeric types added to is_fmt_numeric in the future are assumed to be plain values.

Against master's headers, the only tests of this PR that fail are the six changing-length cases; the others pass there too, apart from the static_asserts on the new trait. Nothing that compiles on master is known to fail now, and class types compile exactly as slowly as before.

Tests

Changed since bf5c31a:

  • test_array_element_types now has five class types that must be constructed: no default constructor, no copy assignment, an assignment that does nothing, a throwing default constructor, and the constructor template from above. ExplicitDefaultCtor is gone, since class types are not assigned any more.
  • static_asserts pin is_plain_array_element.
  • test_array_large also loads a 102400-element array of std::complex<double>.
  • test_array_nested also covers a nested numeric array and the opaque array of const elements.
  • The assigned case in ARRAY_CASTER_LOADS is an array of a registered enum.

Deliberately not added, although review asked for them at first: a test for the order of caster construction and item access, and one for the "caster yields the element type itself" condition. Each needs a purpose-built caster to pin a property that pybind11 has never specified. The code comments state both. The reviewer accepted that.

Reviews and validation

Reviews with Codex (gpt-6.1-sol, reasoning effort "ultra"), all before pushing:

  • Three rounds on the std::complex change. They found the two class types above, the ODR requirement, and the nested const case.
  • Three rounds in a fresh session on the result. They found the order of caster construction and item access, an inaccurate claim about reference counts in a commit message, and wording in the documentation note. The last round ended with "merge as is" for the code and "post as is" for the description.

Local validation at 7dbed0d, Linux x86-64, g++ 13.3, C++20, CPython 3.14.4:

  • Full build and test run with default and free-threaded CPython: Catch2 31 test cases pass in both; pytest 1481 passed, 14 skipped, 2 xfailed (default) and 1479 passed, 16 skipped, 2 xfailed (free-threaded).
  • test_stl.py under AddressSanitizer: 67 passed, 2 skipped, no reports.
  • Five reproducers from review give the same results as on master: boost::multi_array elements, the two class types above, the opaque array of const elements, and a caster with constructor side effects.
  • tests/test_stl.cpp compiles without warnings (-Werror) with g++ 13.3 and clang++ 18.1 as C++11, 14, 17, 20 and 23.

Not tested locally: MSVC, PyPy, GraalPy. The documentation was not built locally.

Would C++17 as the minimum have helped?

Somewhat. std::is_aggregate would probably have kept plain structs on the fast path: both class types above are non-aggregates, and an aggregate's own copy and move constructors are implicit, so direct- and copy-initialization select the same one. That is not prototyped. if constexpr would have replaced the tag-dispatched overloads with one function.

It would not have changed the rest. There are still only two ways to populate a std::array, and the questions about what "equivalent to constructing" means were independent of the language level.

@rwgk

rwgk commented Nov 14, 2026

Copy link
Copy Markdown
Collaborator Author

I'm done now and all tests pass. I think this is ready for merging.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: slow compile if very long std::array being used with clang-18 Support for std array

2 participants