fix: support nvcc and test

[henryiii]

Adding a CI test for NVCC, along with fixes and if statements. Everyone is welcome to suggest better fixes, especially for deactivated tests!

• Added CI test for NVCC (use this recipe to run locally in docker, as well)
• Moved logic (I'm calling it invalid_convert_integral) into compile time to fix warning (pointless comparison with 0) and provide better performance (will add comments and fix my one existing comment if that's the way we go)
• Worked around weird issue with vectorize_helper complaining about private usage internally
  * test_constants_and_functions.cpp skipped due to "incompatible exception specifications"
• Added body to void *operator new(size_t bytes) (fixes warning on Windows, too)
• NVCC joins Intel for NonCopyable not supported.

[bstaletic]

What does "incompatible exception specification" mean?

[henryiii]

That's printed by NVCC when running. It doesn't like either noexcept(false) or throw(), I believe, but the error message line is off (I think), so it's hard to tell what it is unhappy with.

[bstaletic]

This comment deserves a longer explanation. Which compilers behave oddly, is the first thing that popped into my mind when reading this code.

I know we know which ones today, but will we still remember in a year?

[henryiii]

MSVC and nvcc don't like having a definition with no body, even if it is never used. Having a body is better than manually trying to hide the warning, IMO, and when you start trying to hide the warning from multiple compilers, that explodes. (will add something like this to the note)

[henryiii]

I could probably move this out of the test definition, then the compilers would be okay with having a declaration without a definition. What they are complaining about is that these are local, so there's never a way to add the method body.

[henryiii]

I added the longer note; let me know if you think it's a good idea to move it up and out of the test body, so compilers will not complain about the missing definition - then we should be able to leave it empty.

[bstaletic]

What exactly breaks?

[henryiii]

It claims NVectorize is private in this context and quits.

[ 73%] Building CUDA object tests/CMakeFiles/pybind11_tests.dir/test_opaque_types.cpp.o
cd /build/tests && /usr/local/cuda/bin/nvcc  -Dpybind11_tests_EXPORTS -I/pybind11/include -isystem=/usr/include/python3.8  -O1 -DNDEBUG -Xcompiler=-fPIC -Xcompiler=-fvisibility=hidden   -Werror all-warnings -std=c++11 -x cu -c /pybind11/tests/test_opaque_types.cpp -o CMakeFiles/pybind11_tests.dir/test_opaque_types.cpp.o
/pybind11/tests/test_numpy_vectorize.cpp: In lambda function:
/pybind11/tests/test_numpy_vectorize.cpp:86:6: error: 'constexpr const size_t pybind11::detail::vectorize_helper<double (*)(int, float, double), double, int, float, double>::NVectorized' is private within this context
   86 |         std::array<py::buffer_info, 3> buffers {{ arg1.request(), arg2.request(), arg3.request() }};
      |      ^  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/pybind11/include/pybind11/numpy.h:1490:25: note: declared private here
 1490 |     static constexpr size_t NVectorized = constexpr_sum(vectorize_arg<Args>::vectorize...);

[henryiii]

By the way, you can run:

docker run --rm -it -v $PWD:/pybind11 nvidia/cuda:11.0-devel-ubuntu20.04 bash
apt-get update && DEBIAN_FRONTEND="noninteractive" apt-get install -y cmake python3-dev python3-pytest
cmake -S pybind11 -B build -DPYBIND11_CUDA_TESTS=ON -DPYBIND11_WERROR=ON
cmake --build build -v -j2
cmake --build build --target pytest

To do the testing locally.

[wjakob]

Not a big fan of this change, isn't there some way to #pragma be quiet NVCC?

[henryiii]

It's poorly documented, but yes, I think more recent NVCC versions support suppression if you can find the right code. But this will actually generate simpler/faster runtime code with 1-2 fewer checks, if this happens fairly often. We can move the sizeof check to compile time, too, I believe.

I also think the logic may be flawed; it might be good to rework with new logic. Also I'm surprised it's showing up with some very odd types; things like pointers were static_cast to an integer is not allowed (which is why it's using C-style casts); I guess this is used to covert pointer addresses to Python ints?

[henryiii]

Check the new version out! Simpler logic (the tests here are pretty comprehensive, don't ask me how I know that 😊), with no more pointless comparisons.

[wjakob]

Fine with all of these changes, except the range check that has become very complex with the extra SFINAE.

[YannickJadoul]

Looking at the if (py_err || (std::is_integral<T>::value && py_value != (py_type) (T) py_value)) {, this looks good to me :-)

[henryiii]

Okay, thanks! Going in, will rebase the PGI PR on it soon, since they collide a bit on a couple of lines.