diff --git a/.clang-tidy b/.clang-tidy index dbe85a8b4..d853a703c 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -46,6 +46,7 @@ readability-simplify-subscript-expr, readability-static-accessed-through-instance, readability-static-definition-in-anonymous-namespace, readability-string-compare, +readability-suspicious-call-argument, readability-uniqueptr-delete-release, -bugprone-exception-escape, -bugprone-reserved-identifier, diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 39c32b2ac..c08bb173f 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -159,8 +159,9 @@ tests with these targets: * `test_cmake_build`: Install / subdirectory tests If you want to build just a subset of tests, use -`-DPYBIND11_TEST_OVERRIDE="test_callbacks.cpp;test_pickling.cpp"`. If this is -empty, all tests will be built. +`-DPYBIND11_TEST_OVERRIDE="test_callbacks;test_pickling"`. If this is +empty, all tests will be built. Tests are specified without an extension if they need both a .py and +.cpp file. You may also pass flags to the `pytest` target by editing `tests/pytest.ini` or by using the `PYTEST_ADDOPTS` environment variable diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71ca17836..bdb7459ba 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -560,37 +560,37 @@ jobs: set +e; source /opt/intel/oneapi/setvars.sh; set -e cmake --build build-11 --target test_cmake_build - - name: Configure C++14 + - name: Configure C++17 run: | set +e; source /opt/intel/oneapi/setvars.sh; set -e - cmake -S . -B build-14 \ + cmake -S . -B build-17 \ -DPYBIND11_WERROR=ON \ -DDOWNLOAD_CATCH=ON \ -DDOWNLOAD_EIGEN=OFF \ - -DCMAKE_CXX_STANDARD=14 \ + -DCMAKE_CXX_STANDARD=17 \ -DCMAKE_CXX_COMPILER=$(which icpc) \ -DPYTHON_EXECUTABLE=$(python3 -c "import sys; print(sys.executable)") - - name: Build C++14 + - name: Build C++17 run: | set +e; source /opt/intel/oneapi/setvars.sh; set -e - cmake --build build-14 -j 2 -v + cmake --build build-17 -j 2 -v - - name: Python tests C++14 + - name: Python tests C++17 run: | set +e; source /opt/intel/oneapi/setvars.sh; set -e sudo service apport stop - cmake --build build-14 --target check + cmake --build build-17 --target check - - name: C++ tests C++14 + - name: C++ tests C++17 run: | set +e; source /opt/intel/oneapi/setvars.sh; set -e - cmake --build build-14 --target cpptest + cmake --build build-17 --target cpptest - - name: Interface test C++14 + - name: Interface test C++17 run: | set +e; source /opt/intel/oneapi/setvars.sh; set -e - cmake --build build-14 --target test_cmake_build + cmake --build build-17 --target test_cmake_build # Testing on CentOS (manylinux uses a centos base, and this is an easy way diff --git a/.github/workflows/pip.yml b/.github/workflows/pip.yml index 2a35ccba2..632d215d9 100644 --- a/.github/workflows/pip.yml +++ b/.github/workflows/pip.yml @@ -97,13 +97,13 @@ jobs: - uses: actions/download-artifact@v2 - name: Publish standard package - uses: pypa/gh-action-pypi-publish@v1.4.2 + uses: pypa/gh-action-pypi-publish@v1.5.0 with: password: ${{ secrets.pypi_password }} packages_dir: standard/ - name: Publish global package - uses: pypa/gh-action-pypi-publish@v1.4.2 + uses: pypa/gh-action-pypi-publish@v1.5.0 with: password: ${{ secrets.pypi_password_global }} packages_dir: global/ diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index ccdbc976b..4a76cca24 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -106,7 +106,7 @@ repos: # Check static types with mypy - repo: https://github.com/pre-commit/mirrors-mypy - rev: v0.930 + rev: v0.931 hooks: - id: mypy # Running per-file misbehaves a bit, so just run on all files, it's fast diff --git a/include/pybind11/chrono.h b/include/pybind11/chrono.h index 007cc17b4..460a28fa5 100644 --- a/include/pybind11/chrono.h +++ b/include/pybind11/chrono.h @@ -17,8 +17,6 @@ #include #include -#include - #include // Backport the PyDateTime_DELTA functions from Python3.3 if required @@ -108,7 +106,7 @@ inline std::tm *localtime_thread_safe(const std::time_t *time, std::tm *buf) { #else static std::mutex mtx; std::lock_guard lock(mtx); - std::tm *tm_ptr = localtime(time); + std::tm *tm_ptr = std::localtime(time); if (tm_ptr != nullptr) { *buf = *tm_ptr; } diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 1cc1e578b..91750e3fe 100644 --- a/include/pybind11/detail/class.h +++ b/include/pybind11/detail/class.h @@ -626,9 +626,9 @@ inline PyObject* make_new_python_type(const type_record &rec) { if (rec.doc && options::show_user_defined_docstrings()) { /* Allocate memory for docstring (using PyObject_MALLOC, since Python will free this later on) */ - size_t size = strlen(rec.doc) + 1; + size_t size = std::strlen(rec.doc) + 1; tp_doc = (char *) PyObject_MALLOC(size); - memcpy((void *) tp_doc, rec.doc, size); + std::memcpy((void *) tp_doc, rec.doc, size); } auto &internals = get_internals(); diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index af36340f3..9ab1ce9c0 100644 --- a/include/pybind11/embed.h +++ b/include/pybind11/embed.h @@ -110,13 +110,13 @@ inline wchar_t *widen_chars(const char *safe_arg) { #endif # if defined(HAVE_BROKEN_MBSTOWCS) && HAVE_BROKEN_MBSTOWCS - size_t count = strlen(safe_arg); + size_t count = std::strlen(safe_arg); # else - size_t count = mbstowcs(nullptr, safe_arg, 0); + size_t count = std::mbstowcs(nullptr, safe_arg, 0); # endif if (count != static_cast(-1)) { widened_arg = new wchar_t[count + 1]; - mbstowcs(widened_arg, safe_arg, count + 1); + std::mbstowcs(widened_arg, safe_arg, count + 1); } #if defined(_MSC_VER) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 5831f6a53..9b5a7e5de 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -25,7 +25,7 @@ #include #include -#include +#include #if defined(__cpp_lib_launder) && !(defined(_MSC_VER) && (_MSC_VER < 1914)) # define PYBIND11_STD_LAUNDER std::launder @@ -329,8 +329,8 @@ protected: a.descr = guarded_strdup(repr(a.value).cast().c_str()); } - rec->is_constructor - = (strcmp(rec->name, "__init__") == 0) || (strcmp(rec->name, "__setstate__") == 0); + rec->is_constructor = (std::strcmp(rec->name, "__init__") == 0) + || (std::strcmp(rec->name, "__setstate__") == 0); #if !defined(NDEBUG) && !defined(PYBIND11_DISABLE_NEW_STYLE_INIT_WARNING) if (rec->is_constructor && !rec->is_new_style_constructor) { @@ -411,10 +411,10 @@ protected: pybind11_fail("Internal error while parsing type signature (2)"); #if PY_MAJOR_VERSION < 3 - if (strcmp(rec->name, "__next__") == 0) { + if (std::strcmp(rec->name, "__next__") == 0) { std::free(rec->name); rec->name = guarded_strdup("next"); - } else if (strcmp(rec->name, "__bool__") == 0) { + } else if (std::strcmp(rec->name, "__bool__") == 0) { std::free(rec->name); rec->name = guarded_strdup("__nonzero__"); } @@ -998,6 +998,13 @@ protected: "Python type! The signature was\n\t"; msg += it->signature; append_note_if_missing_header_is_suspected(msg); +#if PY_VERSION_HEX >= 0x03030000 + // Attach additional error info to the exception if supported + if (PyErr_Occurred()) { + raise_from(PyExc_TypeError, msg.c_str()); + return nullptr; + } +#endif PyErr_SetString(PyExc_TypeError, msg.c_str()); return nullptr; } @@ -1298,8 +1305,8 @@ inline void call_operator_delete(void *p, size_t s, size_t a) { inline void add_class_method(object& cls, const char *name_, const cpp_function &cf) { cls.attr(cf.name()) = cf; - if (strcmp(name_, "__eq__") == 0 && !cls.attr("__dict__").contains("__hash__")) { - cls.attr("__hash__") = none(); + if (std::strcmp(name_, "__eq__") == 0 && !cls.attr("__dict__").contains("__hash__")) { + cls.attr("__hash__") = none(); } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 90865d320..e67023010 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -19,7 +19,7 @@ endif() # Only needed for CMake < 3.5 support include(CMakeParseArguments) -# Filter out items; print an optional message if any items filtered +# Filter out items; print an optional message if any items filtered. This ignores extensions. # # Usage: # pybind11_filter_tests(LISTNAME file1.cpp file2.cpp ... MESSAGE "") @@ -27,10 +27,17 @@ include(CMakeParseArguments) macro(pybind11_filter_tests LISTNAME) cmake_parse_arguments(ARG "" "MESSAGE" "" ${ARGN}) set(PYBIND11_FILTER_TESTS_FOUND OFF) + # Make a list of the test without any extensions, for easier filtering. + set(_TMP_ACTUAL_LIST "${${LISTNAME}};") # enforce ';' at the end to allow matching last item. + string(REGEX REPLACE "\\.[^.;]*;" ";" LIST_WITHOUT_EXTENSIONS "${_TMP_ACTUAL_LIST}") foreach(filename IN LISTS ARG_UNPARSED_ARGUMENTS) - list(FIND ${LISTNAME} ${filename} _FILE_FOUND) + string(REGEX REPLACE "\\.[^.]*$" "" filename_no_ext ${filename}) + # Search in the list without extensions. + list(FIND LIST_WITHOUT_EXTENSIONS ${filename_no_ext} _FILE_FOUND) if(_FILE_FOUND GREATER -1) - list(REMOVE_AT ${LISTNAME} ${_FILE_FOUND}) + list(REMOVE_AT ${LISTNAME} ${_FILE_FOUND}) # And remove from the list with extensions. + list(REMOVE_AT LIST_WITHOUT_EXTENSIONS ${_FILE_FOUND} + )# And our search list, to ensure it is in sync. set(PYBIND11_FILTER_TESTS_FOUND ON) endif() endforeach() @@ -47,6 +54,18 @@ macro(possibly_uninitialized) endforeach() endmacro() +# Function to add additional targets if any of the provided tests are found. +# Needles; Specifies the test names to look for. +# Additions; Specifies the additional test targets to add when any of the needles are found. +macro(tests_extra_targets needles additions) + # Add the index for this relation to the index extra targets map. + list(LENGTH PYBIND11_TEST_EXTRA_TARGETS PYBIND11_TEST_EXTRA_TARGETS_LEN) + list(APPEND PYBIND11_TEST_EXTRA_TARGETS ${PYBIND11_TEST_EXTRA_TARGETS_LEN}) + # Add the test names to look for, and the associated test target additions. + set(PYBIND11_TEST_EXTRA_TARGETS_NEEDLES_${PYBIND11_TEST_EXTRA_TARGETS_LEN} ${needles}) + set(PYBIND11_TEST_EXTRA_TARGETS_ADDITION_${PYBIND11_TEST_EXTRA_TARGETS_LEN} ${additions}) +endmacro() + # New Python support if(DEFINED Python_EXECUTABLE) set(PYTHON_EXECUTABLE "${Python_EXECUTABLE}") @@ -92,55 +111,67 @@ if(PYBIND11_CUDA_TESTS) set(CMAKE_CUDA_STANDARD_REQUIRED ON) endif() -# Full set of test files (you can override these; see below) +# Full set of test files (you can override these; see below, overrides ignore extension) +# Any test that has no extension is both .py and .cpp, so 'foo' will add 'foo.cpp' and 'foo.py'. +# Any test that has an extension is exclusively that and handled as such. set(PYBIND11_TEST_FILES - test_async.cpp - test_buffers.cpp - test_builtin_casters.cpp - test_call_policies.cpp - test_callbacks.cpp - test_chrono.cpp - test_class.cpp - test_const_name.cpp - test_constants_and_functions.cpp - test_copy_move.cpp - test_custom_type_casters.cpp - test_custom_type_setup.cpp - test_docstring_options.cpp - test_eigen.cpp - test_enum.cpp - test_eval.cpp - test_exceptions.cpp - test_factory_constructors.cpp - test_gil_scoped.cpp - test_iostream.cpp - test_kwargs_and_defaults.cpp - test_local_bindings.cpp - test_methods_and_attributes.cpp - test_modules.cpp - test_multiple_inheritance.cpp - test_numpy_array.cpp - test_numpy_dtypes.cpp - test_numpy_vectorize.cpp - test_opaque_types.cpp - test_operator_overloading.cpp - test_pickling.cpp - test_pytypes.cpp - test_sequences_and_iterators.cpp - test_smart_ptr.cpp - test_stl.cpp - test_stl_binders.cpp - test_tagbased_polymorphic.cpp - test_thread.cpp - test_union.cpp - test_virtual_functions.cpp) + test_async + test_buffers + test_builtin_casters + test_call_policies + test_callbacks + test_chrono + test_class + test_const_name + test_constants_and_functions + test_copy_move + test_custom_type_casters + test_custom_type_setup + test_docstring_options + test_eigen + test_enum + test_eval + test_exceptions + test_factory_constructors + test_gil_scoped + test_iostream + test_kwargs_and_defaults + test_local_bindings + test_methods_and_attributes + test_modules + test_multiple_inheritance + test_numpy_array + test_numpy_dtypes + test_numpy_vectorize + test_opaque_types + test_operator_overloading + test_pickling + test_pytypes + test_sequences_and_iterators + test_smart_ptr + test_stl + test_stl_binders + test_tagbased_polymorphic + test_thread + test_union + test_virtual_functions) # Invoking cmake with something like: # cmake -DPYBIND11_TEST_OVERRIDE="test_callbacks.cpp;test_pickling.cpp" .. # lets you override the tests that get compiled and run. You can restore to all tests with: # cmake -DPYBIND11_TEST_OVERRIDE= .. if(PYBIND11_TEST_OVERRIDE) - set(PYBIND11_TEST_FILES ${PYBIND11_TEST_OVERRIDE}) + # Instead of doing a direct override here, we iterate over the overrides without extension and + # match them against entries from the PYBIND11_TEST_FILES, anything that not matches goes into the filter list. + string(REGEX REPLACE "\\.[^.;]*;" ";" TEST_OVERRIDE_NO_EXT "${PYBIND11_TEST_OVERRIDE};") + string(REGEX REPLACE "\\.[^.;]*;" ";" TEST_FILES_NO_EXT "${PYBIND11_TEST_FILES};") + # This allows the override to be done with extensions, preserving backwards compatibility. + foreach(test_name ${TEST_FILES_NO_EXT}) + if(NOT ${test_name} IN_LIST TEST_OVERRIDE_NO_EXT + )# If not in the whitelist, add to be filtered out. + list(APPEND PYBIND11_TEST_FILTER ${test_name}) + endif() + endforeach() endif() # You can also filter tests: @@ -162,15 +193,34 @@ if(PYBIND11_CUDA_TESTS) "Skipping test_constants_and_functions due to incompatible exception specifications") endif() -string(REPLACE ".cpp" ".py" PYBIND11_PYTEST_FILES "${PYBIND11_TEST_FILES}") +# Now that the test filtering is complete, we need to split the list into the test for PYTEST +# and the list for the cpp targets. +set(PYBIND11_CPPTEST_FILES "") +set(PYBIND11_PYTEST_FILES "") + +foreach(test_name ${PYBIND11_TEST_FILES}) + if(test_name MATCHES "\\.py$") # Ends in .py, purely python test. + list(APPEND PYBIND11_PYTEST_FILES ${test_name}) + elseif(test_name MATCHES "\\.cpp$") # Ends in .cpp, purely cpp test. + list(APPEND PYBIND11_CPPTEST_FILES ${test_name}) + elseif(NOT test_name MATCHES "\\.") # No extension specified, assume both, add extension. + list(APPEND PYBIND11_PYTEST_FILES ${test_name}.py) + list(APPEND PYBIND11_CPPTEST_FILES ${test_name}.cpp) + else() + message(WARNING "Unhanded test extension in test: ${test_name}") + endif() +endforeach() +set(PYBIND11_TEST_FILES ${PYBIND11_CPPTEST_FILES}) +list(SORT PYBIND11_PYTEST_FILES) # Contains the set of test files that require pybind11_cross_module_tests to be # built; if none of these are built (i.e. because TEST_OVERRIDE is used and # doesn't include them) the second module doesn't get built. -set(PYBIND11_CROSS_MODULE_TESTS test_exceptions.py test_local_bindings.py test_stl.py - test_stl_binders.py) +tests_extra_targets("test_exceptions.py;test_local_bindings.py;test_stl.py;test_stl_binders.py" + "pybind11_cross_module_tests") -set(PYBIND11_CROSS_MODULE_GIL_TESTS test_gil_scoped.py) +# And add additional targets for other tests. +tests_extra_targets("test_gil_scoped.py" "cross_module_gil_utils") set(PYBIND11_EIGEN_REPO "https://gitlab.com/libeigen/eigen.git" @@ -327,6 +377,9 @@ function(pybind11_enable_warnings target_name) elseif(CMAKE_CXX_COMPILER_ID MATCHES "(GNU|Clang|IntelLLVM)") target_compile_options(${target_name} PRIVATE -Werror) elseif(CMAKE_CXX_COMPILER_ID STREQUAL "Intel") + if(CMAKE_CXX_STANDARD EQUAL 17) # See PR #3570 + target_compile_options(${target_name} PRIVATE -Wno-conversion) + endif() target_compile_options( ${target_name} PRIVATE @@ -350,21 +403,17 @@ endfunction() set(test_targets pybind11_tests) -# Build pybind11_cross_module_tests if any test_whatever.py are being built that require it -foreach(t ${PYBIND11_CROSS_MODULE_TESTS}) - list(FIND PYBIND11_PYTEST_FILES ${t} i) - if(i GREATER -1) - list(APPEND test_targets pybind11_cross_module_tests) - break() - endif() -endforeach() - -foreach(t ${PYBIND11_CROSS_MODULE_GIL_TESTS}) - list(FIND PYBIND11_PYTEST_FILES ${t} i) - if(i GREATER -1) - list(APPEND test_targets cross_module_gil_utils) - break() - endif() +# Check if any tests need extra targets by iterating through the mappings registered. +foreach(i ${PYBIND11_TEST_EXTRA_TARGETS}) + foreach(needle ${PYBIND11_TEST_EXTRA_TARGETS_NEEDLES_${i}}) + if(needle IN_LIST PYBIND11_PYTEST_FILES) + # Add all the additional targets to the test list. List join in newer cmake. + foreach(extra_target ${PYBIND11_TEST_EXTRA_TARGETS_ADDITION_${i}}) + list(APPEND test_targets ${extra_target}) + endforeach() + break() # Breaks out of the needle search, continues with the next mapping. + endif() + endforeach() endforeach() # Support CUDA testing by forcing the target file to compile with NVCC diff --git a/tests/test_operator_overloading.cpp b/tests/test_operator_overloading.cpp index 0b6c496cf..ffa059d5b 100644 --- a/tests/test_operator_overloading.cpp +++ b/tests/test_operator_overloading.cpp @@ -7,10 +7,11 @@ BSD-style license that can be found in the LICENSE file. */ -#include "pybind11_tests.h" #include "constructor_stats.h" -#include +#include "pybind11_tests.h" #include +#include +#include class Vector2 { public: @@ -71,6 +72,12 @@ int operator+(const C2 &, const C2 &) { return 22; } int operator+(const C2 &, const C1 &) { return 21; } int operator+(const C1 &, const C2 &) { return 12; } +struct HashMe { + std::string member; +}; + +bool operator==(const HashMe &lhs, const HashMe &rhs) { return lhs.member == rhs.member; } + // Note: Specializing explicit within `namespace std { ... }` is done due to a // bug in GCC<7. If you are supporting compilers later than this, consider // specializing `using template<> struct std::hash<...>` in the global @@ -82,6 +89,14 @@ namespace std { // Not a good hash function, but easy to test size_t operator()(const Vector2 &) { return 4; } }; + + // HashMe has a hash function in C++ but no `__hash__` for Python. + template <> + struct hash { + std::size_t operator()(const HashMe &selector) const { + return std::hash()(selector.member); + } + }; } // namespace std // Not a good abs function, but easy to test. @@ -228,8 +243,12 @@ TEST_SUBMODULE(operators, m) { .def("__hash__", &Hashable::hash) .def(py::init()) .def(py::self == py::self); -} + // define __eq__ but not __hash__ + py::class_(m, "HashMe").def(py::self == py::self); + + m.def("get_unhashable_HashMe_set", []() { return std::unordered_set{{"one"}}; }); +} #if !defined(_MSC_VER) && !defined(__INTEL_COMPILER) #pragma GCC diagnostic pop #endif diff --git a/tests/test_operator_overloading.py b/tests/test_operator_overloading.py index b7137d159..8cf375b6d 100644 --- a/tests/test_operator_overloading.py +++ b/tests/test_operator_overloading.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import pytest +import env from pybind11_tests import ConstructorStats from pybind11_tests import operators as m @@ -135,8 +136,9 @@ def test_overriding_eq_reset_hash(): assert m.Comparable(15) is not m.Comparable(15) assert m.Comparable(15) == m.Comparable(15) - with pytest.raises(TypeError): - hash(m.Comparable(15)) # TypeError: unhashable type: 'm.Comparable' + with pytest.raises(TypeError) as excinfo: + hash(m.Comparable(15)) + assert str(excinfo.value).startswith("unhashable type:") for hashable in (m.Hashable, m.Hashable2): assert hashable(15) is not hashable(15) @@ -144,3 +146,10 @@ def test_overriding_eq_reset_hash(): assert hash(hashable(15)) == 15 assert hash(hashable(15)) == hash(hashable(15)) + + +def test_return_set_of_unhashable(): + with pytest.raises(TypeError) as excinfo: + m.get_unhashable_HashMe_set() + if not env.PY2: + assert str(excinfo.value.__cause__).startswith("unhashable type:") diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index a9c945557..ca73922be 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -350,7 +350,7 @@ TEST_SUBMODULE(virtual_functions, m) { m.def("add3", [](const AdderBase::Data& first, const AdderBase::Data& second, const AdderBase::Data& third, const AdderBase& adder, const AdderBase::DataVisitor& visitor) { adder(first, second, [&] (const AdderBase::Data& first_plus_second) { - adder(first_plus_second, third, visitor); + adder(first_plus_second, third, visitor); // NOLINT(readability-suspicious-call-argument) }); });