diff --git a/.clang-tidy b/.clang-tidy index 96cb6f582..3a1995c32 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -63,6 +63,7 @@ Checks: | -bugprone-reserved-identifier, -bugprone-unused-raii, -performance-enum-size, + -performance-inefficient-string-concatenation, CheckOptions: - key: modernize-use-equals-default.IgnoreMacros diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 8ce286f5c..fbdf075f0 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -262,17 +262,17 @@ of the pybind11 repo. more complex to run, compared to `clang-format`, but support for `clang-tidy` is built into the pybind11 CMake configuration. To run `clang-tidy`, the following recipe should work. Run the `docker` command from the top-level -directory inside your pybind11 git clone. Files will be modified in place, -so you can use git to monitor the changes. +directory inside your pybind11 git clone. ```bash -docker run --rm -v $PWD:/mounted_pybind11 -it silkeh/clang:15-bullseye -apt-get update && apt-get install -y git python3-dev python3-pytest -cmake -S /mounted_pybind11/ -B build -DCMAKE_CXX_CLANG_TIDY="$(which clang-tidy);--use-color" -DDOWNLOAD_EIGEN=ON -DDOWNLOAD_CATCH=ON -DCMAKE_CXX_STANDARD=17 -cmake --build build -j 2 +docker run --rm -v $PWD:/pybind11 -w /pybind11 -it silkeh/clang:20 +apt-get update && apt-get install -y git python3-dev python3-pytest ninja-build +cmake --preset tidy +cmake --build --preset tidy ``` -You can add `--fix` to the options list if you want. +You can add `--fix` to the options list in the preset if you want to apply fixes +(remember `-j1` to run only one thread). ### Include what you use @@ -281,7 +281,7 @@ macOS), then run: ```bash cmake -S . -B build-iwyu -DCMAKE_CXX_INCLUDE_WHAT_YOU_USE=$(which include-what-you-use) -cmake --build build +cmake --build build-iwyu ``` The report is sent to stderr; you can pipe it into a file if you wish. diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index bc2db70ea..9258e2792 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -38,23 +38,17 @@ jobs: # in .github/CONTRIBUTING.md and update as needed. name: Clang-Tidy runs-on: ubuntu-latest - container: silkeh/clang:18-bookworm + container: silkeh/clang:20 steps: - uses: actions/checkout@v4 - name: Install requirements - run: apt-get update && apt-get install -y git python3-dev python3-pytest + run: apt-get update && apt-get install -y git python3-dev python3-pytest ninja-build - name: Configure - run: > - cmake -S . -B build - -DCMAKE_CXX_CLANG_TIDY="$(which clang-tidy);--use-color;--warnings-as-errors=*" - -DDOWNLOAD_EIGEN=ON - -DDOWNLOAD_CATCH=ON - -DCMAKE_CXX_STANDARD=17 - + run: cmake --preset tidy - name: Build - run: cmake --build build -j 2 -- --keep-going + run: cmake --build --preset tidy - name: Embedded - run: cmake --build build -t cpptest -j 2 -- --keep-going + run: cmake --build --preset tidy -t cpptest diff --git a/CMakePresets.json b/CMakePresets.json index bf17f858e..94688bfef 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -25,15 +25,19 @@ "displayName": "Venv", "inherits": "default", "cacheVariables": { - "CMAKE_BUILD_TYPE": "Debug", - "CMAKE_EXPORT_COMPILE_COMMANDS": true, - "DOWNLOAD_CATCH": true, - "DOWNLOAD_EIGEN": true, "PYBIND11_CREATE_WITH_UV": "python3", - "PYBIND11_FINDPYTHON": "NEW", - "PYBIND11_WERROR": true, "Python_ROOT_DIR": ".venv" } + }, + { + "name": "tidy", + "displayName": "Clang-tidy", + "inherits": "default", + "binaryDir": "build-tidy", + "cacheVariables": { + "CMAKE_CXX_CLANG_TIDY": "clang-tidy;--use-color;--warnings-as-errors=*", + "CMAKE_CXX_STANDARD": "17" + } } ], "buildPresets": [ @@ -47,6 +51,12 @@ "displayName": "Venv Build", "configurePreset": "venv" }, + { + "name": "tidy", + "displayName": "Clang-tidy Build", + "configurePreset": "tidy", + "nativeToolOptions": ["-k0"] + }, { "name": "tests", "displayName": "Tests (for workflow)", diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 0cc63b0b4..e3b603802 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -817,7 +817,9 @@ protected: cast_impl(T &&src, return_value_policy policy, handle parent, index_sequence) { PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(src, policy, parent); PYBIND11_WORKAROUND_INCORRECT_GCC_UNUSED_BUT_SET_PARAMETER(policy, parent); + std::array entries{{reinterpret_steal( + // NOLINTNEXTLINE(bugprone-use-after-move) make_caster::cast(std::get(std::forward(src)), policy, parent))...}}; for (const auto &entry : entries) { if (!entry) { diff --git a/include/pybind11/chrono.h b/include/pybind11/chrono.h index 69e59b4d6..d9bcc4bc4 100644 --- a/include/pybind11/chrono.h +++ b/include/pybind11/chrono.h @@ -63,6 +63,9 @@ public: get_duration(const std::chrono::duration &src) { return src; } + static const std::chrono::duration & + get_duration(const std::chrono::duration &&) + = delete; // If this is a time_point get the time_since_epoch template diff --git a/include/pybind11/detail/init.h b/include/pybind11/detail/init.h index 92b20cb97..aa993f495 100644 --- a/include/pybind11/detail/init.h +++ b/include/pybind11/detail/init.h @@ -273,7 +273,8 @@ struct constructor { static void execute(Class &cl, const Extra &...extra) { cl.def( "__init__", - [](value_and_holder &v_h, Args... args) { + [](value_and_holder &v_h, + Args... args) { // NOLINT(performance-unnecessary-value-param) v_h.value_ptr() = construct_or_initialize>(std::forward(args)...); }, is_new_style_constructor(), diff --git a/include/pybind11/detail/struct_smart_holder.h b/include/pybind11/detail/struct_smart_holder.h index 5466f8668..758182119 100644 --- a/include/pybind11/detail/struct_smart_holder.h +++ b/include/pybind11/detail/struct_smart_holder.h @@ -340,7 +340,7 @@ struct smart_holder { } template - static smart_holder from_shared_ptr(std::shared_ptr shd_ptr) { + static smart_holder from_shared_ptr(const std::shared_ptr &shd_ptr) { smart_holder hld; hld.vptr = std::static_pointer_cast(shd_ptr); hld.vptr_is_external_shared_ptr = true; diff --git a/include/pybind11/functional.h b/include/pybind11/functional.h index 9327a1364..8f59f5fe5 100644 --- a/include/pybind11/functional.h +++ b/include/pybind11/functional.h @@ -48,7 +48,7 @@ struct func_wrapper_base { template struct func_wrapper : func_wrapper_base { using func_wrapper_base::func_wrapper_base; - Return operator()(Args... args) const { + Return operator()(Args... args) const { // NOLINT(performance-unnecessary-value-param) gil_scoped_acquire acq; // casts the returned object as a rvalue to the return type return hfunc.f(std::forward(args)...).template cast(); diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 9650be596..3d2739264 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -3043,6 +3043,7 @@ template +// NOLINTNEXTLINE(performance-unnecessary-value-param) iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&...extra) { using state = detail::iterator_state; // TODO: state captures only the types of Extra, not the values @@ -3082,6 +3083,7 @@ template ::result_type, typename... Extra> +// NOLINTNEXTLINE(performance-unnecessary-value-param) typing::Iterator make_iterator(Iterator first, Sentinel last, Extra &&...extra) { return detail::make_iterator_impl, Policy, diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index fb78c0f2b..9c60c94c0 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -81,7 +81,9 @@ using is_pyobject = std::is_base_of>; \endrst */ template class object_api : public pyobject_tag { + object_api() = default; const Derived &derived() const { return static_cast(*this); } + friend Derived; public: /** \rst @@ -276,7 +278,7 @@ public: inc_ref_counter(1); #endif #ifdef PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF - if (m_ptr != nullptr && !PyGILState_Check()) { + if (m_ptr != nullptr && PyGILState_Check() == 0) { throw_gilstate_error("pybind11::handle::inc_ref()"); } #endif @@ -291,7 +293,7 @@ public: \endrst */ const handle &dec_ref() const & { #ifdef PYBIND11_ASSERT_GIL_HELD_INCREF_DECREF - if (m_ptr != nullptr && !PyGILState_Check()) { + if (m_ptr != nullptr && PyGILState_Check() == 0) { throw_gilstate_error("pybind11::handle::dec_ref()"); } #endif diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h index ace5ed13b..31cf9b20c 100644 --- a/include/pybind11/stl.h +++ b/include/pybind11/stl.h @@ -178,7 +178,7 @@ private: return true; } - bool convert_anyset(anyset s, bool convert) { + bool convert_anyset(const anyset &s, bool convert) { value.clear(); reserve_maybe(s, &value); return convert_iterable(s, convert); diff --git a/tests/pure_cpp/smart_holder_poc.h b/tests/pure_cpp/smart_holder_poc.h index e57ec13f4..37160f1e6 100644 --- a/tests/pure_cpp/smart_holder_poc.h +++ b/tests/pure_cpp/smart_holder_poc.h @@ -10,6 +10,7 @@ PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(memory) PYBIND11_NAMESPACE_BEGIN(smart_holder_poc) // Proof-of-Concept implementations. +// NOLINTNEXTLINE(bugprone-incorrect-enable-shared-from-this) struct PrivateESFT : private std::enable_shared_from_this {}; static_assert(!type_has_shared_from_this(static_cast(nullptr)), "should detect inaccessible base"); diff --git a/tests/test_class_sh_basic.cpp b/tests/test_class_sh_basic.cpp index b89372b19..d9cbe1f6b 100644 --- a/tests/test_class_sh_basic.cpp +++ b/tests/test_class_sh_basic.cpp @@ -130,7 +130,8 @@ const std::unique_ptr &rtrn_unique_ptr_cref(const std::string &mtxt) { } const std::unique_ptr &unique_ptr_cref_roundtrip(const std::unique_ptr &obj) { - return obj; + // This is passed in, so it can't be given a temporary + return obj; // NOLINT(bugprone-return-const-ref-from-parameter) } struct SharedPtrStash { diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp index ddffbe3a9..9ac4d98d7 100644 --- a/tests/test_copy_move.cpp +++ b/tests/test_copy_move.cpp @@ -19,6 +19,10 @@ template struct empty { static const derived &get_one() { return instance_; } static derived instance_; + +private: + empty() = default; + friend derived; }; struct lacking_copy_ctor : public empty { diff --git a/tests/test_numpy_dtypes.cpp b/tests/test_numpy_dtypes.cpp index 04bf19f3e..f206da732 100644 --- a/tests/test_numpy_dtypes.cpp +++ b/tests/test_numpy_dtypes.cpp @@ -176,7 +176,7 @@ py::array_t create_recarray(size_t n) { } template -py::list print_recarray(py::array_t arr) { +py::list print_recarray(const py::array_t &arr) { const auto req = arr.request(); auto *const ptr = static_cast(req.ptr); auto l = py::list(); @@ -301,7 +301,7 @@ py::list test_dtype_ctors() { } template -py::array_t dispatch_array_increment(py::array_t arr) { +py::array_t dispatch_array_increment(const py::array_t &arr) { py::array_t res(arr.shape(0)); for (py::ssize_t i = 0; i < arr.shape(0); ++i) { res.mutable_at(i) = T(arr.at(i) + 1); diff --git a/tests/test_sequences_and_iterators.cpp b/tests/test_sequences_and_iterators.cpp index c997b7300..ccb0d1499 100644 --- a/tests/test_sequences_and_iterators.cpp +++ b/tests/test_sequences_and_iterators.cpp @@ -90,7 +90,7 @@ PYBIND11_MAKE_OPAQUE(std::vector) PYBIND11_MAKE_OPAQUE(std::vector) template -py::list test_random_access_iterator(PythonType x) { +py::list test_random_access_iterator(const PythonType &x) { if (x.size() < 5) { throw py::value_error("Please provide at least 5 elements for testing."); } diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 19b7b0d80..10a65f535 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -540,6 +540,7 @@ TEST_SUBMODULE(smart_ptr, m) { return list; }); + // NOLINTNEXTLINE(bugprone-incorrect-enable-shared-from-this) class PrivateESFT : /* implicit private */ std::enable_shared_from_this {}; struct ContainerUsingPrivateESFT { std::shared_ptr ptr;