From 06e74d9d46898b4e77f5e2f21f9091b16cea280f Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 7 Oct 2022 14:41:54 -0700 Subject: [PATCH 1/6] Reproducer for issue encountered in smart_holder update. --- tests/test_class.cpp | 31 +++++++++++++++++++++++++++++++ tests/test_class.py | 4 ++++ 2 files changed, 35 insertions(+) diff --git a/tests/test_class.cpp b/tests/test_class.cpp index c8b8071dc9..e7417c08e4 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -36,6 +36,35 @@ struct NoBraceInitialization { std::vector vec; }; +namespace test_class { +namespace pr4220 { // PR #4220 + +template // Using int as a trick to easily generate a series of types. +struct atyp { // Short for "any type". + std::string mtxt; +}; + +template +std::string get_mtxt(const T &obj) { + return obj.mtxt; +} + +using atyp_valu = atyp<0x0>; + +atyp_valu rtrn_valu() { + atyp_valu obj{"Value"}; + return obj; +} + +void bind_all(py::module_ m) { + py::class_(m, "atyp_valu") + .def(py::init(&rtrn_valu)) + .def("get_mtxt", get_mtxt); +} + +} // namespace pr4220 +} // namespace test_class + TEST_SUBMODULE(class_, m) { // test_instance struct NoConstructor { @@ -517,6 +546,8 @@ TEST_SUBMODULE(class_, m) { py::class_(gt, "OtherDuplicateNested"); py::class_(gt, "YetAnotherDuplicateNested"); }); + + test_class::pr4220::bind_all(m); } template diff --git a/tests/test_class.py b/tests/test_class.py index ff9196f0f2..8fdaf98dc2 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -469,3 +469,7 @@ class ClassScope: m.register_duplicate_nested_class_type(ClassScope) expected = 'generic_type: type "YetAnotherDuplicateNested" is already registered!' assert str(exc_info.value) == expected + + +def test_pr4220_bind_all(): + assert m.atyp_valu().get_mtxt() == "Value" From 62311eb431849d135a5db84f6c75ec390f2ede7c Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 7 Oct 2022 14:59:13 -0700 Subject: [PATCH 2/6] clang-tidy compatibility (untested). --- tests/test_class.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_class.cpp b/tests/test_class.cpp index e7417c08e4..31b5bef5ee 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -56,7 +56,7 @@ atyp_valu rtrn_valu() { return obj; } -void bind_all(py::module_ m) { +void bind_all(py::module_ &m) { py::class_(m, "atyp_valu") .def(py::init(&rtrn_valu)) .def("get_mtxt", get_mtxt); From e9070b27e5ff81c365f5673db8bb323b52b44483 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Fri, 7 Oct 2022 23:11:18 -0700 Subject: [PATCH 3/6] Add `enable_if_t` to workaround. --- include/pybind11/operators.h | 7 +++++++ include/pybind11/pybind11.h | 9 +++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/include/pybind11/operators.h b/include/pybind11/operators.h index a0c3b78d65..0add905700 100644 --- a/include/pybind11/operators.h +++ b/include/pybind11/operators.h @@ -84,6 +84,13 @@ struct op_impl {}; /// Operator implementation generator template struct op_ { +#if defined(__CUDACC__) && (__CUDACC_VER_MAJOR__ == 11) && (__CUDACC_VER_MINOR__ >= 4) \ + && (__CUDACC_VER_MINOR__ <= 8) +// Nvidia's NVCC is broken between 11.4.0 and 11.8.0 +// https://github.com/pybind/pybind11/issues/4193 +# define PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8 + static constexpr bool op_enable_if_hook = true; +#endif template void execute(Class &cl, const Extra &...extra) const { using Base = typename Class::type; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 3f6e27b751..2400d8f558 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1578,17 +1578,14 @@ class class_ : public detail::generic_type { return *this; } -// Nvidia's NVCC is broken between 11.4.0 and 11.8.0 -// https://github.com/pybind/pybind11/issues/4193 -#if defined(__CUDACC__) && (__CUDACC_VER_MAJOR__ == 11) && (__CUDACC_VER_MINOR__ >= 4) \ - && (__CUDACC_VER_MINOR__ <= 8) - template +#if defined(PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8) + template = 0> class_ &def(const T &op, const Extra &...extra) { op.execute(*this, extra...); return *this; } - template + template = 0> class_ &def_cast(const T &op, const Extra &...extra) { op.execute_cast(*this, extra...); return *this; From 7557684964e52c05b9d1fc633d4c383462116f11 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 8 Oct 2022 01:17:10 -0700 Subject: [PATCH 4/6] Bug fix: Move `PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8` determination to detail/common.h So that it actually is defined in pybind11.h --- include/pybind11/detail/common.h | 7 +++++++ include/pybind11/operators.h | 6 +----- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 6da7ef859c..1614a3abf8 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -1178,5 +1178,12 @@ constexpr inline bool silence_msvc_c4127(bool cond) { return cond; } # define PYBIND11_DETAILED_ERROR_MESSAGES #endif +#if defined(__CUDACC__) && (__CUDACC_VER_MAJOR__ == 11) && (__CUDACC_VER_MINOR__ >= 4) \ + && (__CUDACC_VER_MINOR__ <= 8) && !defined(PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8) +// Nvidia's NVCC is broken between 11.4.0 and 11.8.0 +// https://github.com/pybind/pybind11/issues/4193 +# define PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8 +#endif + PYBIND11_NAMESPACE_END(detail) PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/operators.h b/include/pybind11/operators.h index 0add905700..a1dad39fde 100644 --- a/include/pybind11/operators.h +++ b/include/pybind11/operators.h @@ -84,11 +84,7 @@ struct op_impl {}; /// Operator implementation generator template struct op_ { -#if defined(__CUDACC__) && (__CUDACC_VER_MAJOR__ == 11) && (__CUDACC_VER_MINOR__ >= 4) \ - && (__CUDACC_VER_MINOR__ <= 8) -// Nvidia's NVCC is broken between 11.4.0 and 11.8.0 -// https://github.com/pybind/pybind11/issues/4193 -# define PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8 +#if defined(PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8) static constexpr bool op_enable_if_hook = true; #endif template From a049f1be0252b753a7a6ee9a7b8378115441386d Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 8 Oct 2022 09:54:00 -0700 Subject: [PATCH 5/6] Try using the workaround (which is nicer than the original code) universally. --- include/pybind11/detail/common.h | 7 ------- include/pybind11/operators.h | 2 -- include/pybind11/pybind11.h | 14 -------------- 3 files changed, 23 deletions(-) diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 1614a3abf8..6da7ef859c 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -1178,12 +1178,5 @@ constexpr inline bool silence_msvc_c4127(bool cond) { return cond; } # define PYBIND11_DETAILED_ERROR_MESSAGES #endif -#if defined(__CUDACC__) && (__CUDACC_VER_MAJOR__ == 11) && (__CUDACC_VER_MINOR__ >= 4) \ - && (__CUDACC_VER_MINOR__ <= 8) && !defined(PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8) -// Nvidia's NVCC is broken between 11.4.0 and 11.8.0 -// https://github.com/pybind/pybind11/issues/4193 -# define PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8 -#endif - PYBIND11_NAMESPACE_END(detail) PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/operators.h b/include/pybind11/operators.h index a1dad39fde..16a88ae171 100644 --- a/include/pybind11/operators.h +++ b/include/pybind11/operators.h @@ -84,9 +84,7 @@ struct op_impl {}; /// Operator implementation generator template struct op_ { -#if defined(PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8) static constexpr bool op_enable_if_hook = true; -#endif template void execute(Class &cl, const Extra &...extra) const { using Base = typename Class::type; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 2400d8f558..fb4b7578d9 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1578,7 +1578,6 @@ class class_ : public detail::generic_type { return *this; } -#if defined(PYBIND11_USING_WORKAROUND_FOR_CUDA_11_4_THROUGH_8) template = 0> class_ &def(const T &op, const Extra &...extra) { op.execute(*this, extra...); @@ -1590,19 +1589,6 @@ class class_ : public detail::generic_type { op.execute_cast(*this, extra...); return *this; } -#else - template - class_ &def(const detail::op_ &op, const Extra &...extra) { - op.execute(*this, extra...); - return *this; - } - - template - class_ &def_cast(const detail::op_ &op, const Extra &...extra) { - op.execute_cast(*this, extra...); - return *this; - } -#endif template class_ &def(const detail::initimpl::constructor &init, const Extra &...extra) { From 9680dd4a14042176be07e27672d1013fdf7c52b2 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Sat, 8 Oct 2022 11:04:18 -0700 Subject: [PATCH 6/6] Reduce reproducer for CUDA 11.7 issue encountered in smart_holder update. This commit tested in isolation on top of current master + first version of reproducer (62311eb431849d135a5db84f6c75ec390f2ede7c). Succeeds with Debian Clang 14.0.6 C++17 (and probably all other compilers). Fails for CUDA 11.7: ``` cd /build/tests && /usr/local/cuda/bin/nvcc -forward-unknown-to-host-compiler -Dpybind11_tests_EXPORTS -I/mounted_pybind11/include -isystem=/usr/include/python3.10 -g --generate-code=arch=compute_52,code=[compute_52,sm_52] -Xcompiler=-fPIC -Xcompiler=-fvisibility=hidden -Werror all-warnings -std=c++17 -MD -MT tests/CMakeFiles/pybind11_tests.dir/test_class.cpp.o -MF CMakeFiles/pybind11_tests.dir/test_class.cpp.o.d -x cu -c /mounted_pybind11/tests/test_class.cpp -o CMakeFiles/pybind11_tests.dir/test_class.cpp.o /mounted_pybind11/tests/test_class.cpp(53): error: more than one instance of overloaded function "pybind11::class_::def [with type_=test_class::pr4220_tripped_over_this::Empty0, options=<>]" matches the argument list: function template "pybind11::class_ &pybind11::class_::def(const char *, Func &&, const Extra &...) [with type_=test_class::pr4220_tripped_over_this::Empty0, options=<>]" /mounted_pybind11/include/pybind11/pybind11.h(1557): here function template "pybind11::class_ &pybind11::class_::def(const T &, const Extra &...) [with type_=test_class::pr4220_tripped_over_this::Empty0, options=<>]" /mounted_pybind11/include/pybind11/pybind11.h(1586): here argument types are: (const char [8], ) object type is: pybind11::class_ 1 error detected in the compilation of "/mounted_pybind11/tests/test_class.cpp". ``` --- tests/test_class.cpp | 29 ++++++++++------------------- tests/test_class.py | 7 +++++-- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/tests/test_class.cpp b/tests/test_class.cpp index 31b5bef5ee..18c8d358bb 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -37,32 +37,23 @@ struct NoBraceInitialization { }; namespace test_class { -namespace pr4220 { // PR #4220 +namespace pr4220_tripped_over_this { // PR #4227 -template // Using int as a trick to easily generate a series of types. -struct atyp { // Short for "any type". - std::string mtxt; -}; +template +struct SoEmpty {}; template -std::string get_mtxt(const T &obj) { - return obj.mtxt; +std::string get_msg(const T &) { + return "This is really only meant to exercise successful compilation."; } -using atyp_valu = atyp<0x0>; - -atyp_valu rtrn_valu() { - atyp_valu obj{"Value"}; - return obj; -} +using Empty0 = SoEmpty<0x0>; -void bind_all(py::module_ &m) { - py::class_(m, "atyp_valu") - .def(py::init(&rtrn_valu)) - .def("get_mtxt", get_mtxt); +void bind_empty0(py::module_ &m) { + py::class_(m, "Empty0").def(py::init<>()).def("get_msg", get_msg); } -} // namespace pr4220 +} // namespace pr4220_tripped_over_this } // namespace test_class TEST_SUBMODULE(class_, m) { @@ -547,7 +538,7 @@ TEST_SUBMODULE(class_, m) { py::class_(gt, "YetAnotherDuplicateNested"); }); - test_class::pr4220::bind_all(m); + test_class::pr4220_tripped_over_this::bind_empty0(m); } template diff --git a/tests/test_class.py b/tests/test_class.py index 8fdaf98dc2..47ba450fc2 100644 --- a/tests/test_class.py +++ b/tests/test_class.py @@ -471,5 +471,8 @@ class ClassScope: assert str(exc_info.value) == expected -def test_pr4220_bind_all(): - assert m.atyp_valu().get_mtxt() == "Value" +def test_pr4220_tripped_over_this(): + assert ( + m.Empty0().get_msg() + == "This is really only meant to exercise successful compilation." + )