Skip to content

BLD: add missing include to fix build with freethreading #27956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 9, 2024

Conversation

tacaswell
Copy link
Contributor

This is required to build with the main and 3.13 branches of CPython when built with freethredaing enabled.

Without this I am seeing failures like

FAILED: numpy/_core/_multiarray_tests.cpython-313t-x86_64-linux-gnu.so.p/src_common_npy_hashtable.cpp.o
ccache c++ -Inumpy/_core/_multiarray_tests.cpython-313t-x86_64-linux-gnu.so.p -Inumpy/_core -I../numpy/_core -I../numpy/_core/src/multiarray -I../numpy/_core/src/npymath -Inumpy/_core/include -I../numpy/_core/include -I../numpy/_core/src/common -I/home/tcaswell/.pybuild/cp3131t/include/python3.13t -I/tmp/build-via-sdist-wsekxulh/numpy-2.3.0.dev0/.mesonpy-evozizae/meson_cpu -fvisibility=default -fdiagnostics-color=always -DNDEBUG -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c++17 -O3 -msse -msse2 -msse3 -mssse3 -msse4.1 -mpopcnt -msse4.2 -mavx -mf16c -mfma -mavx2 -maes -mpclmul -mbmi -mbmi2 -DNPY_HAVE_SSE2 -DNPY_HAVE_SSE -DNPY_HAVE_SSE3 -DNPY_HAVE_SSSE3 -DNPY_HAVE_SSE41 -DNPY_HAVE_POPCNT -DNPY_HAVE_SSE42 -DNPY_HAVE_AVX -DNPY_HAVE_F16C -DNPY_HAVE_FMA3 -DNPY_HAVE_AVX2 -fpermissive -fPIC -MD -MQ numpy/_core/_multiarray_tests.cpython-313t-x86_64-linux-gnu.so.p/src_common_npy_hashtable.cpp.o -MF numpy/_core/_multiarray_tests.cpython-313t-x86_64-linux-gnu.so.p/src_common_npy_hashtable.cpp.o.d -o numpy/_core/_multiarray_tests.cpython-313t-x86_64-linux-gnu.so.p/src_common_npy_hashtable.cpp.o -c ../numpy/_core/src/common/npy_hashtable.cpp
../numpy/_core/src/common/npy_hashtable.cpp: In function ‘PyArrayIdentityHash* PyArrayIdentityHash_New(int)’:
../numpy/_core/src/common/npy_hashtable.cpp:114:27: error: ‘nothrow’ is not a member of ‘std’
  114 |     res->mutex = new(std::nothrow) std::shared_mutex();
      |                           ^~~~~~~
../numpy/_core/src/common/npy_hashtable.cpp:20:1: note: ‘std::nothrow’ is defined in header ‘<new>’; this is probably fixable by adding ‘#include <new>’
   19 | #include "npy_hashtable.h"
  +++ |+#include <new>
   20 |

when building with freethreading with gcc (GCC) 14.2.1 20240910. I see this on main, 3.13, and with v3.13.1 so I'm pretty sure this is not a CPython change. I did not bisect back on numpy to see if this is a recent change in numpy or due to gcc getting upgraded locally.

I will admit to not fully understanding this change, but I did what the compiler said to do and it built....

This is required to build with the main and 3.13 branches of CPython when built
with freethredaing enabled.
@github-actions github-actions bot added the 36 - Build Build related PR label Dec 9, 2024
@seberg seberg added the 09 - Backport-Candidate PRs tagged should be backported label Dec 9, 2024
@seberg seberg merged commit 7650730 into numpy:main Dec 9, 2024
67 checks passed
@seberg
Copy link
Member

seberg commented Dec 9, 2024

Thanks @tacaswell, clearly right.

Hmmm, I wonder if now some linter will warn about unused include, forgot about that...

@tacaswell tacaswell deleted the bld/include_new branch December 9, 2024 20:55
charris pushed a commit to charris/numpy that referenced this pull request Dec 9, 2024
This is required to build with the main and 3.13 branches of CPython when built
with freethredaing enabled.
@rgommers rgommers added this to the 2.3.0 release milestone Dec 10, 2024
@tacaswell tacaswell restored the bld/include_new branch December 11, 2024 03:20
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
36 - Build Build related PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants