New tier 2 counters break some C extensions due to order of field mismatch

[mdboom]

Bug report

Bug description:

@tacaswell reported here:

#177144 appears to have broken building scipy

FAILED: scipy/special/_specfun.cpython-313-x86_64-linux-gnu.so.p/meson-generated__specfun.cpp.o
ccache c++ -Iscipy/special/_specfun.cpython-313-x86_64-linux-gnu.so.p -Iscipy/special -I../scipy/special -I../../../../home/tcaswell/.virtualenvs/py313/lib/python3.13/site-packages/numpy/_core/include -I/home/tcaswell/.pybuild/py313/include/python3.13 -fvisibility=hidden -fvisibility-inlines-hidden -fdiagnostics-color=always -DNDEBUG -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++17 -O3 -fpermissive -fPIC -DNPY_NO_DEPRECATED_API=NPY_1_9_API_VERSION -MD -MQ scipy/special/_specfun.cpython-313-x86_64-linux-gnu.so.p/meson-generated__specfun.cpp.o -MF scipy/special/_specfun.cpython-313-x86_64-linux-gnu.so.p/meson-generated__specfun.cpp.o.d -o scipy/special/_specfun.cpython-313-x86_64-linux-gnu.so.p/meson-generated__specfun.cpp.o -c scipy/special/_specfun.cpython-313-x86_64-linux-gnu.so.p/_specfun.cpp
In file included from /home/tcaswell/.pybuild/py313/include/python3.13/internal/pycore_code.h:461,
                 from /home/tcaswell/.pybuild/py313/include/python3.13/internal/pycore_frame.h:13,
                 from scipy/special/_specfun.cpython-313-x86_64-linux-gnu.so.p/_specfun.cpp:14948:
/home/tcaswell/.pybuild/py313/include/python3.13/internal/pycore_backoff.h: In function ‘_Py_BackoffCounter make_backoff_counter(uint16_t, uint16_t)’:
/home/tcaswell/.pybuild/py313/include/python3.13/internal/pycore_backoff.h:47:67: error: designator order for field ‘_Py_BackoffCounter::<unnamed union>::<unnamed struct>::backoff’ does not match declaration order in ‘_Py_BackoffCounter::<unnamed union>::<unnamed struct>’
   47 |     return (_Py_BackoffCounter){.value = value, .backoff = backoff};
      |   

conformed scipy builds with 63bbe77

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

• gh-117549: Match declaration order for _Py_BackoffCounter initializer #117551
• gh-117549: Don't use designated initializers in headers #118580

[gvanrossum]

Oh, I think I see what it is. I swapped the order in which the fields value and backoff are declared, and they have a compiler (flag?) that insists that the initializer lists the fields in the same order. @mdboom are you planning to make a PR to fix this? I can do it too (it's raining here :-).

[mdboom]

Yeah, I'm planning on making a PR. I think that's all it is ... though I think it's not a compiler flag so much as they are compiling C++ which is maybe strict about these things.

[scoder]

Apparently, designated initializers are a C++20 feature, i.e. they require a really recent C++ compiler with appropriate configuration. I see this in MSVC with Py3.13a6, so I don't think it's fixed:

C:\...\Python\3.13.0-alpha.6\x64\include\internal\pycore_backoff.h(47): error C7555: use of designated initializers requires at least '/std:c++20'
C:\...\Python\3.13.0-alpha.6\x64\include\internal\pycore_backoff.h(47): error C4576: a parenthesized type followed by an initializer list is a non-standard explicit type conversion syntax

[scoder]

The reason why this appears on user side is probably that Cython needs to include pycore_frame.h in Py3.11+ because some parts of frames that it needs for its coroutine implementation were moved from frameobject.h into internal header files at the time.

The discussion that lead to this change is here:
#90992

[gvanrossum]

So if I changed pycore_backoff.h to use something like this, the problem will be solved?

diff --git a/Include/internal/pycore_backoff.h b/Include/internal/pycore_backoff.h
index decf92bc419..50e88487a52 100644
--- a/Include/internal/pycore_backoff.h
+++ b/Include/internal/pycore_backoff.h
@@ -44,7 +44,10 @@ make_backoff_counter(uint16_t value, uint16_t backoff)
 {
     assert(backoff <= 15);
     assert(value <= 0xFFF);
-    return (_Py_BackoffCounter){.backoff = backoff, .value = value};
+    _Py_BackoffCounter counter;
+    counter.value = value;
+    counter.backoff = backoff;
+    return counter;
 }
 
 static inline _Py_BackoffCounter

I know there's another place this is used in that header, I'll fix that too. I'll whip up a PR.

[scoder]

So if I changed pycore_backoff.h to use something like this, the problem will be solved?

Probably, yes. Looks good to me. I'm not sure how to reproduce the issue locally, though, because GCC seems to be fairly relaxed about non-standard features whereas MSVC has the tendency to simply not support them, and I don't have MSVC on my side.

[gvanrossum]

Okay, I've asked you to review gh-118580 anyways, since we're so close to the beta1 release date. Hopefully it'll go in over the weekend and somebody else reading this can test with a nightly build or something. Otherwise we can of course revise this after beta1.

[gvanrossum]

The fix has been merged, let's keep this issue open for a bit to wait for confirmation that this fixes things.

[gvanrossum]

Closing now. If you find a problem with this in beta 1, leave a comment here and I'll reopen.