[smart_holder] git merge master clang tidy followup

.clang-tidy

@@ -5,13 +5,17 @@ cppcoreguidelines-init-variables,
 clang-analyzer-optin.cplusplus.VirtualCall,
 llvm-namespace-comment,
 misc-misplaced-const,
+misc-non-copyable-objects,
 misc-static-assert,
 misc-throw-by-value-catch-by-reference,
 misc-uniqueptr-reset-release,
+misc-unused-parameters,
 modernize-avoid-bind,
+modernize-make-shared,
 modernize-redundant-void-arg,
 modernize-replace-auto-ptr,
 modernize-replace-disallow-copy-and-assign-macro,
+modernize-replace-random-shuffle,
 modernize-shrink-to-fit,
 modernize-use-auto,
 modernize-use-bool-literals,
@@ -23,13 +27,20 @@ modernize-use-emplace,
 modernize-use-override,
 modernize-use-using,
 *performance*,
+readability-avoid-const-params-in-decls,
 readability-container-size-empty,
 readability-else-after-return,
+readability-delete-null-pointer,
+readability-implicit-bool-conversion,
 readability-make-member-function-const,
+readability-misplaced-array-index,
+readability-non-const-parameter,
 readability-redundant-function-ptr-dereference,
 readability-redundant-smartptr-get,
 readability-redundant-string-cstr,
 readability-simplify-subscript-expr,
+readability-static-accessed-through-instance,
+readability-static-definition-in-anonymous-namespace,
 readability-string-compare,
 readability-uniqueptr-delete-release,
 '
@@ -39,6 +50,8 @@ CheckOptions:
   value:           true
 - key:             performance-unnecessary-value-param.AllowedTypes
   value:           'exception_ptr$;'
+- key:             readability-implicit-bool-conversion.AllowPointerConditions
+  value:           true
 
 HeaderFilterRegex: 'pybind11/.*h'
 

CMakeLists.txt

@@ -210,6 +210,12 @@ if(PYBIND11_INSTALL)
       "${CMAKE_INSTALL_DATAROOTDIR}/cmake/${PROJECT_NAME}"
       CACHE STRING "install path for pybind11Config.cmake")
 
+  if(IS_ABSOLUTE "${CMAKE_INSTALL_INCLUDEDIR}")
+    set(pybind11_INCLUDEDIR "${CMAKE_INSTALL_FULL_INCLUDEDIR}")
+  else()
+    set(pybind11_INCLUDEDIR "\$\{PACKAGE_PREFIX_DIR\}/${CMAKE_INSTALL_INCLUDEDIR}")
+  endif()
+
   configure_package_config_file(
     tools/${PROJECT_NAME}Config.cmake.in "${CMAKE_CURRENT_BINARY_DIR}/${PROJECT_NAME}Config.cmake"
     INSTALL_DESTINATION ${PYBIND11_CMAKECONFIG_INSTALL_DIR})

include/pybind11/cast.h

@@ -203,7 +203,7 @@ struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value && !is_std_char_t
         // Signed/unsigned checks happen elsewhere
         if (py_err || (std::is_integral<T>::value && sizeof(py_type) != sizeof(T) && py_value != (py_type) (T) py_value)) {
             PyErr_Clear();
-            if (py_err && convert && PyNumber_Check(src.ptr())) {
+            if (py_err && convert && (PyNumber_Check(src.ptr()) != 0)) {
                 auto tmp = reinterpret_steal<object>(std::is_floating_point<T>::value
                                                      ? PyNumber_Float(src.ptr())
                                                      : PyNumber_Long(src.ptr()));
@@ -322,7 +322,7 @@ template <> class type_caster<bool> {
             value = false;
             return true;
         }
-        if (convert || !std::strcmp("numpy.bool_", Py_TYPE(src.ptr())->tp_name)) {
+        if (convert || (std::strcmp("numpy.bool_", Py_TYPE(src.ptr())->tp_name) == 0)) {
             // (allow non-implicit conversion for numpy booleans)
 
             Py_ssize_t res = -1;
@@ -523,10 +523,14 @@ template <typename CharT> struct type_caster<CharT, enable_if_t<is_std_char_type
         // can fit into a single char value.
         if (StringCaster::UTF_N == 8 && str_len > 1 && str_len <= 4) {
             auto v0 = static_cast<unsigned char>(value[0]);
-            size_t char0_bytes = !(v0 & 0x80) ? 1 : // low bits only: 0-127
-                (v0 & 0xE0) == 0xC0 ? 2 : // 0b110xxxxx - start of 2-byte sequence
-                (v0 & 0xF0) == 0xE0 ? 3 : // 0b1110xxxx - start of 3-byte sequence
-                4; // 0b11110xxx - start of 4-byte sequence
+            // low bits only: 0-127
+            // 0b110xxxxx - start of 2-byte sequence
+            // 0b1110xxxx - start of 3-byte sequence
+            // 0b11110xxx - start of 4-byte sequence
+            size_t char0_bytes = (v0 & 0x80) == 0      ? 1
+                                 : (v0 & 0xE0) == 0xC0 ? 2
+                                 : (v0 & 0xF0) == 0xE0 ? 3
+                                                       : 4;
 
             if (char0_bytes == str_len) {
                 // If we have a 128-255 value, we can decode it into a single char:

include/pybind11/detail/class.h

@@ -129,8 +129,9 @@ extern "C" inline int pybind11_meta_setattro(PyObject* obj, PyObject* name, PyOb
     //   2. `Type.static_prop = other_static_prop` --> setattro:  replace existing `static_prop`
     //   3. `Type.regular_attribute = value`       --> setattro:  regular attribute assignment
     const auto static_prop = (PyObject *) get_internals().static_property_type;
-    const auto call_descr_set = descr && value && PyObject_IsInstance(descr, static_prop)
-                                && !PyObject_IsInstance(value, static_prop);
+    const auto call_descr_set = (descr != nullptr) && (value != nullptr)
+                                && (PyObject_IsInstance(descr, static_prop) != 0)
+                                && (PyObject_IsInstance(value, static_prop) == 0);
     if (call_descr_set) {
         // Call `static_property.__set__()` instead of replacing the `static_property`.
 #if !defined(PYPY_VERSION)
@@ -564,7 +565,7 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla
     view->len = view->itemsize;
     for (auto s : info->shape)
         view->len *= s;
-    view->readonly = info->readonly;
+    view->readonly = static_cast<int>(info->readonly);
     if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT)
         view->format = const_cast<char *>(info->format.c_str());
     if ((flags & PyBUF_STRIDES) == PyBUF_STRIDES) {

include/pybind11/detail/common.h

@@ -56,6 +56,9 @@
 #  elif __INTEL_COMPILER < 1900 && defined(PYBIND11_CPP14)
 #    error pybind11 supports only C++11 with Intel C++ compiler v18. Use v19 or newer for C++14.
 #  endif
+/* The following pragma cannot be pop'ed:
+   https://community.intel.com/t5/Intel-C-Compiler/Inline-and-no-inline-warning/td-p/1216764 */
+#  pragma warning disable 2196 // warning #2196: routine is both "inline" and "noinline"
 #elif defined(__clang__) && !defined(__apple_build_version__)
 #  if __clang_major__ < 3 || (__clang_major__ == 3 && __clang_minor__ < 3)
 #    error pybind11 requires clang 3.3 or newer

include/pybind11/detail/internals.h

@@ -304,7 +304,7 @@ PYBIND11_NOINLINE inline internals &get_internals() {
         PyThreadState *tstate = PyThreadState_Get();
         #if PY_VERSION_HEX >= 0x03070000
             internals_ptr->tstate = PyThread_tss_alloc();
-            if (!internals_ptr->tstate || PyThread_tss_create(internals_ptr->tstate))
+            if (!internals_ptr->tstate || (PyThread_tss_create(internals_ptr->tstate) != 0))
                 pybind11_fail("get_internals: could not successfully initialize the TSS key!");
             PyThread_tss_set(internals_ptr->tstate, tstate);
         #else

include/pybind11/detail/pragma_warning_block.h

@@ -1,15 +1,11 @@
 #pragma once
 
-#if defined(__INTEL_COMPILER)
-#  pragma warning push
-#  pragma warning disable 878   // incompatible exception specifications
-#  pragma warning disable 2196  // warning #2196: routine is both "inline" and "noinline"
-#elif defined(_MSC_VER)
+#if defined(_MSC_VER) && !defined(__INTEL_COMPILER)
 #  pragma warning(push)
 #  pragma warning(disable: 4100) // warning C4100: Unreferenced formal parameter
 #  pragma warning(disable: 4127) // warning C4127: Conditional expression is constant
 #  pragma warning(disable: 4505) // warning C4505: 'PySlice_GetIndicesEx': unreferenced local function has been removed (PyPy only)
-#elif defined(__GNUG__) && !defined(__clang__)
+#elif defined(__GNUG__) && !defined(__clang__) && !defined(__INTEL_COMPILER)
 #  pragma GCC diagnostic push
 #  pragma GCC diagnostic ignored "-Wunused-but-set-parameter"
 #  pragma GCC diagnostic ignored "-Wattributes"

include/pybind11/detail/type_caster_base.h

@@ -238,8 +238,8 @@ struct value_and_holder {
     }
     bool holder_constructed() const {
         return inst->simple_layout
-            ? inst->simple_holder_constructed
-            : inst->nonsimple.status[index] & instance::status_holder_constructed;
+                   ? inst->simple_holder_constructed
+                   : (inst->nonsimple.status[index] & instance::status_holder_constructed) != 0u;
     }
     void set_holder_constructed(bool v = true) const {
         if (inst->simple_layout)

include/pybind11/embed.h

@@ -76,7 +76,7 @@ struct embedded_module {
     using init_t = void (*)();
 #endif
     embedded_module(const char *name, init_t init) {
-        if (Py_IsInitialized())
+        if (Py_IsInitialized() != 0)
             pybind11_fail("Can't add new modules after the interpreter has been initialized");
 
         auto result = PyImport_AppendInittab(name, init);
@@ -101,7 +101,7 @@ PYBIND11_NAMESPACE_END(detail)
     .. _Python documentation: https://docs.python.org/3/c-api/init.html#c.Py_InitializeEx
  \endrst */
 inline void initialize_interpreter(bool init_signal_handlers = true) {
-    if (Py_IsInitialized())
+    if (Py_IsInitialized() != 0)
         pybind11_fail("The interpreter is already running");
 
     Py_InitializeEx(init_signal_handlers ? 1 : 0);

include/pybind11/numpy.h

@@ -465,7 +465,9 @@ class dtype : public object {
     explicit dtype(const buffer_info &info) {
         dtype descr(_dtype_from_pep3118()(PYBIND11_STR_TYPE(info.format)));
         // If info.itemsize == 0, use the value calculated from the format string
-        m_ptr = descr.strip_padding(info.itemsize ? info.itemsize : descr.itemsize()).release().ptr();
+        m_ptr = descr.strip_padding(info.itemsize != 0 ? info.itemsize : descr.itemsize())
+                    .release()
+                    .ptr();
     }
 
     explicit dtype(const std::string &format) {
@@ -486,7 +488,7 @@ class dtype : public object {
     /// This is essentially the same as calling numpy.dtype(args) in Python.
     static dtype from_args(object args) {
         PyObject *ptr = nullptr;
-        if (!detail::npy_api::get().PyArray_DescrConverter_(args.ptr(), &ptr) || !ptr)
+        if ((detail::npy_api::get().PyArray_DescrConverter_(args.ptr(), &ptr) == 0) || !ptr)
             throw error_already_set();
         return reinterpret_steal<dtype>(ptr);
     }
@@ -542,7 +544,7 @@ class dtype : public object {
             auto name = spec[0].cast<pybind11::str>();
             auto format = spec[1].cast<tuple>()[0].cast<dtype>();
             auto offset = spec[1].cast<tuple>()[1].cast<pybind11::int_>();
-            if (!len(name) && format.kind() == 'V')
+            if ((len(name) == 0u) && format.kind() == 'V')
                 continue;
             field_descriptors.push_back({(PYBIND11_STR_TYPE) name, format.strip_padding(format.itemsize()), offset});
         }
@@ -872,11 +874,12 @@ template <typename T, int ExtraFlags = array::forcecast> class array_t : public
         : array(std::move(shape), std::move(strides), ptr, base) { }
 
     explicit array_t(ShapeContainer shape, const T *ptr = nullptr, handle base = handle())
-        : array_t(private_ctor{}, std::move(shape),
-                ExtraFlags & f_style
-                ? detail::f_strides(*shape, itemsize())
-                : detail::c_strides(*shape, itemsize()),
-                ptr, base) { }
+        : array_t(private_ctor{},
+                  std::move(shape),
+                  (ExtraFlags & f_style) != 0 ? detail::f_strides(*shape, itemsize())
+                                              : detail::c_strides(*shape, itemsize()),
+                  ptr,
+                  base) {}
 
     explicit array_t(ssize_t count, const T *ptr = nullptr, handle base = handle())
         : array({count}, {}, ptr, base) { }

include/pybind11/pybind11.h

@@ -306,7 +306,8 @@ class cpp_function : public function {
                 a.descr = guarded_strdup(repr(a.value).cast<std::string>().c_str());
         }
 
-        rec->is_constructor = !strcmp(rec->name, "__init__") || !strcmp(rec->name, "__setstate__");
+        rec->is_constructor
+            = (strcmp(rec->name, "__init__") == 0) || (strcmp(rec->name, "__setstate__") == 0);
 
 #if !defined(NDEBUG) && !defined(PYBIND11_DISABLE_NEW_STYLE_INIT_WARNING)
         if (rec->is_constructor && !rec->is_new_style_constructor) {
@@ -1120,7 +1121,8 @@ class generic_type : public object {
             pybind11_fail("generic_type: cannot initialize type \"" + std::string(rec.name) +
                           "\": an object with that name is already defined");
 
-        if (rec.module_local ? get_local_type_info(*rec.type) : get_global_type_info(*rec.type))
+        if ((rec.module_local ? get_local_type_info(*rec.type) : get_global_type_info(*rec.type))
+            != nullptr)
             pybind11_fail("generic_type: type \"" + std::string(rec.name) +
                           "\" is already registered!");
 
@@ -1198,8 +1200,9 @@ class generic_type : public object {
     void def_property_static_impl(const char *name,
                                   handle fget, handle fset,
                                   detail::function_record *rec_func) {
-        const auto is_static = rec_func && !(rec_func->is_method && rec_func->scope);
-        const auto has_doc = rec_func && rec_func->doc && pybind11::options::show_user_defined_docstrings();
+        const auto is_static = (rec_func != nullptr) && !(rec_func->is_method && rec_func->scope);
+        const auto has_doc = (rec_func != nullptr) && (rec_func->doc != nullptr)
+                             && pybind11::options::show_user_defined_docstrings();
         auto property = handle((PyObject *) (is_static ? get_internals().static_property_type
                                                        : &PyProperty_Type));
         attr(name) = property(fget.ptr() ? fget : none(),
@@ -2320,8 +2323,8 @@ inline function get_type_override(const void *this_ptr, const type_info *this_ty
        Unfortunately this doesn't work on PyPy. */
 #if !defined(PYPY_VERSION)
     PyFrameObject *frame = PyThreadState_Get()->frame;
-    if (frame && (std::string) str(frame->f_code->co_name) == name &&
-        frame->f_code->co_argcount > 0) {
+    if (frame != nullptr && (std::string) str(frame->f_code->co_name) == name
+        && frame->f_code->co_argcount > 0) {
         PyFrame_FastToLocals(frame);
         PyObject *self_caller = dict_getitem(
             frame->f_locals, PyTuple_GET_ITEM(frame->f_code->co_varnames, 0));

include/pybind11/pytypes.h

@@ -773,7 +773,11 @@ class dict_readonly {
     dict_readonly(handle obj, ssize_t pos) : obj(obj), pos(pos) { increment(); }
 
     reference dereference() const { return {key, value}; }
-    void increment() { if (!PyDict_Next(obj.ptr(), &pos, &key, &value)) { pos = -1; } }
+    void increment() {
+        if (PyDict_Next(obj.ptr(), &pos, &key, &value) == 0) {
+            pos = -1;
+        }
+    }
     bool equal(const dict_readonly &b) const { return pos == b.pos; }
 
 private:
@@ -1169,14 +1173,14 @@ class bool_ : public object {
     bool_() : object(Py_False, borrowed_t{}) { }
     // Allow implicit conversion from and to `bool`:
     bool_(bool value) : object(value ? Py_True : Py_False, borrowed_t{}) { }
-    operator bool() const { return m_ptr && PyLong_AsLong(m_ptr) != 0; }
+    operator bool() const { return (m_ptr != nullptr) && PyLong_AsLong(m_ptr) != 0; }
 
 private:
     /// Return the truth value of an object -- always returns a new reference
     static PyObject *raw_bool(PyObject *op) {
         const auto value = PyObject_IsTrue(op);
         if (value == -1) return nullptr;
-        return handle(value ? Py_True : Py_False).inc_ref().ptr();
+        return handle(value != 0 ? Py_True : Py_False).inc_ref().ptr();
     }
 };
 
@@ -1607,7 +1611,7 @@ inline memoryview memoryview::from_buffer(
     size_t ndim = shape->size();
     if (ndim != strides->size())
         pybind11_fail("memoryview: shape length doesn't match strides length");
-    ssize_t size = ndim ? 1 : 0;
+    ssize_t size = ndim != 0u ? 1 : 0;
     for (size_t i = 0; i < ndim; ++i)
         size *= (*shape)[i];
     Py_buffer view;

include/pybind11/stl/filesystem.h

@@ -67,15 +67,15 @@ template<typename T> struct path_caster {
         }
         PyObject* native = nullptr;
         if constexpr (std::is_same_v<typename T::value_type, char>) {
-            if (PyUnicode_FSConverter(buf, &native)) {
+            if (PyUnicode_FSConverter(buf, &native) != 0) {
                 if (auto c_str = PyBytes_AsString(native)) {
                     // AsString returns a pointer to the internal buffer, which
                     // must not be free'd.
                     value = c_str;
                 }
             }
         } else if constexpr (std::is_same_v<typename T::value_type, wchar_t>) {
-            if (PyUnicode_FSDecoder(buf, &native)) {
+            if (PyUnicode_FSDecoder(buf, &native) != 0) {
                 if (auto c_str = PyUnicode_AsWideCharString(native, nullptr)) {
                     // AsWideCharString returns a new string that must be free'd.
                     value = c_str;  // Copies the string.

tests/extra_python_package/test_files.py

@@ -146,6 +146,16 @@ def test_build_sdist(monkeypatch, tmpdir):
         ) as f:
             pyproject_toml = f.read()
 
+        with contextlib.closing(
+            tar.extractfile(
+                tar.getmember(
+                    start + "pybind11/share/cmake/pybind11/pybind11Config.cmake"
+                )
+            )
+        ) as f:
+            contents = f.read().decode("utf8")
+        assert 'set(pybind11_INCLUDE_DIR "${PACKAGE_PREFIX_DIR}/include")' in contents
+
     files = {"pybind11/{}".format(n) for n in all_files}
     files |= sdist_files
     files |= {"pybind11{}".format(n) for n in local_sdist_files}
@@ -159,11 +169,11 @@ def test_build_sdist(monkeypatch, tmpdir):
             .substitute(version=version, extra_cmd="")
             .encode()
         )
-        assert setup_py == contents
+    assert setup_py == contents
 
     with open(os.path.join(MAIN_DIR, "tools", "pyproject.toml"), "rb") as f:
         contents = f.read()
-        assert pyproject_toml == contents
+    assert pyproject_toml == contents
 
 
 def test_build_global_dist(monkeypatch, tmpdir):

tests/test_class_sh_basic.cpp

@@ -43,7 +43,7 @@ std::string pass_mref(atyp& obj)       { return "pass_mref:" + obj.mtxt; }
 std::string pass_cptr(atyp const* obj) { return "pass_cptr:" + obj->mtxt; }
 std::string pass_mptr(atyp* obj)       { return "pass_mptr:" + obj->mtxt; }
 
-std::shared_ptr<atyp>       rtrn_shmp() { return std::shared_ptr<atyp      >(new atyp{"rtrn_shmp"}); }
+std::shared_ptr<atyp>       rtrn_shmp() { return std::make_shared<atyp>("rtrn_shmp"); }
 std::shared_ptr<atyp const> rtrn_shcp() { return std::shared_ptr<atyp const>(new atyp{"rtrn_shcp"}); }
 
 std::string pass_shmp(std::shared_ptr<atyp>       obj) { return "pass_shmp:" + obj->mtxt; } // NOLINT

tests/test_class_sh_factory_constructors.cpp

@@ -40,7 +40,7 @@ atyp_mref&       rtrn_mref() { static atyp_mref obj; obj.mtxt = "Mref"; return o
 atyp_cptr const* rtrn_cptr() { return new atyp_cptr{"Cptr"}; }
 atyp_mptr*       rtrn_mptr() { return new atyp_mptr{"Mptr"}; }
 
-std::shared_ptr<atyp_shmp>       rtrn_shmp() { return std::shared_ptr<atyp_shmp      >(new atyp_shmp{"Shmp"}); }
+std::shared_ptr<atyp_shmp>       rtrn_shmp() { return std::make_shared<atyp_shmp>(atyp_shmp{"Shmp"}); }
 std::shared_ptr<atyp_shcp const> rtrn_shcp() { return std::shared_ptr<atyp_shcp const>(new atyp_shcp{"Shcp"}); }
 
 std::unique_ptr<atyp_uqmp>       rtrn_uqmp() { return std::unique_ptr<atyp_uqmp      >(new atyp_uqmp{"Uqmp"}); }
@@ -164,7 +164,7 @@ TEST_SUBMODULE(class_sh_factory_constructors, m) {
             return p;
         }))
         .def(py::init([](int i, int j, int k) {
-            auto p = std::shared_ptr<with_alias_alias>(new with_alias_alias);
+            auto p = std::make_shared<with_alias_alias>();
             p->val = i * 100 + j * 10 + k;
             return p;
         }))