Skip to content

Commit 2d46869

Browse files
rwgkrwgk
authored
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
1 parent 7a64b8a commit 2d46869

14 files changed

+140
-205
lines changed

‎tests/pybind11_cross_module_tests.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
132132
// test_missing_header_message
133133
// The main module already includes stl.h, but we need to test the error message
134134
// which appears when this header is missing.
135-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
136-
m.def("missing_header_arg", [](std::vector<float>) { });
135+
m.def("missing_header_arg", [](const std::vector<float> &) {});
137136
m.def("missing_header_return", []() { return std::vector<float>(); });
138137
}

‎tests/test_buffers.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,7 @@ TEST_SUBMODULE(buffers, m) {
7979
py::class_<Matrix>(m, "Matrix", py::buffer_protocol())
8080
.def(py::init<py::ssize_t, py::ssize_t>())
8181
/// Construct from a buffer
82-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
83-
.def(py::init([](py::buffer const b) {
82+
.def(py::init([](const py::buffer &b) {
8483
py::buffer_info info = b.request();
8584
if (info.format != py::format_descriptor<float>::format() || info.ndim != 2)
8685
throw std::runtime_error("Incompatible buffer format!");

‎tests/test_builtin_casters.cpp

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,7 @@ TEST_SUBMODULE(builtin_casters, m) {
105105

106106
// test_bytes_to_string
107107
m.def("strlen", [](char *s) { return strlen(s); });
108-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
109-
m.def("string_length", [](std::string s) { return s.length(); });
108+
m.def("string_length", [](const std::string &s) { return s.length(); });
110109

111110
#ifdef PYBIND11_HAS_U8STRING
112111
m.attr("has_u8string") = true;
@@ -185,14 +184,11 @@ TEST_SUBMODULE(builtin_casters, m) {
185184

186185
// test_none_deferred
187186
m.def("defer_none_cstring", [](char *) { return false; });
188-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
189-
m.def("defer_none_cstring", [](py::none) { return true; });
187+
m.def("defer_none_cstring", [](const py::none &) { return true; });
190188
m.def("defer_none_custom", [](UserType *) { return false; });
191-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
192-
m.def("defer_none_custom", [](py::none) { return true; });
189+
m.def("defer_none_custom", [](const py::none &) { return true; });
193190
m.def("nodefer_none_void", [](void *) { return true; });
194-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
195-
m.def("nodefer_none_void", [](py::none) { return false; });
191+
m.def("nodefer_none_void", [](const py::none &) { return false; });
196192

197193
// test_void_caster
198194
m.def("load_nullptr_t", [](std::nullptr_t) {}); // not useful, but it should still compile
@@ -242,8 +238,7 @@ TEST_SUBMODULE(builtin_casters, m) {
242238
}, "copy"_a);
243239

244240
m.def("refwrap_iiw", [](const IncType &w) { return w.value(); });
245-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
246-
m.def("refwrap_call_iiw", [](IncType &w, py::function f) {
241+
m.def("refwrap_call_iiw", [](IncType &w, const py::function &f) {
247242
py::list l;
248243
l.append(f(std::ref(w)));
249244
l.append(f(std::cref(w)));

‎tests/test_class.cpp

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,7 @@ TEST_SUBMODULE(class_, m) {
131131
m.def("return_none", []() -> BaseClass* { return nullptr; });
132132

133133
// test_isinstance
134-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
135-
m.def("check_instances", [](py::list l) {
134+
m.def("check_instances", [](const py::list &l) {
136135
return py::make_tuple(
137136
py::isinstance<py::tuple>(l[0]),
138137
py::isinstance<py::dict>(l[1]),
@@ -217,8 +216,7 @@ TEST_SUBMODULE(class_, m) {
217216
py::implicitly_convertible<UserType, ConvertibleFromUserType>();
218217

219218
m.def("implicitly_convert_argument", [](const ConvertibleFromUserType &r) { return r.i; });
220-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
221-
m.def("implicitly_convert_variable", [](py::object o) {
219+
m.def("implicitly_convert_variable", [](const py::object &o) {
222220
// `o` is `UserType` and `r` is a reference to a temporary created by implicit
223221
// conversion. This is valid when called inside a bound function because the temp
224222
// object is attached to the same life support system as the arguments.
@@ -396,8 +394,7 @@ TEST_SUBMODULE(class_, m) {
396394
struct StringWrapper { std::string str; };
397395
m.def("test_error_after_conversions", [](int) {});
398396
m.def("test_error_after_conversions",
399-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
400-
[](StringWrapper) -> NotRegistered { return {}; });
397+
[](const StringWrapper &) -> NotRegistered { return {}; });
401398
py::class_<StringWrapper>(m, "StringWrapper").def(py::init<std::string>());
402399
py::implicitly_convertible<std::string, StringWrapper>();
403400

‎tests/test_copy_move.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ TEST_SUBMODULE(copy_move_policies, m) {
126126

127127
// test_move_and_copy_casts
128128
// NOLINTNEXTLINE(performance-unnecessary-value-param)
129-
m.def("move_and_copy_casts", [](py::object o) {
129+
m.def("move_and_copy_casts", [](const py::object &o) {
130130
int r = 0;
131131
r += py::cast<MoveOrCopyInt>(o).value; /* moves */
132132
r += py::cast<MoveOnlyInt>(o).value; /* moves */
@@ -141,8 +141,10 @@ TEST_SUBMODULE(copy_move_policies, m) {
141141

142142
// test_move_and_copy_loads
143143
m.def("move_only", [](MoveOnlyInt m) { return m.value; });
144+
// Changing this breaks the existing test: needs careful review.
144145
// NOLINTNEXTLINE(performance-unnecessary-value-param)
145146
m.def("move_or_copy", [](MoveOrCopyInt m) { return m.value; });
147+
// Changing this breaks the existing test: needs careful review.
146148
// NOLINTNEXTLINE(performance-unnecessary-value-param)
147149
m.def("copy_only", [](CopyOnlyInt m) { return m.value; });
148150
m.def("move_pair", [](std::pair<MoveOnlyInt, MoveOrCopyInt> p) {

‎tests/test_eigen.cpp

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,13 @@ TEST_SUBMODULE(eigen, m) {
9898

9999
// test_eigen_ref_to_python
100100
// Different ways of passing via Eigen::Ref; the first and second are the Eigen-recommended
101-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
102-
m.def("cholesky1", [](Eigen::Ref<MatrixXdR> x) -> Eigen::MatrixXd { return x.llt().matrixL(); });
101+
m.def("cholesky1",
102+
[](const Eigen::Ref<MatrixXdR> &x) -> Eigen::MatrixXd { return x.llt().matrixL(); });
103103
m.def("cholesky2", [](const Eigen::Ref<const MatrixXdR> &x) -> Eigen::MatrixXd { return x.llt().matrixL(); });
104104
m.def("cholesky3", [](const Eigen::Ref<MatrixXdR> &x) -> Eigen::MatrixXd { return x.llt().matrixL(); });
105-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
106-
m.def("cholesky4", [](Eigen::Ref<const MatrixXdR> x) -> Eigen::MatrixXd { return x.llt().matrixL(); });
105+
m.def("cholesky4", [](const Eigen::Ref<const MatrixXdR> &x) -> Eigen::MatrixXd {
106+
return x.llt().matrixL();
107+
});
107108

108109
// test_eigen_ref_mutators
109110
// 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) {
248249
m.def("fixed_copy_r", [](const FixedMatrixR &m) -> FixedMatrixR { return m; });
249250
m.def("fixed_copy_c", [](const FixedMatrixC &m) -> FixedMatrixC { return m; });
250251
// test_mutator_descriptors
251-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
252-
m.def("fixed_mutator_r", [](Eigen::Ref<FixedMatrixR>) {});
253-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
254-
m.def("fixed_mutator_c", [](Eigen::Ref<FixedMatrixC>) {});
255-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
256-
m.def("fixed_mutator_a", [](py::EigenDRef<FixedMatrixC>) {});
252+
m.def("fixed_mutator_r", [](const Eigen::Ref<FixedMatrixR> &) {});
253+
m.def("fixed_mutator_c", [](const Eigen::Ref<FixedMatrixC> &) {});
254+
m.def("fixed_mutator_a", [](const py::EigenDRef<FixedMatrixC> &) {});
257255
// test_dense
258256
m.def("dense_r", [mat]() -> DenseMatrixR { return DenseMatrixR(mat); });
259257
m.def("dense_c", [mat]() -> DenseMatrixC { return DenseMatrixC(mat); });
@@ -284,9 +282,10 @@ TEST_SUBMODULE(eigen, m) {
284282
// that would allow copying (if types or strides don't match) for comparison:
285283
m.def("get_elem", &get_elem);
286284
// Now this alternative that calls the tells pybind to fail rather than copy:
287-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
288-
m.def("get_elem_nocopy", [](Eigen::Ref<const Eigen::MatrixXd> m) -> double { return get_elem(m); },
289-
py::arg{}.noconvert());
285+
m.def(
286+
"get_elem_nocopy",
287+
[](const Eigen::Ref<const Eigen::MatrixXd> &m) -> double { return get_elem(m); },
288+
py::arg{}.noconvert());
290289
// Also test a row-major-only no-copy const ref:
291290
m.def("get_elem_rm_nocopy", [](Eigen::Ref<const Eigen::Matrix<long, -1, -1, Eigen::RowMajor>> &m) -> long { return m(2, 1); },
292291
py::arg{}.noconvert());
@@ -301,20 +300,22 @@ TEST_SUBMODULE(eigen, m) {
301300
// test_issue1105
302301
// Issue #1105: when converting from a numpy two-dimensional (Nx1) or (1xN) value into a dense
303302
// eigen Vector or RowVector, the argument would fail to load because the numpy copy would
304-
// fail: numpy won't broadcast a Nx1 into a 1-dimensional vector. NOLINTNEXTLINE
305-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
306-
m.def("iss1105_col", [](Eigen::VectorXd) { return true; });
307-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
308-
m.def("iss1105_row", [](Eigen::RowVectorXd) { return true; });
303+
// fail: numpy won't broadcast a Nx1 into a 1-dimensional vector.
304+
m.def("iss1105_col", [](const Eigen::VectorXd &) { return true; });
305+
m.def("iss1105_row", [](const Eigen::RowVectorXd &) { return true; });
309306

310307
// test_named_arguments
311308
// Make sure named arguments are working properly:
312-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
313-
m.def("matrix_multiply", [](const py::EigenDRef<const Eigen::MatrixXd> A, const py::EigenDRef<const Eigen::MatrixXd> B)
314-
-> Eigen::MatrixXd {
315-
if (A.cols() != B.rows()) throw std::domain_error("Nonconformable matrices!");
316-
return A * B;
317-
}, py::arg("A"), py::arg("B"));
309+
m.def(
310+
"matrix_multiply",
311+
[](const py::EigenDRef<const Eigen::MatrixXd> &A,
312+
const py::EigenDRef<const Eigen::MatrixXd> &B) -> Eigen::MatrixXd {
313+
if (A.cols() != B.rows())
314+
throw std::domain_error("Nonconformable matrices!");
315+
return A * B;
316+
},
317+
py::arg("A"),
318+
py::arg("B"));
318319

319320
// test_custom_operator_new
320321
py::class_<CustomOperatorNew>(m, "CustomOperatorNew")
@@ -326,8 +327,7 @@ TEST_SUBMODULE(eigen, m) {
326327
// In case of a failure (the caster's temp array does not live long enough), creating
327328
// a new array (np.ones(10)) increases the chances that the temp array will be garbage
328329
// collected and/or that its memory will be overridden with different values.
329-
// NOLINTNEXTLINE (performance-unnecessary-value-param)
330-
m.def("get_elem_direct", [](Eigen::Ref<const Eigen::VectorXd> v) {
330+
m.def("get_elem_direct", [](const Eigen::Ref<const Eigen::VectorXd> &v) {
331331
py::module_::import("numpy").attr("ones")(10);
332332
return v(5);
333333
});

‎tests/test_exceptions.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -201,17 +201,16 @@ TEST_SUBMODULE(exceptions, m) {
201201
throw py::error_already_set();
202202
});
203203

204-
// Changing this broke things. Don't know why
205-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
206-
m.def("python_call_in_destructor", [](py::dict d) {
204+
m.def("python_call_in_destructor", [](const py::dict &d) {
205+
bool retval = false;
207206
try {
208207
PythonCallInDestructor set_dict_in_destructor(d);
209208
PyErr_SetString(PyExc_ValueError, "foo");
210209
throw py::error_already_set();
211210
} catch (const py::error_already_set&) {
212-
return true;
211+
retval = true;
213212
}
214-
return false;
213+
return retval;
215214
});
216215

217216
m.def("python_alreadyset_in_destructor", [](const py::str &s) {

‎tests/test_kwargs_and_defaults.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -65,9 +65,10 @@ TEST_SUBMODULE(kwargs_and_defaults, m) {
6565
#endif
6666
m.def("arg_refcount_h", [](py::handle h) { GC_IF_NEEDED; return h.ref_count(); });
6767
m.def("arg_refcount_h", [](py::handle h, py::handle, py::handle) { GC_IF_NEEDED; return h.ref_count(); });
68-
// TODO replace the following nolints as appropriate
69-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
70-
m.def("arg_refcount_o", [](py::object o) { GC_IF_NEEDED; return o.ref_count(); });
68+
m.def("arg_refcount_o", [](const py::object &o) {
69+
GC_IF_NEEDED;
70+
return o.ref_count();
71+
});
7172
m.def("args_refcount", [](py::args a) {
7273
GC_IF_NEEDED;
7374
py::tuple t(a.size());
@@ -76,8 +77,7 @@ TEST_SUBMODULE(kwargs_and_defaults, m) {
7677
t[i] = (int) Py_REFCNT(PyTuple_GET_ITEM(a.ptr(), static_cast<py::ssize_t>(i)));
7778
return t;
7879
});
79-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
80-
m.def("mixed_args_refcount", [](py::object o, py::args a) {
80+
m.def("mixed_args_refcount", [](const py::object &o, py::args a) {
8181
GC_IF_NEEDED;
8282
py::tuple t(a.size() + 1);
8383
t[0] = o.ref_count();

‎tests/test_methods_and_attributes.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -294,20 +294,17 @@ TEST_SUBMODULE(methods_and_attributes, m) {
294294
"static_rw_func", py::cpp_function(static_get2, rvp_copy), static_set2)
295295
// test_property_rvalue_policy
296296
.def_property_readonly("rvalue", &TestPropRVP::get_rvalue)
297-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
298-
.def_property_readonly_static("static_rvalue", [](py::object) { return UserType(1); });
297+
.def_property_readonly_static("static_rvalue",
298+
[](const py::object &) { return UserType(1); });
299299

300300
// test_metaclass_override
301301
struct MetaclassOverride { };
302302
py::class_<MetaclassOverride>(m, "MetaclassOverride", py::metaclass((PyObject *) &PyType_Type))
303-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
304-
.def_property_readonly_static("readonly", [](py::object) { return 1; });
303+
.def_property_readonly_static("readonly", [](const py::object &) { return 1; });
305304

306305
// test_overload_ordering
307-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
308-
m.def("overload_order", [](std::string) { return 1; });
309-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
310-
m.def("overload_order", [](std::string) { return 2; });
306+
m.def("overload_order", [](const std::string &) { return 1; });
307+
m.def("overload_order", [](const std::string &) { return 2; });
311308
m.def("overload_order", [](int) { return 3; });
312309
m.def("overload_order", [](int) { return 4; }, py::prepend{});
313310

‎tests/test_multiple_inheritance.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -141,8 +141,7 @@ TEST_SUBMODULE(multiple_inheritance, m) {
141141
.def(py::init<int, int>());
142142

143143
m.def("bar_base2a", [](Base2a *b) { return b->bar(); });
144-
// NOLINTNEXTLINE(performance-unnecessary-value-param)
145-
m.def("bar_base2a_sharedptr", [](std::shared_ptr<Base2a> b) { return b->bar(); });
144+
m.def("bar_base2a_sharedptr", [](const std::shared_ptr<Base2a> &b) { return b->bar(); });
146145

147146
// test_mi_unaligned_base
148147
// test_mi_base_return

0 commit comments

Comments
 (0)