Skip to content

Commit 7c150ae

Browse files
committed
fix(pymacros): resolve undefined PyStructSequence_UnnamedField symbol on Python 3.9/3.10
`PyStructSequence_UnnamedField` is `extern const char * const` with hidden visibility on Python 3.9 and 3.10, so it is not an exported dynamic symbol for extension modules; it only became `PyAPI_DATA` (exported) in 3.11.0a2. Referencing it directly compiled but left an unresolved dynamic symbol, so `import optree._C` failed on 3.9/3.10 with `undefined symbol: PyStructSequence_UnnamedField`. It built and imported cleanly on 3.11+, so the failure was invisible in local 3.13/3.14 builds. Add a `PyStructSequenceUnnamedField()` helper in pymacros.h returning the symbol on 3.11.0a2+ and its stable value "unnamed field" below. Every use is by value (the serialization.cpp check compares a `std::string`), so the literal fallback is equivalent. Updated pytypes.h, optree.cpp, and serialization.cpp; the explicit `<structseq.h>` include is dropped since Python.h provides it. Also render unnamed struct-seq slots as an index-tagged `<unnamed@N>` placeholder (e.g. `<unnamed@7>`) instead of wrapping the bare marker, matching the format proposed in python/cpython#154387; the repr test is updated. Add "picklable" (in PyTreeSpec.__reduce__'s docstring) to the spelling wordlist.
1 parent 171448a commit 7c150ae

6 files changed

Lines changed: 36 additions & 19 deletions

File tree

‎docs/source/spelling_wordlist.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ optree
7171
OrderedDict
7272
param
7373
params
74+
picklable
7475
pragma
7576
preorder
7677
py

‎include/optree/pymacros.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,19 @@ inline constexpr Py_ALWAYS_INLINE bool Py_IsConstant(PyObject *x) noexcept {
7171
}
7272
#define Py_IsConstant(x) Py_IsConstant(x)
7373

74+
// `PyStructSequence_UnnamedField` is declared `extern` with hidden visibility (so it is not an
75+
// exported dynamic symbol for extension modules) before Python 3.11.0a2, where it became
76+
// `PyAPI_DATA`. Referencing it directly leaves an undefined symbol that makes the module fail to
77+
// import on those versions. Its value is the stable marker "unnamed field", and callers only ever
78+
// use it by value (never by pointer identity), so fall back to that literal there.
79+
inline const char *PyStructSequenceUnnamedField() noexcept {
80+
#if PY_VERSION_HEX >= 0x030B00A2 // Python 3.11.0a2
81+
return PyStructSequence_UnnamedField;
82+
#else
83+
return "unnamed field";
84+
#endif
85+
}
86+
7487
using interpid_t = decltype(PyInterpreterState_GetID(nullptr));
7588

7689
#if defined(PYBIND11_HAS_SUBINTERPRETER_SUPPORT) && \

‎include/optree/pytypes.h‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,10 @@ limitations under the License.
2828
#include <vector> // std::vector
2929

3030
#include <Python.h>
31+
3132
#if PY_VERSION_HEX < 0x030C00F0 // Python 3.12.0
3233
# include <structmember.h> // PyMemberDef
3334
#endif
34-
#include <structseq.h> // PyStructSequence_UnnamedField
3535

3636
#include <pybind11/eval.h> // pybind11::exec
3737
#include <pybind11/pybind11.h>
@@ -529,7 +529,7 @@ inline py::tuple StructSequenceGetFieldsImpl(const py::handle &type) {
529529
)py",
530530
py::dict(py::arg("cls") = type,
531531
py::arg("fields") = fields,
532-
py::arg("unnamed_field") = py::str(PyStructSequence_UnnamedField)));
532+
py::arg("unnamed_field") = py::str(PyStructSequenceUnnamedField())));
533533
return py::tuple{fields};
534534
#else
535535
const auto n_sequence_fields = thread_safe_cast<py::ssize_t>(
@@ -562,7 +562,7 @@ inline py::tuple StructSequenceGetFieldsImpl(const py::handle &type) {
562562
}
563563
for (py::ssize_t i = 0; i < n_sequence_fields; ++i) {
564564
if (!named[i]) {
565-
TupleSetItem(fields, i, py::str(PyStructSequence_UnnamedField));
565+
TupleSetItem(fields, i, py::str(PyStructSequenceUnnamedField()));
566566
}
567567
}
568568
return fields;

‎src/optree.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ void BuildModule(py::module_ &mod) { // NOLINT[runtime/references]
5151
mod.doc() = "Optimized PyTree Utilities. (C extension module built from " +
5252
std::string(__FILE_RELPATH_FROM_PROJECT_ROOT__) + ")";
5353
mod.attr("Py_TPFLAGS_BASETYPE") = py::int_(Py_TPFLAGS_BASETYPE);
54-
mod.attr("PyStructSequence_UnnamedField") = py::str(PyStructSequence_UnnamedField);
54+
mod.attr("PyStructSequence_UnnamedField") = py::str(PyStructSequenceUnnamedField());
5555

5656
// Meta information during build
5757
py::dict BUILDTIME_METADATA{};

‎src/treespec/serialization.cpp‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -223,24 +223,26 @@ std::string PyTreeSpec::ToStringImpl() const {
223223
const py::object qualname =
224224
EVALUATE_WITH_LOCK_HELD(py::getattr(type, "__qualname__"), type);
225225
sstream << PyStr(qualname) << "(";
226-
bool first = true;
226+
ssize_t index = 0;
227227
auto child_it = agenda.cend() - node.arity;
228228
for (const py::handle &field : fields) {
229-
if (!first) [[likely]] {
229+
if (index > 0) [[likely]] {
230230
sstream << ", ";
231231
}
232232
const std::string name = PyStr(field);
233-
// An unnamed slot has no valid identifier; render it with the synthetic
234-
// `<unnamed field>` placeholder (cf. CPython's `<lambda>`) rather than the bare
235-
// marker, which would read as an invalid keyword argument.
236-
if (name == PyStructSequence_UnnamedField) [[unlikely]] {
237-
sstream << "<" << name << ">";
233+
// An unnamed slot has no valid identifier, so render it as a synthetic
234+
// `<unnamed@INDEX>` placeholder (angle-bracketed like CPython's `<lambda>`)
235+
// rather than the bare marker, which would read as an invalid keyword argument.
236+
// The index disambiguates multiple unnamed slots (e.g. `os.stat_result` slots
237+
// 7-9).
238+
if (name == PyStructSequenceUnnamedField()) [[unlikely]] {
239+
sstream << "<unnamed@" << index << ">";
238240
} else [[likely]] {
239241
sstream << name;
240242
}
241243
sstream << "=" << *child_it;
242244
++child_it;
243-
first = false;
245+
++index;
244246
}
245247
sstream << ")";
246248
break;

‎tests/test_treespec.py‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -267,20 +267,21 @@ def test_treespec_string_representation(data):
267267
@skipif_pypy # CPython-only: `os.stat_result` slots 7, 8, 9 are unnamed; PyPy names them
268268
def test_treespec_structseq_unnamed_field_string_representation():
269269
# `os.stat_result` renders its UNNAMED sequence slots (7, 8, 9, the integer atime/mtime/ctime)
270-
# in the treespec repr with the synthetic `<unnamed field>` placeholder, following CPython's
271-
# `<lambda>`/`<genexpr>` convention for names that are not real identifiers (rather than the
272-
# bare `unnamed field` marker, which reads as an invalid keyword). CPython's `stat_result_desc`
273-
# has pinned the 7 named + 3 unnamed sequence fields (`n_in_sequence == 10`) for 16 years, so
274-
# the full sequence repr is stable and asserted exactly; the hidden float `st_atime`/etc. fields
275-
# (indices >= 10) are not part of the sequence and must not leak into the repr.
270+
# in the treespec repr with the synthetic `<unnamed@N>` placeholder (N is the sequence slot),
271+
# following CPython's `<lambda>`/`<genexpr>` convention for names that are not real identifiers
272+
# (rather than the bare `unnamed field` marker, which reads as an invalid keyword). CPython's
273+
# `stat_result_desc` has pinned the 7 named + 3 unnamed sequence fields (`n_in_sequence == 10`)
274+
# for 16 years, so the full sequence repr is stable and asserted exactly; the hidden float
275+
# `st_atime`/etc. fields (indices >= 10) are not part of the sequence and must not leak into the
276+
# repr.
276277
assert os.stat_result.n_sequence_fields == 10
277278
assert os.stat_result.n_unnamed_fields == 3
278279
st = os.stat_result(range(os.stat_result.n_fields))
279280
representation = str(optree.tree_structure(st))
280281
assert representation == (
281282
'PyTreeSpec(os.stat_result('
282283
'st_mode=*, st_ino=*, st_dev=*, st_nlink=*, st_uid=*, st_gid=*, st_size=*, '
283-
'<unnamed field>=*, <unnamed field>=*, <unnamed field>=*))'
284+
'<unnamed@7>=*, <unnamed@8>=*, <unnamed@9>=*))'
284285
)
285286

286287

0 commit comments

Comments
 (0)