diff --git a/.clang-tidy b/.clang-tidy index 72304528a5..db5077c227 100644 --- a/.clang-tidy +++ b/.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' diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 1da432358f..898047b304 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -181,7 +181,7 @@ struct type_caster::value && !is_std_char_t // Signed/unsigned checks happen elsewhere if (py_err || (std::is_integral::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(std::is_floating_point::value ? PyNumber_Float(src.ptr()) : PyNumber_Long(src.ptr())); @@ -300,7 +300,7 @@ template <> class type_caster { 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; @@ -501,10 +501,14 @@ template struct type_caster 1 && str_len <= 4) { auto v0 = static_cast(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: diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h index 15e09165de..f9822c7b35 100644 --- a/include/pybind11/detail/class.h +++ b/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) @@ -562,7 +563,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(info->readonly); if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT) view->format = const_cast(info->format.c_str()); if ((flags & PyBUF_STRIDES) == PyBUF_STRIDES) { diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index a621aed2a6..a93a83e268 100644 --- a/include/pybind11/detail/internals.h +++ b/include/pybind11/detail/internals.h @@ -297,7 +297,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 diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index a6b6bea6a3..2a675418a7 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/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) diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index 204aaf989f..abb1cd3cca 100644 --- a/include/pybind11/embed.h +++ b/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); diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index edba8bac92..7313897fe1 100644 --- a/include/pybind11/numpy.h +++ b/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(ptr); } @@ -542,7 +544,7 @@ class dtype : public object { auto name = spec[0].cast(); auto format = spec[1].cast()[0].cast(); auto offset = spec[1].cast()[1].cast(); - 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 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) { } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 93b920d852..e2cbb54493 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -318,7 +318,8 @@ class cpp_function : public function { a.descr = guarded_strdup(repr(a.value).cast().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) { @@ -1131,7 +1132,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!"); @@ -1209,8 +1211,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(), @@ -2220,8 +2223,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)); diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index cea4e7eb01..161aed06f4 100644 --- a/include/pybind11/pytypes.h +++ b/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; diff --git a/include/pybind11/stl/filesystem.h b/include/pybind11/stl/filesystem.h index 7a8acdb60b..431b94b4f7 100644 --- a/include/pybind11/stl/filesystem.h +++ b/include/pybind11/stl/filesystem.h @@ -67,7 +67,7 @@ template struct path_caster { } PyObject* native = nullptr; if constexpr (std::is_same_v) { - 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. @@ -75,7 +75,7 @@ template struct path_caster { } } } else if constexpr (std::is_same_v) { - 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. diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 6925ee9782..cd50e952f6 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -103,7 +103,7 @@ bool has_pybind11_internals_builtin() { bool has_pybind11_internals_static() { auto **&ipp = py::detail::get_internals_pp(); - return ipp && *ipp; + return (ipp != nullptr) && (*ipp != nullptr); } TEST_CASE("Restart the interpreter") { diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp index 9db31a0b82..4cf6f08b85 100644 --- a/tests/test_methods_and_attributes.cpp +++ b/tests/test_methods_and_attributes.cpp @@ -43,6 +43,7 @@ class ExampleMandA { void add6(int other) { value += other; } // passing by value void add7(int &other) { value += other; } // passing by reference void add8(const int &other) { value += other; } // passing by const reference + // NOLINTNEXTLINE(readability-non-const-parameter) Deliberately non-const for testing void add9(int *other) { value += *other; } // passing by pointer void add10(const int *other) { value += *other; } // passing by const pointer diff --git a/tests/test_numpy_dtypes.cpp b/tests/test_numpy_dtypes.cpp index edf9cf200f..340c972db2 100644 --- a/tests/test_numpy_dtypes.cpp +++ b/tests/test_numpy_dtypes.cpp @@ -108,9 +108,11 @@ PYBIND11_PACKED(struct EnumStruct { std::ostream& operator<<(std::ostream& os, const StringStruct& v) { os << "a='"; - for (size_t i = 0; i < 3 && v.a[i]; i++) os << v.a[i]; + for (size_t i = 0; i < 3 && (v.a[i] != 0); i++) + os << v.a[i]; os << "',b='"; - for (size_t i = 0; i < 3 && v.b[i]; i++) os << v.b[i]; + for (size_t i = 0; i < 3 && (v.b[i] != 0); i++) + os << v.b[i]; return os << "'"; } diff --git a/tests/test_numpy_vectorize.cpp b/tests/test_numpy_vectorize.cpp index ed08a42bef..b08a9f7edd 100644 --- a/tests/test_numpy_vectorize.cpp +++ b/tests/test_numpy_vectorize.cpp @@ -59,7 +59,7 @@ TEST_SUBMODULE(numpy_vectorize, m) { .def(py::init()) .def_readwrite("value", &NonPODClass::value); m.def("vec_passthrough", - py::vectorize([](double *a, + py::vectorize([](const double *a, double b, // Changing this broke things // NOLINTNEXTLINE(performance-unnecessary-value-param) diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 7fd5a9b36e..57b2d894e2 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -101,7 +101,7 @@ class MyObject3 : public std::enable_shared_from_this { // test_unique_nodelete // Object with a private destructor class MyObject4; -static std::unordered_set myobject4_instances; +std::unordered_set myobject4_instances; class MyObject4 { public: MyObject4(int value) : value{value} { @@ -127,7 +127,7 @@ class MyObject4 { // Object with std::unique_ptr where D is not matching the base class // Object with a protected destructor class MyObject4a; -static std::unordered_set myobject4a_instances; +std::unordered_set myobject4a_instances; class MyObject4a { public: MyObject4a(int i) { diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index eb8cdab7b5..23e2c07b32 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -102,7 +102,7 @@ TEST_SUBMODULE(stl, m) { // test_set m.def("cast_set", []() { return std::set{"key1", "key2"}; }); m.def("load_set", [](const std::set &set) { - return set.count("key1") && set.count("key2") && set.count("key3"); + return (set.count("key1") != 0u) && (set.count("key2") != 0u) && (set.count("key3") != 0u); }); // test_recursive_casting @@ -196,9 +196,7 @@ TEST_SUBMODULE(stl, m) { m.def("double_or_zero", [](const opt_int& x) -> int { return x.value_or(0) * 2; }); - m.def("half_or_none", [](int x) -> opt_int { - return x ? opt_int(x / 2) : opt_int(); - }); + m.def("half_or_none", [](int x) -> opt_int { return x != 0 ? opt_int(x / 2) : opt_int(); }); m.def("test_nullopt", [](opt_int x) { return x.value_or(42); }, py::arg_v("x", std::nullopt, "None"));