From 2d468697d9c1a6209c2ccf13f3b57ba548230655 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Mon, 12 Jul 2021 13:10:28 -0700 Subject: [PATCH] NOLINT reduction (#3096) * Copying from prework_no_rst branch (PR #3087): test_numpy_array.cpp, test_stl.cpp * Manual changes reducing NOLINTs. * clang-format-diff.py * Minor adjustment to avoid MSVC warning C4702: unreachable code --- tests/pybind11_cross_module_tests.cpp | 3 +- tests/test_buffers.cpp | 3 +- tests/test_builtin_casters.cpp | 15 ++-- tests/test_class.cpp | 9 +- tests/test_copy_move.cpp | 4 +- tests/test_eigen.cpp | 52 +++++------ tests/test_exceptions.cpp | 9 +- tests/test_kwargs_and_defaults.cpp | 10 +-- tests/test_methods_and_attributes.cpp | 13 ++- tests/test_multiple_inheritance.cpp | 3 +- tests/test_numpy_array.cpp | 122 ++++++++++---------------- tests/test_numpy_vectorize.cpp | 24 +++-- tests/test_pytypes.cpp | 60 +++++-------- tests/test_stl.cpp | 14 ++- 14 files changed, 138 insertions(+), 203 deletions(-) diff --git a/tests/pybind11_cross_module_tests.cpp b/tests/pybind11_cross_module_tests.cpp index 27a349d23..e79701c6c 100644 --- a/tests/pybind11_cross_module_tests.cpp +++ b/tests/pybind11_cross_module_tests.cpp @@ -132,7 +132,6 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) { // test_missing_header_message // The main module already includes stl.h, but we need to test the error message // which appears when this header is missing. - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("missing_header_arg", [](std::vector) { }); + m.def("missing_header_arg", [](const std::vector &) {}); m.def("missing_header_return", []() { return std::vector(); }); } diff --git a/tests/test_buffers.cpp b/tests/test_buffers.cpp index 9902c1084..15dd4f1c7 100644 --- a/tests/test_buffers.cpp +++ b/tests/test_buffers.cpp @@ -79,8 +79,7 @@ TEST_SUBMODULE(buffers, m) { py::class_(m, "Matrix", py::buffer_protocol()) .def(py::init()) /// Construct from a buffer - // NOLINTNEXTLINE(performance-unnecessary-value-param) - .def(py::init([](py::buffer const b) { + .def(py::init([](const py::buffer &b) { py::buffer_info info = b.request(); if (info.format != py::format_descriptor::format() || info.ndim != 2) throw std::runtime_error("Incompatible buffer format!"); diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index caafb0631..51e410cad 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -105,8 +105,7 @@ TEST_SUBMODULE(builtin_casters, m) { // test_bytes_to_string m.def("strlen", [](char *s) { return strlen(s); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("string_length", [](std::string s) { return s.length(); }); + m.def("string_length", [](const std::string &s) { return s.length(); }); #ifdef PYBIND11_HAS_U8STRING m.attr("has_u8string") = true; @@ -185,14 +184,11 @@ TEST_SUBMODULE(builtin_casters, m) { // test_none_deferred m.def("defer_none_cstring", [](char *) { return false; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("defer_none_cstring", [](py::none) { return true; }); + m.def("defer_none_cstring", [](const py::none &) { return true; }); m.def("defer_none_custom", [](UserType *) { return false; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("defer_none_custom", [](py::none) { return true; }); + m.def("defer_none_custom", [](const py::none &) { return true; }); m.def("nodefer_none_void", [](void *) { return true; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("nodefer_none_void", [](py::none) { return false; }); + m.def("nodefer_none_void", [](const py::none &) { return false; }); // test_void_caster m.def("load_nullptr_t", [](std::nullptr_t) {}); // not useful, but it should still compile @@ -242,8 +238,7 @@ TEST_SUBMODULE(builtin_casters, m) { }, "copy"_a); m.def("refwrap_iiw", [](const IncType &w) { return w.value(); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("refwrap_call_iiw", [](IncType &w, py::function f) { + m.def("refwrap_call_iiw", [](IncType &w, const py::function &f) { py::list l; l.append(f(std::ref(w))); l.append(f(std::cref(w))); diff --git a/tests/test_class.cpp b/tests/test_class.cpp index bcc73ef0b..73de2a61a 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -131,8 +131,7 @@ TEST_SUBMODULE(class_, m) { m.def("return_none", []() -> BaseClass* { return nullptr; }); // test_isinstance - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("check_instances", [](py::list l) { + m.def("check_instances", [](const py::list &l) { return py::make_tuple( py::isinstance(l[0]), py::isinstance(l[1]), @@ -217,8 +216,7 @@ TEST_SUBMODULE(class_, m) { py::implicitly_convertible(); m.def("implicitly_convert_argument", [](const ConvertibleFromUserType &r) { return r.i; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("implicitly_convert_variable", [](py::object o) { + m.def("implicitly_convert_variable", [](const py::object &o) { // `o` is `UserType` and `r` is a reference to a temporary created by implicit // conversion. This is valid when called inside a bound function because the temp // object is attached to the same life support system as the arguments. @@ -396,8 +394,7 @@ TEST_SUBMODULE(class_, m) { struct StringWrapper { std::string str; }; m.def("test_error_after_conversions", [](int) {}); m.def("test_error_after_conversions", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](StringWrapper) -> NotRegistered { return {}; }); + [](const StringWrapper &) -> NotRegistered { return {}; }); py::class_(m, "StringWrapper").def(py::init()); py::implicitly_convertible(); diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp index 05caf28ff..859da9df7 100644 --- a/tests/test_copy_move.cpp +++ b/tests/test_copy_move.cpp @@ -126,7 +126,7 @@ TEST_SUBMODULE(copy_move_policies, m) { // test_move_and_copy_casts // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("move_and_copy_casts", [](py::object o) { + m.def("move_and_copy_casts", [](const py::object &o) { int r = 0; r += py::cast(o).value; /* moves */ r += py::cast(o).value; /* moves */ @@ -141,8 +141,10 @@ TEST_SUBMODULE(copy_move_policies, m) { // test_move_and_copy_loads m.def("move_only", [](MoveOnlyInt m) { return m.value; }); + // Changing this breaks the existing test: needs careful review. // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("move_or_copy", [](MoveOrCopyInt m) { return m.value; }); + // Changing this breaks the existing test: needs careful review. // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("copy_only", [](CopyOnlyInt m) { return m.value; }); m.def("move_pair", [](std::pair p) { diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index a24bf273d..ba74f5815 100644 --- a/tests/test_eigen.cpp +++ b/tests/test_eigen.cpp @@ -98,12 +98,13 @@ TEST_SUBMODULE(eigen, m) { // test_eigen_ref_to_python // Different ways of passing via Eigen::Ref; the first and second are the Eigen-recommended - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("cholesky1", [](Eigen::Ref x) -> Eigen::MatrixXd { return x.llt().matrixL(); }); + m.def("cholesky1", + [](const Eigen::Ref &x) -> Eigen::MatrixXd { return x.llt().matrixL(); }); m.def("cholesky2", [](const Eigen::Ref &x) -> Eigen::MatrixXd { return x.llt().matrixL(); }); m.def("cholesky3", [](const Eigen::Ref &x) -> Eigen::MatrixXd { return x.llt().matrixL(); }); - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("cholesky4", [](Eigen::Ref x) -> Eigen::MatrixXd { return x.llt().matrixL(); }); + m.def("cholesky4", [](const Eigen::Ref &x) -> Eigen::MatrixXd { + return x.llt().matrixL(); + }); // test_eigen_ref_mutators // Mutators: these add some value to the given element using Eigen, but Eigen should be mapping into @@ -248,12 +249,9 @@ TEST_SUBMODULE(eigen, m) { m.def("fixed_copy_r", [](const FixedMatrixR &m) -> FixedMatrixR { return m; }); m.def("fixed_copy_c", [](const FixedMatrixC &m) -> FixedMatrixC { return m; }); // test_mutator_descriptors - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("fixed_mutator_r", [](Eigen::Ref) {}); - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("fixed_mutator_c", [](Eigen::Ref) {}); - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("fixed_mutator_a", [](py::EigenDRef) {}); + m.def("fixed_mutator_r", [](const Eigen::Ref &) {}); + m.def("fixed_mutator_c", [](const Eigen::Ref &) {}); + m.def("fixed_mutator_a", [](const py::EigenDRef &) {}); // test_dense m.def("dense_r", [mat]() -> DenseMatrixR { return DenseMatrixR(mat); }); m.def("dense_c", [mat]() -> DenseMatrixC { return DenseMatrixC(mat); }); @@ -284,9 +282,10 @@ TEST_SUBMODULE(eigen, m) { // that would allow copying (if types or strides don't match) for comparison: m.def("get_elem", &get_elem); // Now this alternative that calls the tells pybind to fail rather than copy: - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("get_elem_nocopy", [](Eigen::Ref m) -> double { return get_elem(m); }, - py::arg{}.noconvert()); + m.def( + "get_elem_nocopy", + [](const Eigen::Ref &m) -> double { return get_elem(m); }, + py::arg{}.noconvert()); // Also test a row-major-only no-copy const ref: m.def("get_elem_rm_nocopy", [](Eigen::Ref> &m) -> long { return m(2, 1); }, py::arg{}.noconvert()); @@ -301,20 +300,22 @@ TEST_SUBMODULE(eigen, m) { // test_issue1105 // Issue #1105: when converting from a numpy two-dimensional (Nx1) or (1xN) value into a dense // eigen Vector or RowVector, the argument would fail to load because the numpy copy would - // fail: numpy won't broadcast a Nx1 into a 1-dimensional vector. NOLINTNEXTLINE - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("iss1105_col", [](Eigen::VectorXd) { return true; }); - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("iss1105_row", [](Eigen::RowVectorXd) { return true; }); + // fail: numpy won't broadcast a Nx1 into a 1-dimensional vector. + m.def("iss1105_col", [](const Eigen::VectorXd &) { return true; }); + m.def("iss1105_row", [](const Eigen::RowVectorXd &) { return true; }); // test_named_arguments // Make sure named arguments are working properly: - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("matrix_multiply", [](const py::EigenDRef A, const py::EigenDRef B) - -> Eigen::MatrixXd { - if (A.cols() != B.rows()) throw std::domain_error("Nonconformable matrices!"); - return A * B; - }, py::arg("A"), py::arg("B")); + m.def( + "matrix_multiply", + [](const py::EigenDRef &A, + const py::EigenDRef &B) -> Eigen::MatrixXd { + if (A.cols() != B.rows()) + throw std::domain_error("Nonconformable matrices!"); + return A * B; + }, + py::arg("A"), + py::arg("B")); // test_custom_operator_new py::class_(m, "CustomOperatorNew") @@ -326,8 +327,7 @@ TEST_SUBMODULE(eigen, m) { // In case of a failure (the caster's temp array does not live long enough), creating // a new array (np.ones(10)) increases the chances that the temp array will be garbage // collected and/or that its memory will be overridden with different values. - // NOLINTNEXTLINE (performance-unnecessary-value-param) - m.def("get_elem_direct", [](Eigen::Ref v) { + m.def("get_elem_direct", [](const Eigen::Ref &v) { py::module_::import("numpy").attr("ones")(10); return v(5); }); diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index d34f1a94e..f7bacd07e 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -201,17 +201,16 @@ TEST_SUBMODULE(exceptions, m) { throw py::error_already_set(); }); - // Changing this broke things. Don't know why - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("python_call_in_destructor", [](py::dict d) { + m.def("python_call_in_destructor", [](const py::dict &d) { + bool retval = false; try { PythonCallInDestructor set_dict_in_destructor(d); PyErr_SetString(PyExc_ValueError, "foo"); throw py::error_already_set(); } catch (const py::error_already_set&) { - return true; + retval = true; } - return false; + return retval; }); m.def("python_alreadyset_in_destructor", [](const py::str &s) { diff --git a/tests/test_kwargs_and_defaults.cpp b/tests/test_kwargs_and_defaults.cpp index 3e078bd9b..63332d32e 100644 --- a/tests/test_kwargs_and_defaults.cpp +++ b/tests/test_kwargs_and_defaults.cpp @@ -65,9 +65,10 @@ TEST_SUBMODULE(kwargs_and_defaults, m) { #endif m.def("arg_refcount_h", [](py::handle h) { GC_IF_NEEDED; return h.ref_count(); }); m.def("arg_refcount_h", [](py::handle h, py::handle, py::handle) { GC_IF_NEEDED; return h.ref_count(); }); - // TODO replace the following nolints as appropriate - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("arg_refcount_o", [](py::object o) { GC_IF_NEEDED; return o.ref_count(); }); + m.def("arg_refcount_o", [](const py::object &o) { + GC_IF_NEEDED; + return o.ref_count(); + }); m.def("args_refcount", [](py::args a) { GC_IF_NEEDED; py::tuple t(a.size()); @@ -76,8 +77,7 @@ TEST_SUBMODULE(kwargs_and_defaults, m) { t[i] = (int) Py_REFCNT(PyTuple_GET_ITEM(a.ptr(), static_cast(i))); return t; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("mixed_args_refcount", [](py::object o, py::args a) { + m.def("mixed_args_refcount", [](const py::object &o, py::args a) { GC_IF_NEEDED; py::tuple t(a.size() + 1); t[0] = o.ref_count(); diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp index 67ee117c6..6e5999f0c 100644 --- a/tests/test_methods_and_attributes.cpp +++ b/tests/test_methods_and_attributes.cpp @@ -294,20 +294,17 @@ TEST_SUBMODULE(methods_and_attributes, m) { "static_rw_func", py::cpp_function(static_get2, rvp_copy), static_set2) // test_property_rvalue_policy .def_property_readonly("rvalue", &TestPropRVP::get_rvalue) - // NOLINTNEXTLINE(performance-unnecessary-value-param) - .def_property_readonly_static("static_rvalue", [](py::object) { return UserType(1); }); + .def_property_readonly_static("static_rvalue", + [](const py::object &) { return UserType(1); }); // test_metaclass_override struct MetaclassOverride { }; py::class_(m, "MetaclassOverride", py::metaclass((PyObject *) &PyType_Type)) - // NOLINTNEXTLINE(performance-unnecessary-value-param) - .def_property_readonly_static("readonly", [](py::object) { return 1; }); + .def_property_readonly_static("readonly", [](const py::object &) { return 1; }); // test_overload_ordering - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("overload_order", [](std::string) { return 1; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("overload_order", [](std::string) { return 2; }); + m.def("overload_order", [](const std::string &) { return 1; }); + m.def("overload_order", [](const std::string &) { return 2; }); m.def("overload_order", [](int) { return 3; }); m.def("overload_order", [](int) { return 4; }, py::prepend{}); diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index f4acf7861..d6b24d34d 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -141,8 +141,7 @@ TEST_SUBMODULE(multiple_inheritance, m) { .def(py::init()); m.def("bar_base2a", [](Base2a *b) { return b->bar(); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("bar_base2a_sharedptr", [](std::shared_ptr b) { return b->bar(); }); + m.def("bar_base2a_sharedptr", [](const std::shared_ptr &b) { return b->bar(); }); // test_mi_unaligned_base // test_mi_base_return diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index a6c7ae8d7..5c22a3d25 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -226,8 +226,7 @@ TEST_SUBMODULE(numpy_array, sm) { return py::isinstance(std::move(yes)) && !py::isinstance(std::move(no)); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("isinstance_typed", [](py::object o) { + sm.def("isinstance_typed", [](const py::object &o) { return py::isinstance>(o) && !py::isinstance>(o); }); @@ -248,60 +247,47 @@ TEST_SUBMODULE(numpy_array, sm) { }); // test_overload_resolution - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded", [](py::array_t) { return "double"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded", [](py::array_t) { return "float"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded", [](py::array_t) { return "int"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded", [](py::array_t) { return "unsigned short"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded", [](py::array_t) { return "long long"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded", [](py::array_t>) { return "double complex"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded", [](py::array_t>) { return "float complex"; }); + sm.def("overloaded", [](const py::array_t &) { return "double"; }); + sm.def("overloaded", [](const py::array_t &) { return "float"; }); + sm.def("overloaded", [](const py::array_t &) { return "int"; }); + sm.def("overloaded", [](const py::array_t &) { return "unsigned short"; }); + sm.def("overloaded", [](const py::array_t &) { return "long long"; }); + sm.def("overloaded", + [](const py::array_t> &) { return "double complex"; }); + sm.def("overloaded", [](const py::array_t> &) { return "float complex"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded2", [](py::array_t>) { return "double complex"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded2", [](py::array_t) { return "double"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded2", [](py::array_t>) { return "float complex"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded2", [](py::array_t) { return "float"; }); + sm.def("overloaded2", + [](const py::array_t> &) { return "double complex"; }); + sm.def("overloaded2", [](const py::array_t &) { return "double"; }); + sm.def("overloaded2", + [](const py::array_t> &) { return "float complex"; }); + sm.def("overloaded2", [](const py::array_t &) { return "float"; }); // [workaround(intel)] ICC 20/21 breaks with py::arg().stuff, using py::arg{}.stuff works. // Only accept the exact types: - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded3", [](py::array_t) { return "int"; }, py::arg{}.noconvert()); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded3", [](py::array_t) { return "double"; }, py::arg{}.noconvert()); + sm.def( + "overloaded3", [](const py::array_t &) { return "int"; }, py::arg{}.noconvert()); + sm.def( + "overloaded3", + [](const py::array_t &) { return "double"; }, + py::arg{}.noconvert()); // Make sure we don't do unsafe coercion (e.g. float to int) when not using forcecast, but // rather that float gets converted via the safe (conversion to double) overload: - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded4", [](py::array_t) { return "long long"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded4", [](py::array_t) { return "double"; }); + sm.def("overloaded4", [](const py::array_t &) { return "long long"; }); + sm.def("overloaded4", [](const py::array_t &) { return "double"; }); // But we do allow conversion to int if forcecast is enabled (but only if no overload matches // without conversion) - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded5", [](py::array_t) { return "unsigned int"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("overloaded5", [](py::array_t) { return "double"; }); + sm.def("overloaded5", [](const py::array_t &) { return "unsigned int"; }); + sm.def("overloaded5", [](const py::array_t &) { return "double"; }); // test_greedy_string_overload // Issue 685: ndarray shouldn't go to std::string overload - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("issue685", [](std::string) { return "string"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("issue685", [](py::array) { return "array"; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("issue685", [](py::object) { return "other"; }); + sm.def("issue685", [](const std::string &) { return "string"; }); + sm.def("issue685", [](const py::array &) { return "array"; }); + sm.def("issue685", [](const py::object &) { return "other"; }); // test_array_unchecked_fixed_dims sm.def("proxy_add2", [](py::array_t a, double v) { @@ -424,73 +410,53 @@ TEST_SUBMODULE(numpy_array, sm) { // test_argument_conversions sm.def( - "accept_double", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, - py::arg("a")); + "accept_double", [](const py::array_t &) {}, py::arg("a")); sm.def( "accept_double_forcecast", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, py::arg("a")); sm.def( "accept_double_c_style", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, py::arg("a")); sm.def( "accept_double_c_style_forcecast", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, py::arg("a")); sm.def( "accept_double_f_style", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, py::arg("a")); sm.def( "accept_double_f_style_forcecast", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, py::arg("a")); sm.def( - "accept_double_noconvert", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, - "a"_a.noconvert()); + "accept_double_noconvert", [](const py::array_t &) {}, "a"_a.noconvert()); sm.def( "accept_double_forcecast_noconvert", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, "a"_a.noconvert()); sm.def( "accept_double_c_style_noconvert", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, "a"_a.noconvert()); sm.def( "accept_double_c_style_forcecast_noconvert", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, "a"_a.noconvert()); sm.def( "accept_double_f_style_noconvert", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, "a"_a.noconvert()); sm.def( "accept_double_f_style_forcecast_noconvert", - // NOLINTNEXTLINE(performance-unnecessary-value-param) - [](py::array_t) {}, + [](const py::array_t &) {}, "a"_a.noconvert()); // Check that types returns correct npy format descriptor - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("test_fmt_desc_float", [](py::array_t) {}); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("test_fmt_desc_double", [](py::array_t) {}); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("test_fmt_desc_const_float", [](py::array_t) {}); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - sm.def("test_fmt_desc_const_double", [](py::array_t) {}); + sm.def("test_fmt_desc_float", [](const py::array_t &) {}); + sm.def("test_fmt_desc_double", [](const py::array_t &) {}); + sm.def("test_fmt_desc_const_float", [](const py::array_t &) {}); + sm.def("test_fmt_desc_const_double", [](const py::array_t &) {}); } diff --git a/tests/test_numpy_vectorize.cpp b/tests/test_numpy_vectorize.cpp index 0be30d8e8..42fb2002e 100644 --- a/tests/test_numpy_vectorize.cpp +++ b/tests/test_numpy_vectorize.cpp @@ -40,13 +40,13 @@ TEST_SUBMODULE(numpy_vectorize, m) { // test_type_selection // NumPy function which only accepts specific data types // A lot of these no lints could be replaced with const refs, and probably should at some point. - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("selective_func", [](py::array_t) { return "Int branch taken."; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("selective_func", [](py::array_t) { return "Float branch taken."; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("selective_func", [](py::array_t, py::array::c_style>) { return "Complex float branch taken."; }); - + m.def("selective_func", + [](const py::array_t &) { return "Int branch taken."; }); + m.def("selective_func", + [](const py::array_t &) { return "Float branch taken."; }); + m.def("selective_func", [](const py::array_t, py::array::c_style> &) { + return "Complex float branch taken."; + }); // test_passthrough_arguments // Passthrough test: references and non-pod types should be automatically passed through (in the @@ -89,13 +89,9 @@ TEST_SUBMODULE(numpy_vectorize, m) { .value("c_trivial", py::detail::broadcast_trivial::c_trivial) .value("non_trivial", py::detail::broadcast_trivial::non_trivial); m.def("vectorized_is_trivial", - []( - // NOLINTNEXTLINE(performance-unnecessary-value-param) - py::array_t arg1, - // NOLINTNEXTLINE(performance-unnecessary-value-param) - py::array_t arg2, - // NOLINTNEXTLINE(performance-unnecessary-value-param) - py::array_t arg3) { + [](const py::array_t &arg1, + const py::array_t &arg2, + const py::array_t &arg3) { py::ssize_t ndim; std::vector shape; std::array buffers{ diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 2b91af37b..6ed59aad2 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -237,8 +237,7 @@ TEST_SUBMODULE(pytypes, m) { ); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("cast_functions", [](py::dict d) { + m.def("cast_functions", [](const py::dict &d) { // When converting between Python types, obj.cast() should be the same as T(obj) return py::dict( "bytes"_a=d["bytes"].cast(), @@ -255,25 +254,24 @@ TEST_SUBMODULE(pytypes, m) { ); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("convert_to_pybind11_str", [](py::object o) { return py::str(o); }); + m.def("convert_to_pybind11_str", [](const py::object &o) { return py::str(o); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("nonconverting_constructor", [](std::string type, py::object value, bool move) -> py::object { - if (type == "bytes") { - return move ? py::bytes(std::move(value)) : py::bytes(value); - } - if (type == "none") { - return move ? py::none(std::move(value)) : py::none(value); - } - if (type == "ellipsis") { - return move ? py::ellipsis(std::move(value)) : py::ellipsis(value); - } - if (type == "type") { - return move ? py::type(std::move(value)) : py::type(value); - } - throw std::runtime_error("Invalid type"); - }); + m.def("nonconverting_constructor", + [](const std::string &type, py::object value, bool move) -> py::object { + if (type == "bytes") { + return move ? py::bytes(std::move(value)) : py::bytes(value); + } + if (type == "none") { + return move ? py::none(std::move(value)) : py::none(value); + } + if (type == "ellipsis") { + return move ? py::ellipsis(std::move(value)) : py::ellipsis(value); + } + if (type == "type") { + return move ? py::type(std::move(value)) : py::type(value); + } + throw std::runtime_error("Invalid type"); + }); m.def("get_implicit_casting", []() { py::dict d; @@ -333,8 +331,7 @@ TEST_SUBMODULE(pytypes, m) { m.def("hash_function", [](py::object obj) { return py::hash(std::move(obj)); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("test_number_protocol", [](py::object a, py::object b) { + m.def("test_number_protocol", [](const py::object &a, const py::object &b) { py::list l; l.append(a.equal(b)); l.append(a.not_equal(b)); @@ -354,10 +351,7 @@ TEST_SUBMODULE(pytypes, m) { return l; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("test_list_slicing", [](py::list a) { - return a[py::slice(0, -1, 2)]; - }); + m.def("test_list_slicing", [](const py::list &a) { return a[py::slice(0, -1, 2)]; }); // See #2361 m.def("issue2361_str_implicit_copy_none", []() { @@ -369,15 +363,10 @@ TEST_SUBMODULE(pytypes, m) { return is_this_none; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("test_memoryview_object", [](py::buffer b) { - return py::memoryview(b); - }); + m.def("test_memoryview_object", [](const py::buffer &b) { return py::memoryview(b); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("test_memoryview_buffer_info", [](py::buffer b) { - return py::memoryview(b.request()); - }); + m.def("test_memoryview_buffer_info", + [](const py::buffer &b) { return py::memoryview(b.request()); }); m.def("test_memoryview_from_buffer", [](bool is_unsigned) { static const int16_t si16[] = { 3, 1, 4, 1, 5 }; @@ -432,8 +421,7 @@ TEST_SUBMODULE(pytypes, m) { m.def("pass_to_pybind11_bytes", [](py::bytes b) { return py::len(std::move(b)); }); m.def("pass_to_pybind11_str", [](py::str s) { return py::len(std::move(s)); }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("pass_to_std_string", [](std::string s) { return s.size(); }); + m.def("pass_to_std_string", [](const std::string &s) { return s.size(); }); // test_weakref m.def("weakref_from_handle", diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index dc75762e8..1f308e139 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -207,8 +207,7 @@ TEST_SUBMODULE(stl, m) { }, py::arg_v("x", std::nullopt, "None")); m.def("nodefer_none_optional", [](std::optional) { return true; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("nodefer_none_optional", [](py::none) { return false; }); + m.def("nodefer_none_optional", [](const py::none &) { return false; }); using opt_holder = OptionalHolder; py::class_(m, "OptionalHolder", "Class with optional member") @@ -299,12 +298,11 @@ TEST_SUBMODULE(stl, m) { m.def("stl_pass_by_pointer", [](std::vector* v) { return *v; }, "v"_a=nullptr); // #1258: pybind11/stl.h converts string to vector - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("func_with_string_or_vector_string_arg_overload", [](std::vector) { return 1; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("func_with_string_or_vector_string_arg_overload", [](std::list) { return 2; }); - // NOLINTNEXTLINE(performance-unnecessary-value-param) - m.def("func_with_string_or_vector_string_arg_overload", [](std::string) { return 3; }); + m.def("func_with_string_or_vector_string_arg_overload", + [](const std::vector &) { return 1; }); + m.def("func_with_string_or_vector_string_arg_overload", + [](const std::list &) { return 2; }); + m.def("func_with_string_or_vector_string_arg_overload", [](const std::string &) { return 3; }); class Placeholder { public: