chore: bump maximum clang tested to 20 (#5692)

.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

.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.

.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

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)",

include/pybind11/cast.h

@@ -817,7 +817,9 @@ protected:
     cast_impl(T &&src, return_value_policy policy, handle parent, index_sequence<Is...>) {
         PYBIND11_WORKAROUND_INCORRECT_MSVC_C4100(src, policy, parent);
         PYBIND11_WORKAROUND_INCORRECT_GCC_UNUSED_BUT_SET_PARAMETER(policy, parent);
+
         std::array<object, size> entries{{reinterpret_steal<object>(
+            // NOLINTNEXTLINE(bugprone-use-after-move)
             make_caster<Ts>::cast(std::get<Is>(std::forward<T>(src)), policy, parent))...}};
         for (const auto &entry : entries) {
             if (!entry) {

include/pybind11/chrono.h

@@ -63,6 +63,9 @@ public:
     get_duration(const std::chrono::duration<rep, period> &src) {
         return src;
     }
+    static const std::chrono::duration<rep, period> &
+    get_duration(const std::chrono::duration<rep, period> &&)
+        = delete;
 
     // If this is a time_point get the time_since_epoch
     template <typename Clock>

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<Cpp<Class>>(std::forward<Args>(args)...);
             },
             is_new_style_constructor(),

include/pybind11/detail/struct_smart_holder.h

@@ -340,7 +340,7 @@ struct smart_holder {
     }
 
     template <typename T>
-    static smart_holder from_shared_ptr(std::shared_ptr<T> shd_ptr) {
+    static smart_holder from_shared_ptr(const std::shared_ptr<T> &shd_ptr) {
         smart_holder hld;
         hld.vptr = std::static_pointer_cast<void>(shd_ptr);
         hld.vptr_is_external_shared_ptr = true;

include/pybind11/functional.h

@@ -48,7 +48,7 @@ struct func_wrapper_base {
 template <typename Return, typename... Args>
 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>(args)...).template cast<Return>();

include/pybind11/pybind11.h

@@ -3043,6 +3043,7 @@ template <typename Access,
           typename Sentinel,
           typename ValueType,
           typename... Extra>
+// NOLINTNEXTLINE(performance-unnecessary-value-param)
 iterator make_iterator_impl(Iterator first, Sentinel last, Extra &&...extra) {
     using state = detail::iterator_state<Access, Policy, Iterator, Sentinel, ValueType, Extra...>;
     // TODO: state captures only the types of Extra, not the values
@@ -3082,6 +3083,7 @@ template <return_value_policy Policy = return_value_policy::reference_internal,
           typename Sentinel,
           typename ValueType = typename detail::iterator_access<Iterator>::result_type,
           typename... Extra>
+// NOLINTNEXTLINE(performance-unnecessary-value-param)
 typing::Iterator<ValueType> make_iterator(Iterator first, Sentinel last, Extra &&...extra) {
     return detail::make_iterator_impl<detail::iterator_access<Iterator>,
                                       Policy,

include/pybind11/pytypes.h

@@ -81,7 +81,9 @@ using is_pyobject = std::is_base_of<pyobject_tag, remove_reference_t<T>>;
 \endrst */
 template <typename Derived>
 class object_api : public pyobject_tag {
+    object_api() = default;
     const Derived &derived() const { return static_cast<const Derived &>(*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

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);

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<PrivateESFT> {};
 static_assert(!type_has_shared_from_this(static_cast<PrivateESFT *>(nullptr)),
               "should detect inaccessible base");

tests/test_class_sh_basic.cpp

@@ -130,7 +130,8 @@ const std::unique_ptr<atyp> &rtrn_unique_ptr_cref(const std::string &mtxt) {
 }
 
 const std::unique_ptr<atyp> &unique_ptr_cref_roundtrip(const std::unique_ptr<atyp> &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 {

tests/test_copy_move.cpp

@@ -19,6 +19,10 @@ template <typename derived>
 struct empty {
     static const derived &get_one() { return instance_; }
     static derived instance_;
+
+private:
+    empty() = default;
+    friend derived;
 };
 
 struct lacking_copy_ctor : public empty<lacking_copy_ctor> {

tests/test_numpy_dtypes.cpp

@@ -176,7 +176,7 @@ py::array_t<S, 0> create_recarray(size_t n) {
 }
 
 template <typename S>
-py::list print_recarray(py::array_t<S, 0> arr) {
+py::list print_recarray(const py::array_t<S, 0> &arr) {
     const auto req = arr.request();
     auto *const ptr = static_cast<S *>(req.ptr);
     auto l = py::list();
@@ -301,7 +301,7 @@ py::list test_dtype_ctors() {
 }
 
 template <typename T>
-py::array_t<T> dispatch_array_increment(py::array_t<T> arr) {
+py::array_t<T> dispatch_array_increment(const py::array_t<T> &arr) {
     py::array_t<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);

tests/test_sequences_and_iterators.cpp

@@ -90,7 +90,7 @@ PYBIND11_MAKE_OPAQUE(std::vector<NonCopyableInt>)
 PYBIND11_MAKE_OPAQUE(std::vector<NonCopyableIntPair>)
 
 template <typename PythonType>
-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.");
     }

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<PrivateESFT> {};
     struct ContainerUsingPrivateESFT {
         std::shared_ptr<PrivateESFT> ptr;