diff --git a/.clang-tidy b/.clang-tidy index 60531abe0..22a736208 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -18,6 +18,7 @@ modernize-use-noexcept, modernize-use-emplace, modernize-use-override, modernize-use-using, +*performance*, readability-container-size-empty, readability-make-member-function-const, readability-redundant-function-ptr-dereference, diff --git a/.github/workflows/format.yml b/.github/workflows/format.yml index 6550acccd..27f52932b 100644 --- a/.github/workflows/format.yml +++ b/.github/workflows/format.yml @@ -27,7 +27,7 @@ jobs: clang-tidy: name: Clang-Tidy runs-on: ubuntu-latest - container: silkeh/clang:10 + container: silkeh/clang:12 steps: - uses: actions/checkout@v2 @@ -37,10 +37,10 @@ jobs: - name: Configure run: > cmake -S . -B build - -DCMAKE_CXX_CLANG_TIDY="$(which clang-tidy);--warnings-as-errors=*" + -DCMAKE_CXX_CLANG_TIDY="$(which clang-tidy)" -DDOWNLOAD_EIGEN=ON -DDOWNLOAD_CATCH=ON -DCMAKE_CXX_STANDARD=17 - name: Build - run: cmake --build build -j 2 + run: cmake --build build -j 2 -- --keep-going diff --git a/include/pybind11/iostream.h b/include/pybind11/iostream.h index 6a793eadc..89d1813ba 100644 --- a/include/pybind11/iostream.h +++ b/include/pybind11/iostream.h @@ -262,7 +262,7 @@ add_ostream_redirect(module_ m, const std::string &name = "ostream_redirect") { return class_(std::move(m), name.c_str(), module_local()) .def(init(), arg("stdout") = true, arg("stderr") = true) .def("__enter__", &detail::OstreamRedirect::enter) - .def("__exit__", [](detail::OstreamRedirect &self_, args) { self_.exit(); }); + .def("__exit__", [](detail::OstreamRedirect &self_, const args &) { self_.exit(); }); } PYBIND11_NAMESPACE_END(PYBIND11_NAMESPACE) diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 497923605..1fb0d1fa1 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -1629,12 +1629,13 @@ struct enum_base { auto static_property = handle((PyObject *) get_internals().static_property_type); m_base.attr("__repr__") = cpp_function( - [](object arg) -> str { + [](const object &arg) -> str { handle type = type::handle_of(arg); object type_name = type.attr("__name__"); return pybind11::str("<{}.{}: {}>").format(type_name, enum_name(arg), int_(arg)); - }, name("__repr__"), is_method(m_base) - ); + }, + name("__repr__"), + is_method(m_base)); m_base.attr("name") = property(cpp_function(&enum_name, name("name"), is_method(m_base))); @@ -1718,8 +1719,10 @@ struct enum_base { PYBIND11_ENUM_OP_CONV("__ror__", a | b); PYBIND11_ENUM_OP_CONV("__xor__", a ^ b); PYBIND11_ENUM_OP_CONV("__rxor__", a ^ b); - m_base.attr("__invert__") = cpp_function( - [](object arg) { return ~(int_(arg)); }, name("__invert__"), is_method(m_base)); + m_base.attr("__invert__") + = cpp_function([](const object &arg) { return ~(int_(arg)); }, + name("__invert__"), + is_method(m_base)); } } else { PYBIND11_ENUM_OP_STRICT("__eq__", int_(a).equal(int_(b)), return false); @@ -1740,10 +1743,10 @@ struct enum_base { #undef PYBIND11_ENUM_OP_STRICT m_base.attr("__getstate__") = cpp_function( - [](object arg) { return int_(arg); }, name("__getstate__"), is_method(m_base)); + [](const object &arg) { return int_(arg); }, name("__getstate__"), is_method(m_base)); m_base.attr("__hash__") = cpp_function( - [](object arg) { return int_(arg); }, name("__hash__"), is_method(m_base)); + [](const object &arg) { return int_(arg); }, name("__hash__"), is_method(m_base)); } PYBIND11_NOINLINE void value(char const* name_, object value, const char *doc = nullptr) { diff --git a/tests/local_bindings.h b/tests/local_bindings.h index 91c23fe59..8629ed778 100644 --- a/tests/local_bindings.h +++ b/tests/local_bindings.h @@ -1,4 +1,6 @@ #pragma once +#include + #include "pybind11_tests.h" /// Simple class used to test py::local: @@ -54,7 +56,7 @@ py::class_ bind_local(Args && ...args) { namespace pets { class Pet { public: - Pet(std::string name) : name_(name) {} + Pet(std::string name) : name_(std::move(name)) {} std::string name_; const std::string &name() const { return name_; } }; diff --git a/tests/object.h b/tests/object.h index 9235f19c2..865a9beae 100644 --- a/tests/object.h +++ b/tests/object.h @@ -81,7 +81,7 @@ public: } /// Move constructor - ref(ref &&r) : m_ptr(r.m_ptr) { + ref(ref &&r) noexcept : m_ptr(r.m_ptr) { r.m_ptr = nullptr; print_move_created(this, "with pointer", m_ptr); track_move_created((ref_tag*) this); @@ -96,7 +96,7 @@ public: } /// Move another reference into the current one - ref& operator=(ref&& r) { + ref &operator=(ref &&r) noexcept { print_move_assigned(this, "pointer", r.m_ptr); track_move_assigned((ref_tag*) this); if (*this == r) diff --git a/tests/pybind11_cross_module_tests.cpp b/tests/pybind11_cross_module_tests.cpp index 944d08559..f238ddb04 100644 --- a/tests/pybind11_cross_module_tests.cpp +++ b/tests/pybind11_cross_module_tests.cpp @@ -104,6 +104,8 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) { m.def("return_self", [](LocalVec *v) { return v; }); m.def("return_copy", [](const LocalVec &v) { return LocalVec(v); }); + // Changing this broke things with pygrep. TODO fix + // NOLINTNEXTLINE class Dog : public pets::Pet { public: Dog(std::string name) : Pet(name) {}; }; py::class_(m, "Pet", py::module_local()) .def("name", &pets::Pet::name); @@ -126,6 +128,7 @@ 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_return", []() { return std::vector(); }); } diff --git a/tests/test_buffers.cpp b/tests/test_buffers.cpp index 46eabf396..9902c1084 100644 --- a/tests/test_buffers.cpp +++ b/tests/test_buffers.cpp @@ -27,7 +27,7 @@ TEST_SUBMODULE(buffers, m) { memcpy(m_data, s.m_data, sizeof(float) * (size_t) (m_rows * m_cols)); } - Matrix(Matrix &&s) : m_rows(s.m_rows), m_cols(s.m_cols), m_data(s.m_data) { + Matrix(Matrix &&s) noexcept : m_rows(s.m_rows), m_cols(s.m_cols), m_data(s.m_data) { print_move_created(this); s.m_rows = 0; s.m_cols = 0; @@ -49,7 +49,7 @@ TEST_SUBMODULE(buffers, m) { return *this; } - Matrix &operator=(Matrix &&s) { + Matrix &operator=(Matrix &&s) noexcept { print_move_assigned(this, std::to_string(m_rows) + "x" + std::to_string(m_cols) + " matrix"); if (&s != this) { delete[] m_data; @@ -79,6 +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) { py::buffer_info info = b.request(); if (info.format != py::format_descriptor::format() || info.ndim != 2) @@ -89,31 +90,31 @@ TEST_SUBMODULE(buffers, m) { return v; })) - .def("rows", &Matrix::rows) - .def("cols", &Matrix::cols) + .def("rows", &Matrix::rows) + .def("cols", &Matrix::cols) /// Bare bones interface - .def("__getitem__", [](const Matrix &m, std::pair i) { - if (i.first >= m.rows() || i.second >= m.cols()) - throw py::index_error(); - return m(i.first, i.second); - }) - .def("__setitem__", [](Matrix &m, std::pair i, float v) { - if (i.first >= m.rows() || i.second >= m.cols()) - throw py::index_error(); - m(i.first, i.second) = v; - }) - /// Provide buffer access - .def_buffer([](Matrix &m) -> py::buffer_info { + .def("__getitem__", + [](const Matrix &m, std::pair i) { + if (i.first >= m.rows() || i.second >= m.cols()) + throw py::index_error(); + return m(i.first, i.second); + }) + .def("__setitem__", + [](Matrix &m, std::pair i, float v) { + if (i.first >= m.rows() || i.second >= m.cols()) + throw py::index_error(); + m(i.first, i.second) = v; + }) + /// Provide buffer access + .def_buffer([](Matrix &m) -> py::buffer_info { return py::buffer_info( m.data(), /* Pointer to buffer */ { m.rows(), m.cols() }, /* Buffer dimensions */ { sizeof(float) * size_t(m.cols()), /* Strides (in bytes) for each index */ sizeof(float) } ); - }) - ; - + }); // test_inherited_protocol class SquareMatrix : public Matrix { @@ -208,7 +209,5 @@ TEST_SUBMODULE(buffers, m) { }) ; - m.def("get_buffer_info", [](py::buffer buffer) { - return buffer.request(); - }); + m.def("get_buffer_info", [](const py::buffer &buffer) { return buffer.request(); }); } diff --git a/tests/test_builtin_casters.cpp b/tests/test_builtin_casters.cpp index f4e775676..a98b67bdb 100644 --- a/tests/test_builtin_casters.cpp +++ b/tests/test_builtin_casters.cpp @@ -30,7 +30,11 @@ class type_caster { // cast operator. bool load(handle, bool) { return true; } - operator ConstRefCasted&&() { value = {1}; return std::move(value); } + operator ConstRefCasted &&() { + value = {1}; + // NOLINTNEXTLINE(performance-move-const-arg) + return std::move(value); + } operator ConstRefCasted&() { value = {2}; return value; } operator ConstRefCasted*() { value = {3}; return &value; } @@ -101,6 +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(); }); #ifdef PYBIND11_HAS_U8STRING @@ -146,6 +151,7 @@ TEST_SUBMODULE(builtin_casters, m) { m.def("int_passthrough_noconvert", [](int arg) { return arg; }, py::arg{}.noconvert()); // test_tuple + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("pair_passthrough", [](std::pair input) { return std::make_pair(input.second, input.first); }, "Return a pair in reversed order"); @@ -177,10 +183,13 @@ 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_custom", [](UserType *) { return false; }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("defer_none_custom", [](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; }); // test_void_caster @@ -231,6 +240,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) { py::list l; l.append(f(std::ref(w))); diff --git a/tests/test_callbacks.cpp b/tests/test_callbacks.cpp index 33927f5c3..908d5743e 100644 --- a/tests/test_callbacks.cpp +++ b/tests/test_callbacks.cpp @@ -17,8 +17,8 @@ int dummy_function(int i) { return i + 1; } TEST_SUBMODULE(callbacks, m) { // test_callbacks, test_function_signatures - m.def("test_callback1", [](py::object func) { return func(); }); - m.def("test_callback2", [](py::object func) { return func("Hello", 'x', true, 5); }); + m.def("test_callback1", [](const py::object &func) { return func(); }); + m.def("test_callback2", [](const py::object &func) { return func("Hello", 'x', true, 5); }); m.def("test_callback3", [](const std::function &func) { return "func(43) = " + std::to_string(func(43)); }); m.def("test_callback4", []() -> std::function { return [](int i) { return i+1; }; }); @@ -27,51 +27,48 @@ TEST_SUBMODULE(callbacks, m) { }); // test_keyword_args_and_generalized_unpacking - m.def("test_tuple_unpacking", [](py::function f) { + m.def("test_tuple_unpacking", [](const py::function &f) { auto t1 = py::make_tuple(2, 3); auto t2 = py::make_tuple(5, 6); return f("positional", 1, *t1, 4, *t2); }); - m.def("test_dict_unpacking", [](py::function f) { + m.def("test_dict_unpacking", [](const py::function &f) { auto d1 = py::dict("key"_a="value", "a"_a=1); auto d2 = py::dict(); auto d3 = py::dict("b"_a=2); return f("positional", 1, **d1, **d2, **d3); }); - m.def("test_keyword_args", [](py::function f) { - return f("x"_a=10, "y"_a=20); - }); + m.def("test_keyword_args", [](const py::function &f) { return f("x"_a = 10, "y"_a = 20); }); - m.def("test_unpacking_and_keywords1", [](py::function f) { + m.def("test_unpacking_and_keywords1", [](const py::function &f) { auto args = py::make_tuple(2); auto kwargs = py::dict("d"_a=4); return f(1, *args, "c"_a=3, **kwargs); }); - m.def("test_unpacking_and_keywords2", [](py::function f) { + m.def("test_unpacking_and_keywords2", [](const py::function &f) { auto kwargs1 = py::dict("a"_a=1); auto kwargs2 = py::dict("c"_a=3, "d"_a=4); return f("positional", *py::make_tuple(1), 2, *py::make_tuple(3, 4), 5, "key"_a="value", **kwargs1, "b"_a=2, **kwargs2, "e"_a=5); }); - m.def("test_unpacking_error1", [](py::function f) { + m.def("test_unpacking_error1", [](const py::function &f) { auto kwargs = py::dict("x"_a=3); return f("x"_a=1, "y"_a=2, **kwargs); // duplicate ** after keyword }); - m.def("test_unpacking_error2", [](py::function f) { + m.def("test_unpacking_error2", [](const py::function &f) { auto kwargs = py::dict("x"_a=3); return f(**kwargs, "x"_a=1); // duplicate keyword after ** }); - m.def("test_arg_conversion_error1", [](py::function f) { - f(234, UnregisteredType(), "kw"_a=567); - }); + m.def("test_arg_conversion_error1", + [](const py::function &f) { f(234, UnregisteredType(), "kw"_a = 567); }); - m.def("test_arg_conversion_error2", [](py::function f) { + m.def("test_arg_conversion_error2", [](const py::function &f) { f(234, "expected_name"_a=UnregisteredType(), "kw"_a=567); }); @@ -80,7 +77,7 @@ TEST_SUBMODULE(callbacks, m) { Payload() { print_default_created(this); } ~Payload() { print_destroyed(this); } Payload(const Payload &) { print_copy_created(this); } - Payload(Payload &&) { print_move_created(this); } + Payload(Payload &&) noexcept { print_move_created(this); } }; // Export the payload constructor statistics for testing purposes: m.def("payload_cstats", &ConstructorStats::get); @@ -127,7 +124,8 @@ TEST_SUBMODULE(callbacks, m) { virtual ~AbstractBase() {}; // NOLINT(modernize-use-equals-default) virtual unsigned int func() = 0; }; - m.def("func_accepting_func_accepting_base", [](std::function) { }); + m.def("func_accepting_func_accepting_base", + [](const std::function &) {}); struct MovableObject { bool valid = true; @@ -135,8 +133,8 @@ TEST_SUBMODULE(callbacks, m) { MovableObject() = default; MovableObject(const MovableObject &) = default; MovableObject &operator=(const MovableObject &) = default; - MovableObject(MovableObject &&o) : valid(o.valid) { o.valid = false; } - MovableObject &operator=(MovableObject &&o) { + MovableObject(MovableObject &&o) noexcept : valid(o.valid) { o.valid = false; } + MovableObject &operator=(MovableObject &&o) noexcept { valid = o.valid; o.valid = false; return *this; @@ -145,7 +143,7 @@ TEST_SUBMODULE(callbacks, m) { py::class_(m, "MovableObject"); // test_movable_object - m.def("callback_with_movable", [](std::function f) { + m.def("callback_with_movable", [](const std::function &f) { auto x = MovableObject(); f(x); // lvalue reference shouldn't move out object return x.valid; // must still return `true` @@ -159,7 +157,7 @@ TEST_SUBMODULE(callbacks, m) { // test async Python callbacks using callback_f = std::function; - m.def("test_async_callback", [](callback_f f, py::list work) { + m.def("test_async_callback", [](const callback_f &f, const py::list &work) { // make detached thread that calls `f` with piece of work after a little delay auto start_f = [f](int j) { auto invoke_f = [f, j] { @@ -175,7 +173,7 @@ TEST_SUBMODULE(callbacks, m) { start_f(py::cast(i)); }); - m.def("callback_num_times", [](py::function f, std::size_t num) { + m.def("callback_num_times", [](const py::function &f, std::size_t num) { for (std::size_t i = 0; i < num; i++) { f(); } diff --git a/tests/test_class.cpp b/tests/test_class.cpp index bd545e8c6..a26c59c1c 100644 --- a/tests/test_class.cpp +++ b/tests/test_class.cpp @@ -19,6 +19,8 @@ #include "local_bindings.h" #include +#include + #if defined(_MSC_VER) # pragma warning(disable: 4324) // warning C4324: structure was padded due to alignment specifier #endif @@ -129,6 +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) { return py::make_tuple( py::isinstance(l[0]), @@ -155,17 +158,13 @@ TEST_SUBMODULE(class_, m) { return py::type::of(); }); - m.def("get_type_of", [](py::object ob) { - return py::type::of(ob); - }); + m.def("get_type_of", [](py::object ob) { return py::type::of(std::move(ob)); }); m.def("get_type_classic", [](py::handle h) { return h.get_type(); }); - m.def("as_type", [](py::object ob) { - return py::type(ob); - }); + m.def("as_type", [](const py::object &ob) { return py::type(ob); }); // test_mismatched_holder struct MismatchBase1 { }; @@ -219,6 +218,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) { // `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 @@ -397,6 +397,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 {}; }); py::class_(m, "StringWrapper").def(py::init()); py::implicitly_convertible(); @@ -461,19 +462,20 @@ TEST_SUBMODULE(class_, m) { struct OtherDuplicate {}; struct DuplicateNested {}; struct OtherDuplicateNested {}; - m.def("register_duplicate_class_name", [](py::module_ m) { + + m.def("register_duplicate_class_name", [](const py::module_ &m) { py::class_(m, "Duplicate"); py::class_(m, "Duplicate"); }); - m.def("register_duplicate_class_type", [](py::module_ m) { + m.def("register_duplicate_class_type", [](const py::module_ &m) { py::class_(m, "OtherDuplicate"); py::class_(m, "YetAnotherDuplicate"); }); - m.def("register_duplicate_nested_class_name", [](py::object gt) { + m.def("register_duplicate_nested_class_name", [](const py::object >) { py::class_(gt, "DuplicateNested"); py::class_(gt, "DuplicateNested"); }); - m.def("register_duplicate_nested_class_type", [](py::object gt) { + m.def("register_duplicate_nested_class_type", [](const py::object >) { py::class_(gt, "OtherDuplicateNested"); py::class_(gt, "YetAnotherDuplicateNested"); }); diff --git a/tests/test_constants_and_functions.cpp b/tests/test_constants_and_functions.cpp index 04830d8d2..e7f018540 100644 --- a/tests/test_constants_and_functions.cpp +++ b/tests/test_constants_and_functions.cpp @@ -34,7 +34,7 @@ py::bytes return_bytes() { return std::string(data, 4); } -std::string print_bytes(py::bytes bytes) { +std::string print_bytes(const py::bytes &bytes) { std::string ret = "bytes["; const auto value = static_cast(bytes); for (size_t i = 0; i < value.length(); ++i) { @@ -146,8 +146,13 @@ TEST_SUBMODULE(constants_and_functions, m) { LargeCapture capture; // VS 2015's MSVC is acting up if we create the array here m.def("should_raise", [capture](int) { return capture.zeros[9] + 33; }, py::kw_only(), py::arg()); }); - m.def("register_with_raising_repr", [](py::module_ m, py::object default_value) { - m.def("should_raise", [](int, int, py::object) { return 42; }, "some docstring", - py::arg_v("x", 42), py::arg_v("y", 42, ""), py::arg_v("z", default_value)); + m.def("register_with_raising_repr", [](py::module_ m, const py::object &default_value) { + m.def( + "should_raise", + [](int, int, const py::object &) { return 42; }, + "some docstring", + py::arg_v("x", 42), + py::arg_v("y", 42, ""), + py::arg_v("z", default_value)); }); } diff --git a/tests/test_copy_move.cpp b/tests/test_copy_move.cpp index 322e9bb85..08f4dd749 100644 --- a/tests/test_copy_move.cpp +++ b/tests/test_copy_move.cpp @@ -37,9 +37,16 @@ template <> lacking_move_ctor empty::instance_ = {}; class MoveOnlyInt { public: MoveOnlyInt() { print_default_created(this); } - MoveOnlyInt(int v) : value{std::move(v)} { print_created(this, value); } - MoveOnlyInt(MoveOnlyInt &&m) { print_move_created(this, m.value); std::swap(value, m.value); } - MoveOnlyInt &operator=(MoveOnlyInt &&m) { print_move_assigned(this, m.value); std::swap(value, m.value); return *this; } + MoveOnlyInt(int v) : value{v} { print_created(this, value); } + MoveOnlyInt(MoveOnlyInt &&m) noexcept { + print_move_created(this, m.value); + std::swap(value, m.value); + } + MoveOnlyInt &operator=(MoveOnlyInt &&m) noexcept { + print_move_assigned(this, m.value); + std::swap(value, m.value); + return *this; + } MoveOnlyInt(const MoveOnlyInt &) = delete; MoveOnlyInt &operator=(const MoveOnlyInt &) = delete; ~MoveOnlyInt() { print_destroyed(this); } @@ -49,9 +56,16 @@ public: class MoveOrCopyInt { public: MoveOrCopyInt() { print_default_created(this); } - MoveOrCopyInt(int v) : value{std::move(v)} { print_created(this, value); } - MoveOrCopyInt(MoveOrCopyInt &&m) { print_move_created(this, m.value); std::swap(value, m.value); } - MoveOrCopyInt &operator=(MoveOrCopyInt &&m) { print_move_assigned(this, m.value); std::swap(value, m.value); return *this; } + MoveOrCopyInt(int v) : value{v} { print_created(this, value); } + MoveOrCopyInt(MoveOrCopyInt &&m) noexcept { + print_move_created(this, m.value); + std::swap(value, m.value); + } + MoveOrCopyInt &operator=(MoveOrCopyInt &&m) noexcept { + print_move_assigned(this, m.value); + std::swap(value, m.value); + return *this; + } MoveOrCopyInt(const MoveOrCopyInt &c) { print_copy_created(this, c.value); value = c.value; } MoveOrCopyInt &operator=(const MoveOrCopyInt &c) { print_copy_assigned(this, c.value); value = c.value; return *this; } ~MoveOrCopyInt() { print_destroyed(this); } @@ -61,7 +75,7 @@ public: class CopyOnlyInt { public: CopyOnlyInt() { print_default_created(this); } - CopyOnlyInt(int v) : value{std::move(v)} { print_created(this, value); } + CopyOnlyInt(int v) : value{v} { print_created(this, value); } CopyOnlyInt(const CopyOnlyInt &c) { print_copy_created(this, c.value); value = c.value; } CopyOnlyInt &operator=(const CopyOnlyInt &c) { print_copy_assigned(this, c.value); value = c.value; return *this; } ~CopyOnlyInt() { print_destroyed(this); } @@ -111,6 +125,7 @@ TEST_SUBMODULE(copy_move_policies, m) { py::return_value_policy::move); // test_move_and_copy_casts + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("move_and_copy_casts", [](py::object o) { int r = 0; r += py::cast(o).value; /* moves */ @@ -126,7 +141,9 @@ TEST_SUBMODULE(copy_move_policies, m) { // test_move_and_copy_loads m.def("move_only", [](MoveOnlyInt m) { return m.value; }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("move_or_copy", [](MoveOrCopyInt m) { return m.value; }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("copy_only", [](CopyOnlyInt m) { return m.value; }); m.def("move_pair", [](std::pair p) { return p.first.value + p.second.value; diff --git a/tests/test_custom_type_casters.cpp b/tests/test_custom_type_casters.cpp index 3fe910d49..076777d6f 100644 --- a/tests/test_custom_type_casters.cpp +++ b/tests/test_custom_type_casters.cpp @@ -67,9 +67,12 @@ public: DestructionTester() { print_default_created(this); } ~DestructionTester() { print_destroyed(this); } DestructionTester(const DestructionTester &) { print_copy_created(this); } - DestructionTester(DestructionTester &&) { print_move_created(this); } + DestructionTester(DestructionTester &&) noexcept { print_move_created(this); } DestructionTester &operator=(const DestructionTester &) { print_copy_assigned(this); return *this; } - DestructionTester &operator=(DestructionTester &&) { print_move_assigned(this); return *this; } + DestructionTester &operator=(DestructionTester &&) noexcept { + print_move_assigned(this); + return *this; + } }; namespace pybind11 { namespace detail { template <> struct type_caster { @@ -94,7 +97,11 @@ TEST_SUBMODULE(custom_type_casters, m) { class ArgInspector { public: ArgInspector1 f(ArgInspector1 a, ArgAlwaysConverts) { return a; } - std::string g(ArgInspector1 a, const ArgInspector1 &b, int c, ArgInspector2 *d, ArgAlwaysConverts) { + std::string g(const ArgInspector1 &a, + const ArgInspector1 &b, + int c, + ArgInspector2 *d, + ArgAlwaysConverts) { return a.arg + "\n" + b.arg + "\n" + std::to_string(c) + "\n" + d->arg; } static ArgInspector2 h(ArgInspector2 a, ArgAlwaysConverts) { return a; } @@ -106,8 +113,14 @@ TEST_SUBMODULE(custom_type_casters, m) { .def("g", &ArgInspector::g, "a"_a.noconvert(), "b"_a, "c"_a.noconvert()=13, "d"_a=ArgInspector2(), py::arg() = ArgAlwaysConverts()) .def_static("h", &ArgInspector::h, py::arg{}.noconvert(), py::arg() = ArgAlwaysConverts()) ; - m.def("arg_inspect_func", [](ArgInspector2 a, ArgInspector1 b, ArgAlwaysConverts) { return a.arg + "\n" + b.arg; }, - py::arg{}.noconvert(false), py::arg_v(nullptr, ArgInspector1()).noconvert(true), py::arg() = ArgAlwaysConverts()); + m.def( + "arg_inspect_func", + [](const ArgInspector2 &a, const ArgInspector1 &b, ArgAlwaysConverts) { + return a.arg + "\n" + b.arg; + }, + py::arg{}.noconvert(false), + py::arg_v(nullptr, ArgInspector1()).noconvert(true), + py::arg() = ArgAlwaysConverts()); m.def("floats_preferred", [](double f) { return 0.5 * f; }, "f"_a); m.def("floats_only", [](double f) { return 0.5 * f; }, "f"_a.noconvert()); diff --git a/tests/test_eigen.cpp b/tests/test_eigen.cpp index 843254743..a24bf273d 100644 --- a/tests/test_eigen.cpp +++ b/tests/test_eigen.cpp @@ -54,8 +54,7 @@ void reset_refs() { } // Returns element 2,1 from a matrix (used to test copy/nocopy) -double get_elem(Eigen::Ref m) { return m(2, 1); }; - +double get_elem(const Eigen::Ref &m) { return m(2, 1); }; // Returns a matrix with 10*r + 100*c added to each matrix element (to help test that the matrix // reference is referencing rows/columns correctly). @@ -94,14 +93,16 @@ TEST_SUBMODULE(eigen, m) { m.def("double_complex", [](const Eigen::VectorXcf &x) -> Eigen::VectorXcf { return 2.0f * x; }); m.def("double_threec", [](py::EigenDRef x) { x *= 2; }); m.def("double_threer", [](py::EigenDRef x) { x *= 2; }); - m.def("double_mat_cm", [](Eigen::MatrixXf x) -> Eigen::MatrixXf { return 2.0f * x; }); - m.def("double_mat_rm", [](DenseMatrixR x) -> DenseMatrixR { return 2.0f * x; }); + m.def("double_mat_cm", [](const Eigen::MatrixXf &x) -> Eigen::MatrixXf { return 2.0f * x; }); + m.def("double_mat_rm", [](const DenseMatrixR &x) -> DenseMatrixR { return 2.0f * x; }); // 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("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(); }); // test_eigen_ref_mutators @@ -247,8 +248,11 @@ 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) {}); // test_dense m.def("dense_r", [mat]() -> DenseMatrixR { return DenseMatrixR(mat); }); @@ -280,6 +284,7 @@ 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()); // Also test a row-major-only no-copy const ref: @@ -295,13 +300,16 @@ 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. + // 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; }); // 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!"); @@ -318,6 +326,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) { py::module_::import("numpy").attr("ones")(10); return v(5); diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 944334ce6..6925ee978 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/tests/test_embed/test_interpreter.cpp @@ -8,16 +8,17 @@ #include -#include #include #include +#include +#include namespace py = pybind11; using namespace py::literals; class Widget { public: - Widget(std::string message) : message(message) { } + Widget(std::string message) : message(std::move(message)) {} virtual ~Widget() = default; std::string the_message() const { return message; } diff --git a/tests/test_eval.cpp b/tests/test_eval.cpp index 5416c2ec4..66cdf7105 100644 --- a/tests/test_eval.cpp +++ b/tests/test_eval.cpp @@ -9,7 +9,9 @@ #include + #include "pybind11_tests.h" +#include TEST_SUBMODULE(eval_, m) { // test_evals @@ -67,7 +69,7 @@ TEST_SUBMODULE(eval_, m) { int val_out; local["call_test2"] = py::cpp_function([&](int value) { val_out = value; }); - auto result = py::eval_file(filename, global, local); + auto result = py::eval_file(std::move(filename), global, local); return val_out == 43 && result.is_none(); }); diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 4c5e10dbe..d34f1a94e 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -8,7 +8,9 @@ */ #include "test_exceptions.h" + #include "pybind11_tests.h" +#include // A type that should be raised as an exception in Python class MyException : public std::exception { @@ -199,6 +201,8 @@ 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) { try { PythonCallInDestructor set_dict_in_destructor(d); @@ -210,21 +214,23 @@ TEST_SUBMODULE(exceptions, m) { return false; }); - m.def("python_alreadyset_in_destructor", [](py::str s) { + m.def("python_alreadyset_in_destructor", [](const py::str &s) { PythonAlreadySetInDestructor alreadyset_in_destructor(s); return true; }); // test_nested_throws - m.def("try_catch", [m](py::object exc_type, py::function f, py::args args) { - try { f(*args); } - catch (py::error_already_set &ex) { - if (ex.matches(exc_type)) - py::print(ex.what()); - else - throw; - } - }); + m.def("try_catch", + [m](const py::object &exc_type, const py::function &f, const py::args &args) { + try { + f(*args); + } catch (py::error_already_set &ex) { + if (ex.matches(exc_type)) + py::print(ex.what()); + else + throw; + } + }); // Test repr that cannot be displayed m.def("simple_bool_passthrough", [](bool x) {return x;}); diff --git a/tests/test_factory_constructors.cpp b/tests/test_factory_constructors.cpp index 7308ad5fc..235ec4dca 100644 --- a/tests/test_factory_constructors.cpp +++ b/tests/test_factory_constructors.cpp @@ -8,10 +8,11 @@ BSD-style license that can be found in the LICENSE file. */ -#include "pybind11_tests.h" #include "constructor_stats.h" +#include "pybind11_tests.h" #include #include +#include // Classes for testing python construction via C++ factory function: // Not publicly constructible, copyable, or movable: @@ -35,8 +36,15 @@ class TestFactory2 { TestFactory2(int v) : value(std::to_string(v)) { print_created(this, value); } TestFactory2(std::string v) : value(std::move(v)) { print_created(this, value); } public: - TestFactory2(TestFactory2 &&m) { value = std::move(m.value); print_move_created(this); } - TestFactory2 &operator=(TestFactory2 &&m) { value = std::move(m.value); print_move_assigned(this); return *this; } + TestFactory2(TestFactory2 &&m) noexcept { + value = std::move(m.value); + print_move_created(this); + } + TestFactory2 &operator=(TestFactory2 &&m) noexcept { + value = std::move(m.value); + print_move_assigned(this); + return *this; + } std::string value; ~TestFactory2() { print_destroyed(this); } }; @@ -48,8 +56,15 @@ protected: TestFactory3(int v) : value(std::to_string(v)) { print_created(this, value); } public: TestFactory3(std::string v) : value(std::move(v)) { print_created(this, value); } - TestFactory3(TestFactory3 &&m) { value = std::move(m.value); print_move_created(this); } - TestFactory3 &operator=(TestFactory3 &&m) { value = std::move(m.value); print_move_assigned(this); return *this; } + TestFactory3(TestFactory3 &&m) noexcept { + value = std::move(m.value); + print_move_created(this); + } + TestFactory3 &operator=(TestFactory3 &&m) noexcept { + value = std::move(m.value); + print_move_assigned(this); + return *this; + } std::string value; virtual ~TestFactory3() { print_destroyed(this); } }; @@ -73,7 +88,11 @@ protected: bool alias = false; public: TestFactory6(int i) : value{i} { print_created(this, i); } - TestFactory6(TestFactory6 &&f) { print_move_created(this); value = f.value; alias = f.alias; } + TestFactory6(TestFactory6 &&f) noexcept { + print_move_created(this); + value = f.value; + alias = f.alias; + } TestFactory6(const TestFactory6 &f) { print_copy_created(this); value = f.value; alias = f.alias; } virtual ~TestFactory6() { print_destroyed(this); } virtual int get() { return value; } @@ -85,7 +104,7 @@ public: // when an alias is needed: PyTF6(TestFactory6 &&base) : TestFactory6(std::move(base)) { alias = true; print_created(this, "move", value); } PyTF6(int i) : TestFactory6(i) { alias = true; print_created(this, i); } - PyTF6(PyTF6 &&f) : TestFactory6(std::move(f)) { print_move_created(this); } + PyTF6(PyTF6 &&f) noexcept : TestFactory6(std::move(f)) { print_move_created(this); } PyTF6(const PyTF6 &f) : TestFactory6(f) { print_copy_created(this); } PyTF6(std::string s) : TestFactory6((int) s.size()) { alias = true; print_created(this, s); } ~PyTF6() override { print_destroyed(this); } @@ -98,7 +117,11 @@ protected: bool alias = false; public: TestFactory7(int i) : value{i} { print_created(this, i); } - TestFactory7(TestFactory7 &&f) { print_move_created(this); value = f.value; alias = f.alias; } + TestFactory7(TestFactory7 &&f) noexcept { + print_move_created(this); + value = f.value; + alias = f.alias; + } TestFactory7(const TestFactory7 &f) { print_copy_created(this); value = f.value; alias = f.alias; } virtual ~TestFactory7() { print_destroyed(this); } virtual int get() { return value; } @@ -107,7 +130,7 @@ public: class PyTF7 : public TestFactory7 { public: PyTF7(int i) : TestFactory7(i) { alias = true; print_created(this, i); } - PyTF7(PyTF7 &&f) : TestFactory7(std::move(f)) { print_move_created(this); } + PyTF7(PyTF7 &&f) noexcept : TestFactory7(std::move(f)) { print_move_created(this); } PyTF7(const PyTF7 &f) : TestFactory7(f) { print_copy_created(this); } ~PyTF7() override { print_destroyed(this); } int get() override { PYBIND11_OVERRIDE(int, TestFactory7, get, /*no args*/); } @@ -122,7 +145,9 @@ public: // Holder: static std::unique_ptr construct1(int a) { return std::unique_ptr(new TestFactory1(a)); } // pointer again - static TestFactory1 *construct1_string(std::string a) { return new TestFactory1(a); } + static TestFactory1 *construct1_string(std::string a) { + return new TestFactory1(std::move(a)); + } // Moveable type: // pointer: @@ -130,7 +155,7 @@ public: // holder: static std::unique_ptr construct2(int a) { return std::unique_ptr(new TestFactory2(a)); } // by value moving: - static TestFactory2 construct2(std::string a) { return TestFactory2(a); } + static TestFactory2 construct2(std::string a) { return TestFactory2(std::move(a)); } // shared_ptr holder type: // pointer: @@ -173,10 +198,11 @@ TEST_SUBMODULE(factory_constructors, m) { ; py::class_(m, "TestFactory2") .def(py::init([](pointer_tag, int v) { return TestFactoryHelper::construct2(v); })) - .def(py::init([](unique_ptr_tag, std::string v) { return TestFactoryHelper::construct2(v); })) + .def(py::init([](unique_ptr_tag, std::string v) { + return TestFactoryHelper::construct2(std::move(v)); + })) .def(py::init([](move_tag) { return TestFactoryHelper::construct2(); })) - .def_readwrite("value", &TestFactory2::value) - ; + .def_readwrite("value", &TestFactory2::value); // Stateful & reused: int c = 1; @@ -188,7 +214,9 @@ TEST_SUBMODULE(factory_constructors, m) { .def(py::init([](pointer_tag, int v) { return TestFactoryHelper::construct3(v); })) .def(py::init([](shared_ptr_tag) { return TestFactoryHelper::construct3(); })); ignoreOldStyleInitWarnings([&pyTestFactory3]() { - pyTestFactory3.def("__init__", [](TestFactory3 &self, std::string v) { new (&self) TestFactory3(v); }); // placement-new ctor + pyTestFactory3.def("__init__", [](TestFactory3 &self, std::string v) { + new (&self) TestFactory3(std::move(v)); + }); // placement-new ctor }); pyTestFactory3 // factories returning a derived type: @@ -219,52 +247,54 @@ TEST_SUBMODULE(factory_constructors, m) { py::class_(m, "TestFactory6") .def(py::init([](base_tag, int i) { return TestFactory6(i); })) .def(py::init([](alias_tag, int i) { return PyTF6(i); })) - .def(py::init([](alias_tag, std::string s) { return PyTF6(s); })) + .def(py::init([](alias_tag, std::string s) { return PyTF6(std::move(s)); })) .def(py::init([](alias_tag, pointer_tag, int i) { return new PyTF6(i); })) .def(py::init([](base_tag, pointer_tag, int i) { return new TestFactory6(i); })) - .def(py::init([](base_tag, alias_tag, pointer_tag, int i) { return (TestFactory6 *) new PyTF6(i); })) + .def(py::init( + [](base_tag, alias_tag, pointer_tag, int i) { return (TestFactory6 *) new PyTF6(i); })) .def("get", &TestFactory6::get) .def("has_alias", &TestFactory6::has_alias) - .def_static("get_cstats", &ConstructorStats::get, py::return_value_policy::reference) - .def_static("get_alias_cstats", &ConstructorStats::get, py::return_value_policy::reference) - ; + .def_static( + "get_cstats", &ConstructorStats::get, py::return_value_policy::reference) + .def_static( + "get_alias_cstats", &ConstructorStats::get, py::return_value_policy::reference); // test_init_factory_dual // Separate alias constructor testing py::class_>(m, "TestFactory7") - .def(py::init( - [](int i) { return TestFactory7(i); }, - [](int i) { return PyTF7(i); })) - .def(py::init( - [](pointer_tag, int i) { return new TestFactory7(i); }, - [](pointer_tag, int i) { return new PyTF7(i); })) - .def(py::init( - [](mixed_tag, int i) { return new TestFactory7(i); }, - [](mixed_tag, int i) { return PyTF7(i); })) - .def(py::init( - [](mixed_tag, std::string s) { return TestFactory7((int) s.size()); }, - [](mixed_tag, std::string s) { return new PyTF7((int) s.size()); })) - .def(py::init( - [](base_tag, pointer_tag, int i) { return new TestFactory7(i); }, - [](base_tag, pointer_tag, int i) { return (TestFactory7 *) new PyTF7(i); })) - .def(py::init( - [](alias_tag, pointer_tag, int i) { return new PyTF7(i); }, - [](alias_tag, pointer_tag, int i) { return new PyTF7(10*i); })) + .def(py::init([](int i) { return TestFactory7(i); }, [](int i) { return PyTF7(i); })) + .def(py::init([](pointer_tag, int i) { return new TestFactory7(i); }, + [](pointer_tag, int i) { return new PyTF7(i); })) + .def(py::init([](mixed_tag, int i) { return new TestFactory7(i); }, + [](mixed_tag, int i) { return PyTF7(i); })) + .def(py::init([](mixed_tag, const std::string &s) { return TestFactory7((int) s.size()); }, + [](mixed_tag, const std::string &s) { return new PyTF7((int) s.size()); })) + .def(py::init([](base_tag, pointer_tag, int i) { return new TestFactory7(i); }, + [](base_tag, pointer_tag, int i) { return (TestFactory7 *) new PyTF7(i); })) + .def(py::init([](alias_tag, pointer_tag, int i) { return new PyTF7(i); }, + [](alias_tag, pointer_tag, int i) { return new PyTF7(10 * i); })) .def(py::init( [](shared_ptr_tag, base_tag, int i) { return std::make_shared(i); }, - [](shared_ptr_tag, base_tag, int i) { auto *p = new PyTF7(i); return std::shared_ptr(p); })) - .def(py::init( - [](shared_ptr_tag, invalid_base_tag, int i) { return std::make_shared(i); }, - [](shared_ptr_tag, invalid_base_tag, int i) { return std::make_shared(i); })) // <-- invalid alias factory + [](shared_ptr_tag, base_tag, int i) { + auto *p = new PyTF7(i); + return std::shared_ptr(p); + })) + .def(py::init([](shared_ptr_tag, + invalid_base_tag, + int i) { return std::make_shared(i); }, + [](shared_ptr_tag, invalid_base_tag, int i) { + return std::make_shared(i); + })) // <-- invalid alias factory .def("get", &TestFactory7::get) .def("has_alias", &TestFactory7::has_alias) - .def_static("get_cstats", &ConstructorStats::get, py::return_value_policy::reference) - .def_static("get_alias_cstats", &ConstructorStats::get, py::return_value_policy::reference) - ; + .def_static( + "get_cstats", &ConstructorStats::get, py::return_value_policy::reference) + .def_static( + "get_alias_cstats", &ConstructorStats::get, py::return_value_policy::reference); // test_placement_new_alternative // Class with a custom new operator but *without* a placement new operator (issue #948) @@ -331,12 +361,10 @@ TEST_SUBMODULE(factory_constructors, m) { pyNoisyAlloc.def(py::init([](int i, double) { return new NoisyAlloc(i); })); // Regular again: requires yet another preallocation ignoreOldStyleInitWarnings([&pyNoisyAlloc]() { - pyNoisyAlloc.def("__init__", [](NoisyAlloc &a, int i, std::string) { new (&a) NoisyAlloc(i); }); + pyNoisyAlloc.def( + "__init__", [](NoisyAlloc &a, int i, const std::string &) { new (&a) NoisyAlloc(i); }); }); - - - // static_assert testing (the following def's should all fail with appropriate compilation errors): #if 0 struct BadF1Base {}; diff --git a/tests/test_gil_scoped.cpp b/tests/test_gil_scoped.cpp index b6a45a5f0..b261085c8 100644 --- a/tests/test_gil_scoped.cpp +++ b/tests/test_gil_scoped.cpp @@ -35,20 +35,15 @@ TEST_SUBMODULE(gil_scoped, m) { .def("virtual_func", &VirtClass::virtual_func) .def("pure_virtual_func", &VirtClass::pure_virtual_func); - m.def("test_callback_py_obj", - [](py::object func) { func(); }); - m.def("test_callback_std_func", - [](const std::function &func) { func(); }); - m.def("test_callback_virtual_func", - [](VirtClass &virt) { virt.virtual_func(); }); - m.def("test_callback_pure_virtual_func", - [](VirtClass &virt) { virt.pure_virtual_func(); }); - m.def("test_cross_module_gil", - []() { - auto cm = py::module_::import("cross_module_gil_utils"); - auto gil_acquire = reinterpret_cast( - PyLong_AsVoidPtr(cm.attr("gil_acquire_funcaddr").ptr())); - py::gil_scoped_release gil_release; - gil_acquire(); - }); + m.def("test_callback_py_obj", [](py::object &func) { func(); }); + m.def("test_callback_std_func", [](const std::function &func) { func(); }); + m.def("test_callback_virtual_func", [](VirtClass &virt) { virt.virtual_func(); }); + m.def("test_callback_pure_virtual_func", [](VirtClass &virt) { virt.pure_virtual_func(); }); + m.def("test_cross_module_gil", []() { + auto cm = py::module_::import("cross_module_gil_utils"); + auto gil_acquire + = reinterpret_cast(PyLong_AsVoidPtr(cm.attr("gil_acquire_funcaddr").ptr())); + py::gil_scoped_release gil_release; + gil_acquire(); + }); } diff --git a/tests/test_iostream.cpp b/tests/test_iostream.cpp index f21fed546..908788007 100644 --- a/tests/test_iostream.cpp +++ b/tests/test_iostream.cpp @@ -17,15 +17,14 @@ #include #include - -void noisy_function(std::string msg, bool flush) { +void noisy_function(const std::string &msg, bool flush) { std::cout << msg; if (flush) std::cout << std::flush; } -void noisy_funct_dual(std::string msg, std::string emsg) { +void noisy_funct_dual(const std::string &msg, const std::string &emsg) { std::cout << msg; std::cerr << emsg; } @@ -70,12 +69,12 @@ TEST_SUBMODULE(iostream, m) { // test_evals - m.def("captured_output_default", [](std::string msg) { + m.def("captured_output_default", [](const std::string &msg) { py::scoped_ostream_redirect redir; std::cout << msg << std::flush; }); - m.def("captured_output", [](std::string msg) { + m.def("captured_output", [](const std::string &msg) { py::scoped_ostream_redirect redir(std::cout, py::module_::import("sys").attr("stdout")); std::cout << msg << std::flush; }); @@ -84,7 +83,7 @@ TEST_SUBMODULE(iostream, m) { py::call_guard(), py::arg("msg"), py::arg("flush")=true); - m.def("captured_err", [](std::string msg) { + m.def("captured_err", [](const std::string &msg) { py::scoped_ostream_redirect redir(std::cerr, py::module_::import("sys").attr("stderr")); std::cerr << msg << std::flush; }); @@ -95,15 +94,11 @@ TEST_SUBMODULE(iostream, m) { py::call_guard(), py::arg("msg"), py::arg("emsg")); - m.def("raw_output", [](std::string msg) { - std::cout << msg << std::flush; - }); + m.def("raw_output", [](const std::string &msg) { std::cout << msg << std::flush; }); - m.def("raw_err", [](std::string msg) { - std::cerr << msg << std::flush; - }); + m.def("raw_err", [](const std::string &msg) { std::cerr << msg << std::flush; }); - m.def("captured_dual", [](std::string msg, std::string emsg) { + m.def("captured_dual", [](const std::string &msg, const std::string &emsg) { py::scoped_ostream_redirect redirout(std::cout, py::module_::import("sys").attr("stdout")); py::scoped_ostream_redirect redirerr(std::cerr, py::module_::import("sys").attr("stderr")); std::cout << msg << std::flush; diff --git a/tests/test_kwargs_and_defaults.cpp b/tests/test_kwargs_and_defaults.cpp index 627a79690..ab1c94c91 100644 --- a/tests/test_kwargs_and_defaults.cpp +++ b/tests/test_kwargs_and_defaults.cpp @@ -11,6 +11,8 @@ #include "constructor_stats.h" #include +#include + TEST_SUBMODULE(kwargs_and_defaults, m) { auto kw_func = [](int x, int y) { return "x=" + std::to_string(x) + ", y=" + std::to_string(y); }; @@ -37,18 +39,16 @@ TEST_SUBMODULE(kwargs_and_defaults, m) { m.def("args_function", [](py::args args) -> py::tuple { return std::move(args); }); - m.def("args_kwargs_function", [](py::args args, py::kwargs kwargs) { + m.def("args_kwargs_function", [](const py::args &args, const py::kwargs &kwargs) { return py::make_tuple(args, kwargs); }); // test_mixed_args_and_kwargs - m.def("mixed_plus_args", [](int i, double j, py::args args) { - return py::make_tuple(i, j, args); - }); - m.def("mixed_plus_kwargs", [](int i, double j, py::kwargs kwargs) { - return py::make_tuple(i, j, kwargs); - }); - auto mixed_plus_both = [](int i, double j, py::args args, py::kwargs kwargs) { + m.def("mixed_plus_args", + [](int i, double j, const py::args &args) { return py::make_tuple(i, j, args); }); + m.def("mixed_plus_kwargs", + [](int i, double j, const py::kwargs &kwargs) { return py::make_tuple(i, j, kwargs); }); + auto mixed_plus_both = [](int i, double j, const py::args &args, const py::kwargs &kwargs) { return py::make_tuple(i, j, args, kwargs); }; m.def("mixed_plus_args_kwargs", mixed_plus_both); @@ -65,6 +65,8 @@ 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 appropiate + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("arg_refcount_o", [](py::object o) { GC_IF_NEEDED; return o.ref_count(); }); m.def("args_refcount", [](py::args a) { GC_IF_NEEDED; @@ -74,6 +76,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) { GC_IF_NEEDED; py::tuple t(a.size() + 1); @@ -103,6 +106,7 @@ TEST_SUBMODULE(kwargs_and_defaults, m) { py::arg() = 3, "j"_a = 4, py::kw_only(), "k"_a = 5, "z"_a); m.def("kw_only_mixed", [](int i, int j) { return py::make_tuple(i, j); }, "i"_a, py::kw_only(), "j"_a); + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("kw_only_plus_more", [](int i, int j, int k, py::kwargs kwargs) { return py::make_tuple(i, j, k, kwargs); }, py::arg() /* positional */, py::arg("j") = -1 /* both */, py::kw_only(), py::arg("k") /* kw-only */); @@ -137,6 +141,8 @@ TEST_SUBMODULE(kwargs_and_defaults, m) { // Make sure a class (not an instance) can be used as a default argument. // The return value doesn't matter, only that the module is importable. - m.def("class_default_argument", [](py::object a) { return py::repr(a); }, + m.def( + "class_default_argument", + [](py::object a) { return py::repr(std::move(a)); }, "a"_a = py::module_::import("decimal").attr("Decimal")); } diff --git a/tests/test_local_bindings.cpp b/tests/test_local_bindings.cpp index c61e38883..2ac77b143 100644 --- a/tests/test_local_bindings.cpp +++ b/tests/test_local_bindings.cpp @@ -86,6 +86,8 @@ TEST_SUBMODULE(local_bindings, m) { m.def("return_self", [](LocalVec *v) { return v; }); m.def("return_copy", [](const LocalVec &v) { return LocalVec(v); }); + // Reformatting this class broke pygrep checks + // NOLINTNEXTLINE class Cat : public pets::Pet { public: Cat(std::string name) : Pet(name) {}; }; py::class_(m, "Pet", py::module_local()) .def("get_name", &pets::Pet::name); diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp index 1f855ef35..3eb22b4a3 100644 --- a/tests/test_methods_and_attributes.cpp +++ b/tests/test_methods_and_attributes.cpp @@ -22,14 +22,18 @@ public: ExampleMandA(int value) : value(value) { print_created(this, value); } ExampleMandA(const ExampleMandA &e) : value(e.value) { print_copy_created(this); } ExampleMandA(std::string&&) {} - ExampleMandA(ExampleMandA &&e) : value(e.value) { print_move_created(this); } + ExampleMandA(ExampleMandA &&e) noexcept : value(e.value) { print_move_created(this); } ~ExampleMandA() { print_destroyed(this); } std::string toString() const { return "ExampleMandA[value=" + std::to_string(value) + "]"; } void operator=(const ExampleMandA &e) { print_copy_assigned(this); value = e.value; } - void operator=(ExampleMandA &&e) { print_move_assigned(this); value = e.value; } + void operator=(ExampleMandA &&e) noexcept { + print_move_assigned(this); + value = e.value; + } + // NOLINTNEXTLINE(performance-unnecessary-value-param) void add1(ExampleMandA other) { value += other.value; } // passing by value void add2(ExampleMandA &other) { value += other.value; } // passing by reference void add3(const ExampleMandA &other) { value += other.value; } // passing by const reference @@ -112,7 +116,7 @@ int none1(const NoneTester &obj) { return obj.answer; } int none2(NoneTester *obj) { return obj ? obj->answer : -1; } int none3(std::shared_ptr &obj) { return obj ? obj->answer : -1; } int none4(std::shared_ptr *obj) { return obj && *obj ? (*obj)->answer : -1; } -int none5(std::shared_ptr obj) { return obj ? obj->answer : -1; } +int none5(const std::shared_ptr &obj) { return obj ? obj->answer : -1; } struct StrIssue { int val = -1; @@ -226,36 +230,41 @@ TEST_SUBMODULE(methods_and_attributes, m) { .def(py::init<>()) .def_readonly("def_readonly", &TestProperties::value) .def_readwrite("def_readwrite", &TestProperties::value) - .def_property("def_writeonly", nullptr, - [](TestProperties& s,int v) { s.value = v; } ) + .def_property("def_writeonly", nullptr, [](TestProperties &s, int v) { s.value = v; }) .def_property("def_property_writeonly", nullptr, &TestProperties::set) .def_property_readonly("def_property_readonly", &TestProperties::get) .def_property("def_property", &TestProperties::get, &TestProperties::set) .def_property("def_property_impossible", nullptr, nullptr) .def_readonly_static("def_readonly_static", &TestProperties::static_value) .def_readwrite_static("def_readwrite_static", &TestProperties::static_value) - .def_property_static("def_writeonly_static", nullptr, - [](py::object, int v) { TestProperties::static_value = v; }) - .def_property_readonly_static("def_property_readonly_static", - [](py::object) { return TestProperties::static_get(); }) - .def_property_static("def_property_writeonly_static", nullptr, - [](py::object, int v) { return TestProperties::static_set(v); }) - .def_property_static("def_property_static", - [](py::object) { return TestProperties::static_get(); }, - [](py::object, int v) { TestProperties::static_set(v); }) - .def_property_static("static_cls", - [](py::object cls) { return cls; }, - [](py::object cls, py::function f) { f(cls); }); + .def_property_static("def_writeonly_static", + nullptr, + [](const py::object &, int v) { TestProperties::static_value = v; }) + .def_property_readonly_static( + "def_property_readonly_static", + [](const py::object &) { return TestProperties::static_get(); }) + .def_property_static( + "def_property_writeonly_static", + nullptr, + [](const py::object &, int v) { return TestProperties::static_set(v); }) + .def_property_static( + "def_property_static", + [](const py::object &) { return TestProperties::static_get(); }, + [](const py::object &, int v) { TestProperties::static_set(v); }) + .def_property_static( + "static_cls", + [](py::object cls) { return cls; }, + [](const py::object &cls, const py::function &f) { f(cls); }); py::class_(m, "TestPropertiesOverride") .def(py::init<>()) .def_readonly("def_readonly", &TestPropertiesOverride::value) .def_readonly_static("def_readonly_static", &TestPropertiesOverride::static_value); - auto static_get1 = [](py::object) -> const UserType & { return TestPropRVP::sv1; }; - auto static_get2 = [](py::object) -> const UserType & { return TestPropRVP::sv2; }; - auto static_set1 = [](py::object, int v) { TestPropRVP::sv1.set(v); }; - auto static_set2 = [](py::object, int v) { TestPropRVP::sv2.set(v); }; + auto static_get1 = [](const py::object &) -> const UserType & { return TestPropRVP::sv1; }; + auto static_get2 = [](const py::object &) -> const UserType & { return TestPropRVP::sv2; }; + auto static_set1 = [](const py::object &, int v) { TestPropRVP::sv1.set(v); }; + auto static_set2 = [](const py::object &, int v) { TestPropRVP::sv2.set(v); }; auto rvp_copy = py::return_value_policy::copy; // test_property_return_value_policies @@ -266,24 +275,30 @@ TEST_SUBMODULE(methods_and_attributes, m) { .def_property_readonly("ro_func", py::cpp_function(&TestPropRVP::get2, rvp_copy)) .def_property("rw_ref", &TestPropRVP::get1, &TestPropRVP::set1) .def_property("rw_copy", &TestPropRVP::get2, &TestPropRVP::set2, rvp_copy) - .def_property("rw_func", py::cpp_function(&TestPropRVP::get2, rvp_copy), &TestPropRVP::set2) + .def_property( + "rw_func", py::cpp_function(&TestPropRVP::get2, rvp_copy), &TestPropRVP::set2) .def_property_readonly_static("static_ro_ref", static_get1) .def_property_readonly_static("static_ro_copy", static_get2, rvp_copy) .def_property_readonly_static("static_ro_func", py::cpp_function(static_get2, rvp_copy)) .def_property_static("static_rw_ref", static_get1, static_set1) .def_property_static("static_rw_copy", static_get2, static_set2, rvp_copy) - .def_property_static("static_rw_func", py::cpp_function(static_get2, rvp_copy), static_set2) + .def_property_static( + "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); }); // 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; }); // 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", [](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 daecb9564..f4acf7861 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -141,6 +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(); }); // test_mi_unaligned_base diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index 204ea8367..a6c7ae8d7 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -13,6 +13,7 @@ #include #include +#include // Size / dtype checks. struct DtypeCheck { @@ -192,7 +193,7 @@ TEST_SUBMODULE(numpy_array, sm) { sm.def("scalar_int", []() { return py::array(py::dtype("i"), {}, {}, &data_i); }); // test_wrap - sm.def("wrap", [](py::array a) { + sm.def("wrap", [](const py::array &a) { return py::array( a.dtype(), {a.shape(), a.shape() + a.ndim()}, @@ -222,8 +223,10 @@ TEST_SUBMODULE(numpy_array, sm) { // test_isinstance sm.def("isinstance_untyped", [](py::object yes, py::object no) { - return py::isinstance(yes) && !py::isinstance(no); + return py::isinstance(std::move(yes)) + && !py::isinstance(std::move(no)); }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) sm.def("isinstance_typed", [](py::object o) { return py::isinstance>(o) && !py::isinstance>(o); }); @@ -236,7 +239,7 @@ TEST_SUBMODULE(numpy_array, sm) { "array_t"_a=py::array_t() ); }); - sm.def("converting_constructors", [](py::object o) { + sm.def("converting_constructors", [](const py::object &o) { return py::dict( "array"_a=py::array(o), "array_t"_a=py::array_t(o), @@ -245,39 +248,59 @@ 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"; }); + // 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"; }); // [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()); // 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"; }); // 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"; }); // 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"; }); // test_array_unchecked_fixed_dims @@ -306,7 +329,7 @@ TEST_SUBMODULE(numpy_array, sm) { r(i, j, k) = start++; return a; }); - sm.def("proxy_squared_L2_norm", [](py::array_t a) { + sm.def("proxy_squared_L2_norm", [](const py::array_t &a) { auto r = a.unchecked<1>(); double sumsq = 0; for (py::ssize_t i = 0; i < r.shape(0); i++) @@ -396,51 +419,78 @@ TEST_SUBMODULE(numpy_array, sm) { return a; }); - sm.def("index_using_ellipsis", [](py::array a) { - return a[py::make_tuple(0, py::ellipsis(), 0)]; - }); + sm.def("index_using_ellipsis", + [](const py::array &a) { return a[py::make_tuple(0, py::ellipsis(), 0)]; }); // test_argument_conversions - sm.def("accept_double", - [](py::array_t) {}, - py::arg("a")); - sm.def("accept_double_forcecast", - [](py::array_t) {}, - py::arg("a")); - sm.def("accept_double_c_style", - [](py::array_t) {}, - py::arg("a")); - sm.def("accept_double_c_style_forcecast", - [](py::array_t) {}, - py::arg("a")); - sm.def("accept_double_f_style", - [](py::array_t) {}, - py::arg("a")); - sm.def("accept_double_f_style_forcecast", - [](py::array_t) {}, - py::arg("a")); - sm.def("accept_double_noconvert", - [](py::array_t) {}, - "a"_a.noconvert()); - sm.def("accept_double_forcecast_noconvert", - [](py::array_t) {}, - "a"_a.noconvert()); - sm.def("accept_double_c_style_noconvert", - [](py::array_t) {}, - "a"_a.noconvert()); - sm.def("accept_double_c_style_forcecast_noconvert", - [](py::array_t) {}, - "a"_a.noconvert()); - sm.def("accept_double_f_style_noconvert", - [](py::array_t) {}, - "a"_a.noconvert()); - sm.def("accept_double_f_style_forcecast_noconvert", - [](py::array_t) {}, - "a"_a.noconvert()); + sm.def( + "accept_double", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + py::arg("a")); + sm.def( + "accept_double_forcecast", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + py::arg("a")); + sm.def( + "accept_double_c_style", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + py::arg("a")); + sm.def( + "accept_double_c_style_forcecast", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + py::arg("a")); + sm.def( + "accept_double_f_style", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + py::arg("a")); + sm.def( + "accept_double_f_style_forcecast", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + py::arg("a")); + sm.def( + "accept_double_noconvert", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + "a"_a.noconvert()); + sm.def( + "accept_double_forcecast_noconvert", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + "a"_a.noconvert()); + sm.def( + "accept_double_c_style_noconvert", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + "a"_a.noconvert()); + sm.def( + "accept_double_c_style_forcecast_noconvert", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + "a"_a.noconvert()); + sm.def( + "accept_double_f_style_noconvert", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](py::array_t) {}, + "a"_a.noconvert()); + sm.def( + "accept_double_f_style_forcecast_noconvert", + // NOLINTNEXTLINE(performance-unnecessary-value-param) + [](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) {}); } diff --git a/tests/test_numpy_dtypes.cpp b/tests/test_numpy_dtypes.cpp index 9dece73ec..edf9cf200 100644 --- a/tests/test_numpy_dtypes.cpp +++ b/tests/test_numpy_dtypes.cpp @@ -266,10 +266,11 @@ TEST_SUBMODULE(numpy_dtypes, m) { .def_readwrite("uint_", &SimpleStruct::uint_) .def_readwrite("float_", &SimpleStruct::float_) .def_readwrite("ldbl_", &SimpleStruct::ldbl_) - .def("astuple", [](const SimpleStruct& self) { - return py::make_tuple(self.bool_, self.uint_, self.float_, self.ldbl_); - }) - .def_static("fromtuple", [](const py::tuple tup) { + .def("astuple", + [](const SimpleStruct &self) { + return py::make_tuple(self.bool_, self.uint_, self.float_, self.ldbl_); + }) + .def_static("fromtuple", [](const py::tuple &tup) { if (py::len(tup) != 4) { throw py::cast_error("Invalid size"); } diff --git a/tests/test_numpy_vectorize.cpp b/tests/test_numpy_vectorize.cpp index 8ed9b27e1..77e281bbd 100644 --- a/tests/test_numpy_vectorize.cpp +++ b/tests/test_numpy_vectorize.cpp @@ -11,6 +11,8 @@ #include "pybind11_tests.h" #include +#include + double my_func(int x, float y, double z) { py::print("my_func(x:int={}, y:float={:.0f}, z:float={:.0f})"_s.format(x, y, z)); return (float) x*y*z; @@ -25,11 +27,10 @@ TEST_SUBMODULE(numpy_vectorize, m) { m.def("vectorized_func", py::vectorize(my_func)); // Vectorize a lambda function with a capture object (e.g. to exclude some arguments from the vectorization) - m.def("vectorized_func2", - [](py::array_t x, py::array_t y, float z) { - return py::vectorize([z](int x, float y) { return my_func(x, y, z); })(x, y); - } - ); + m.def("vectorized_func2", [](py::array_t x, py::array_t y, float z) { + return py::vectorize([z](int x, float y) { return my_func(x, y, z); })(std::move(x), + std::move(y)); + }); // Vectorize a complex-valued function m.def("vectorized_func3", py::vectorize( @@ -38,8 +39,12 @@ TEST_SUBMODULE(numpy_vectorize, m) { // test_type_selection // NumPy function which only accepts specific data types + // Alot 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."; }); @@ -53,11 +58,16 @@ TEST_SUBMODULE(numpy_vectorize, m) { py::class_(m, "NonPODClass") .def(py::init()) .def_readwrite("value", &NonPODClass::value); - m.def("vec_passthrough", py::vectorize( - [](double *a, double b, py::array_t c, const int &d, int &e, NonPODClass f, const double g) { - return *a + b + c.at(0) + d + e + f.value + g; - } - )); + m.def("vec_passthrough", + py::vectorize([](double *a, + double b, + // Changing this broke things + // NOLINTNEXTLINE(performance-unnecessary-value-param) + py::array_t c, + const int &d, + int &e, + NonPODClass f, + const double g) { return *a + b + c.at(0) + d + e + f.value + g; })); // test_method_vectorization struct VectorizeTestClass { @@ -78,16 +88,20 @@ TEST_SUBMODULE(numpy_vectorize, m) { .value("f_trivial", py::detail::broadcast_trivial::f_trivial) .value("c_trivial", py::detail::broadcast_trivial::c_trivial) .value("non_trivial", py::detail::broadcast_trivial::non_trivial); - m.def("vectorized_is_trivial", []( - py::array_t arg1, - py::array_t arg2, - py::array_t arg3 - ) { - py::ssize_t ndim; - std::vector shape; - std::array buffers {{ arg1.request(), arg2.request(), arg3.request() }}; - return py::detail::broadcast(buffers, ndim, shape); - }); + 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) { + py::ssize_t ndim; + std::vector shape; + std::array buffers{ + {arg1.request(), arg2.request(), arg3.request()}}; + return py::detail::broadcast(buffers, ndim, shape); + }); m.def("add_to", py::vectorize([](NonPODClass& x, int a) { x.value += a; })); } diff --git a/tests/test_opaque_types.cpp b/tests/test_opaque_types.cpp index 5a2343163..804de6d4f 100644 --- a/tests/test_opaque_types.cpp +++ b/tests/test_opaque_types.cpp @@ -44,7 +44,7 @@ TEST_SUBMODULE(opaque_types, m) { m.def("print_opaque_list", [](const StringList &l) { std::string ret = "Opaque list: ["; bool first = true; - for (auto entry : l) { + for (const auto &entry : l) { if (!first) ret += ", "; ret += entry; diff --git a/tests/test_operator_overloading.cpp b/tests/test_operator_overloading.cpp index eb3167dc0..0b6c496cf 100644 --- a/tests/test_operator_overloading.cpp +++ b/tests/test_operator_overloading.cpp @@ -16,9 +16,18 @@ class Vector2 { public: Vector2(float x, float y) : x(x), y(y) { print_created(this, toString()); } Vector2(const Vector2 &v) : x(v.x), y(v.y) { print_copy_created(this); } - Vector2(Vector2 &&v) : x(v.x), y(v.y) { print_move_created(this); v.x = v.y = 0; } + Vector2(Vector2 &&v) noexcept : x(v.x), y(v.y) { + print_move_created(this); + v.x = v.y = 0; + } Vector2 &operator=(const Vector2 &v) { x = v.x; y = v.y; print_copy_assigned(this); return *this; } - Vector2 &operator=(Vector2 &&v) { x = v.x; y = v.y; v.x = v.y = 0; print_move_assigned(this); return *this; } + Vector2 &operator=(Vector2 &&v) noexcept { + x = v.x; + y = v.y; + v.x = v.y = 0; + print_move_assigned(this); + return *this; + } ~Vector2() { print_destroyed(this); } std::string toString() const { return "[" + std::to_string(x) + ", " + std::to_string(y) + "]"; } diff --git a/tests/test_pickling.cpp b/tests/test_pickling.cpp index 1a48595af..c2cee6c3c 100644 --- a/tests/test_pickling.cpp +++ b/tests/test_pickling.cpp @@ -46,17 +46,16 @@ TEST_SUBMODULE(pickling, m) { return py::make_tuple(p.value(), p.extra1(), p.extra2()); }); ignoreOldStyleInitWarnings([&pyPickleable]() { - pyPickleable - .def("__setstate__", [](Pickleable &p, py::tuple t) { - if (t.size() != 3) - throw std::runtime_error("Invalid state!"); - /* Invoke the constructor (need to use in-place version) */ - new (&p) Pickleable(t[0].cast()); + pyPickleable.def("__setstate__", [](Pickleable &p, const py::tuple &t) { + if (t.size() != 3) + throw std::runtime_error("Invalid state!"); + /* Invoke the constructor (need to use in-place version) */ + new (&p) Pickleable(t[0].cast()); - /* Assign any additional state */ - p.setExtra1(t[1].cast()); - p.setExtra2(t[2].cast()); - }); + /* Assign any additional state */ + p.setExtra1(t[1].cast()); + p.setExtra2(t[2].cast()); + }); }); py::class_(m, "PickleableNew") @@ -65,7 +64,7 @@ TEST_SUBMODULE(pickling, m) { [](const PickleableNew &p) { return py::make_tuple(p.value(), p.extra1(), p.extra2()); }, - [](py::tuple t) { + [](const py::tuple &t) { if (t.size() != 3) throw std::runtime_error("Invalid state!"); auto p = PickleableNew(t[0].cast()); @@ -73,8 +72,7 @@ TEST_SUBMODULE(pickling, m) { p.setExtra1(t[1].cast()); p.setExtra2(t[2].cast()); return p; - } - )); + })); #if !defined(PYPY_VERSION) // test_roundtrip_with_dict @@ -92,35 +90,33 @@ TEST_SUBMODULE(pickling, m) { }; py::class_ pyPickleableWithDict(m, "PickleableWithDict", py::dynamic_attr()); - pyPickleableWithDict - .def(py::init()) + pyPickleableWithDict.def(py::init()) .def_readwrite("value", &PickleableWithDict::value) .def_readwrite("extra", &PickleableWithDict::extra) - .def("__getstate__", [](py::object self) { + .def("__getstate__", [](const py::object &self) { /* Also include __dict__ in state */ return py::make_tuple(self.attr("value"), self.attr("extra"), self.attr("__dict__")); }); ignoreOldStyleInitWarnings([&pyPickleableWithDict]() { - pyPickleableWithDict - .def("__setstate__", [](py::object self, py::tuple t) { - if (t.size() != 3) - throw std::runtime_error("Invalid state!"); - /* Cast and construct */ - auto& p = self.cast(); - new (&p) PickleableWithDict(t[0].cast()); + pyPickleableWithDict.def("__setstate__", [](const py::object &self, const py::tuple &t) { + if (t.size() != 3) + throw std::runtime_error("Invalid state!"); + /* Cast and construct */ + auto &p = self.cast(); + new (&p) PickleableWithDict(t[0].cast()); - /* Assign C++ state */ - p.extra = t[1].cast(); + /* Assign C++ state */ + p.extra = t[1].cast(); - /* Assign Python state */ - self.attr("__dict__") = t[2]; - }); + /* Assign Python state */ + self.attr("__dict__") = t[2]; + }); }); py::class_(m, "PickleableWithDictNew") .def(py::init()) .def(py::pickle( - [](py::object self) { + [](const py::object &self) { return py::make_tuple(self.attr("value"), self.attr("extra"), self.attr("__dict__")); }, [](const py::tuple &t) { @@ -132,7 +128,6 @@ TEST_SUBMODULE(pickling, m) { auto py_state = t[2].cast(); return std::make_pair(cpp_state, py_state); - } - )); + })); #endif } diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 6921796aa..9e0ebc206 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -7,6 +7,8 @@ BSD-style license that can be found in the LICENSE file. */ +#include + #include "pybind11_tests.h" @@ -27,16 +29,14 @@ TEST_SUBMODULE(pytypes, m) { list.insert(2, "inserted-2"); return list; }); - m.def("print_list", [](py::list list) { + m.def("print_list", [](const py::list &list) { int index = 0; for (auto item : list) py::print("list item {}: {}"_s.format(index++, item)); }); // test_none m.def("get_none", []{return py::none();}); - m.def("print_none", [](py::none none) { - py::print("none: {}"_s.format(none)); - }); + m.def("print_none", [](const py::none &none) { py::print("none: {}"_s.format(none)); }); // test_set m.def("get_set", []() { @@ -46,20 +46,17 @@ TEST_SUBMODULE(pytypes, m) { set.add(std::string("key3")); return set; }); - m.def("print_set", [](py::set set) { + m.def("print_set", [](const py::set &set) { for (auto item : set) py::print("key:", item); }); - m.def("set_contains", [](py::set set, py::object key) { - return set.contains(key); - }); - m.def("set_contains", [](py::set set, const char* key) { - return set.contains(key); - }); + m.def("set_contains", + [](const py::set &set, const py::object &key) { return set.contains(key); }); + m.def("set_contains", [](const py::set &set, const char *key) { return set.contains(key); }); // test_dict m.def("get_dict", []() { return py::dict("key"_a="value"); }); - m.def("print_dict", [](py::dict dict) { + m.def("print_dict", [](const py::dict &dict) { for (auto item : dict) py::print("key: {}, value={}"_s.format(item.first, item.second)); }); @@ -68,12 +65,10 @@ TEST_SUBMODULE(pytypes, m) { auto d2 = py::dict("z"_a=3, **d1); return d2; }); - m.def("dict_contains", [](py::dict dict, py::object val) { - return dict.contains(val); - }); - m.def("dict_contains", [](py::dict dict, const char* val) { - return dict.contains(val); - }); + m.def("dict_contains", + [](const py::dict &dict, py::object val) { return dict.contains(val); }); + m.def("dict_contains", + [](const py::dict &dict, const char *val) { return dict.contains(val); }); // test_str m.def("str_from_string", []() { return py::str(std::string("baz")); }); @@ -137,7 +132,7 @@ TEST_SUBMODULE(pytypes, m) { }); // test_accessors - m.def("accessor_api", [](py::object o) { + m.def("accessor_api", [](const py::object &o) { auto d = py::dict(); d["basic_attr"] = o.attr("basic_attr"); @@ -178,7 +173,7 @@ TEST_SUBMODULE(pytypes, m) { return d; }); - m.def("tuple_accessor", [](py::tuple existing_t) { + m.def("tuple_accessor", [](const py::tuple &existing_t) { try { existing_t[0] = 1; } catch (const py::error_already_set &) { @@ -226,7 +221,7 @@ TEST_SUBMODULE(pytypes, m) { ); }); - m.def("converting_constructors", [](py::dict d) { + m.def("converting_constructors", [](const py::dict &d) { return py::dict( "bytes"_a=py::bytes(d["bytes"]), "bytearray"_a=py::bytearray(d["bytearray"]), @@ -242,6 +237,7 @@ TEST_SUBMODULE(pytypes, m) { ); }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("cast_functions", [](py::dict d) { // When converting between Python types, obj.cast() should be the same as T(obj) return py::dict( @@ -259,8 +255,10 @@ TEST_SUBMODULE(pytypes, m) { ); }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("convert_to_pybind11_str", [](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); @@ -333,8 +331,9 @@ TEST_SUBMODULE(pytypes, m) { m.def("print_failure", []() { py::print(42, UnregisteredType()); }); - m.def("hash_function", [](py::object obj) { return py::hash(obj); }); + 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) { py::list l; l.append(a.equal(b)); @@ -355,6 +354,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)]; }); @@ -369,10 +369,12 @@ 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); }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("test_memoryview_buffer_info", [](py::buffer b) { return py::memoryview(b.request()); }); @@ -425,20 +427,22 @@ TEST_SUBMODULE(pytypes, m) { m.attr("PYBIND11_STR_LEGACY_PERMISSIVE") = true; #endif - m.def("isinstance_pybind11_bytes", [](py::object o) { return py::isinstance(o); }); - m.def("isinstance_pybind11_str", [](py::object o) { return py::isinstance(o); }); + m.def("isinstance_pybind11_bytes", + [](py::object o) { return py::isinstance(std::move(o)); }); + m.def("isinstance_pybind11_str", + [](py::object o) { return py::isinstance(std::move(o)); }); - m.def("pass_to_pybind11_bytes", [](py::bytes b) { return py::len(b); }); - m.def("pass_to_pybind11_str", [](py::str s) { return py::len(s); }); + 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(); }); // test_weakref m.def("weakref_from_handle", [](py::handle h) { return py::weakref(h); }); m.def("weakref_from_handle_and_function", - [](py::handle h, py::function f) { return py::weakref(h, f); }); - m.def("weakref_from_object", - [](py::object o) { return py::weakref(o); }); + [](py::handle h, py::function f) { return py::weakref(h, std::move(f)); }); + m.def("weakref_from_object", [](const py::object &o) { return py::weakref(o); }); m.def("weakref_from_object_and_function", - [](py::object o, py::function f) { return py::weakref(o, f); }); + [](py::object o, py::function f) { return py::weakref(std::move(o), std::move(f)); }); } diff --git a/tests/test_sequences_and_iterators.cpp b/tests/test_sequences_and_iterators.cpp index d318052a4..c39798229 100644 --- a/tests/test_sequences_and_iterators.cpp +++ b/tests/test_sequences_and_iterators.cpp @@ -14,6 +14,7 @@ #include #include +#include template class NonZeroIterator { @@ -80,18 +81,17 @@ TEST_SUBMODULE(sequences_and_iterators, m) { int start,stop,step; int size; }; - py::class_(m,"Sliceable") + py::class_(m, "Sliceable") .def(py::init()) - .def("__getitem__",[](const Sliceable &s, py::slice slice) { - py::ssize_t start, stop, step, slicelength; - if (!slice.compute(s.size, &start, &stop, &step, &slicelength)) - throw py::error_already_set(); - int istart = static_cast(start); - int istop = static_cast(stop); - int istep = static_cast(step); - return std::make_tuple(istart,istop,istep); - }) - ; + .def("__getitem__", [](const Sliceable &s, const py::slice &slice) { + py::ssize_t start, stop, step, slicelength; + if (!slice.compute(s.size, &start, &stop, &step, &slicelength)) + throw py::error_already_set(); + int istart = static_cast(start); + int istop = static_cast(stop); + int istep = static_cast(step); + return std::make_tuple(istart, istop, istep); + }); // test_sequence class Sequence { @@ -111,7 +111,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) { m_data = new float[m_size]; memcpy(m_data, s.m_data, sizeof(float)*m_size); } - Sequence(Sequence &&s) : m_size(s.m_size), m_data(s.m_data) { + Sequence(Sequence &&s) noexcept : m_size(s.m_size), m_data(s.m_data) { print_move_created(this); s.m_size = 0; s.m_data = nullptr; @@ -130,7 +130,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) { return *this; } - Sequence &operator=(Sequence &&s) { + Sequence &operator=(Sequence &&s) noexcept { if (&s != this) { delete[] m_data; m_size = s.m_size; @@ -179,43 +179,54 @@ TEST_SUBMODULE(sequences_and_iterators, m) { }; py::class_(m, "Sequence") .def(py::init()) - .def(py::init&>()) + .def(py::init &>()) /// Bare bones interface - .def("__getitem__", [](const Sequence &s, size_t i) { - if (i >= s.size()) throw py::index_error(); - return s[i]; - }) - .def("__setitem__", [](Sequence &s, size_t i, float v) { - if (i >= s.size()) throw py::index_error(); - s[i] = v; - }) + .def("__getitem__", + [](const Sequence &s, size_t i) { + if (i >= s.size()) + throw py::index_error(); + return s[i]; + }) + .def("__setitem__", + [](Sequence &s, size_t i, float v) { + if (i >= s.size()) + throw py::index_error(); + s[i] = v; + }) .def("__len__", &Sequence::size) /// Optional sequence protocol operations - .def("__iter__", [](const Sequence &s) { return py::make_iterator(s.begin(), s.end()); }, - py::keep_alive<0, 1>() /* Essential: keep object alive while iterator exists */) + .def( + "__iter__", + [](const Sequence &s) { return py::make_iterator(s.begin(), s.end()); }, + py::keep_alive<0, 1>() /* Essential: keep object alive while iterator exists */) .def("__contains__", [](const Sequence &s, float v) { return s.contains(v); }) .def("__reversed__", [](const Sequence &s) -> Sequence { return s.reversed(); }) /// Slicing protocol (optional) - .def("__getitem__", [](const Sequence &s, py::slice slice) -> Sequence* { - size_t start, stop, step, slicelength; - if (!slice.compute(s.size(), &start, &stop, &step, &slicelength)) - throw py::error_already_set(); - auto *seq = new Sequence(slicelength); - for (size_t i = 0; i < slicelength; ++i) { - (*seq)[i] = s[start]; start += step; - } - return seq; - }) - .def("__setitem__", [](Sequence &s, py::slice slice, const Sequence &value) { - size_t start, stop, step, slicelength; - if (!slice.compute(s.size(), &start, &stop, &step, &slicelength)) - throw py::error_already_set(); - if (slicelength != value.size()) - throw std::runtime_error("Left and right hand size of slice assignment have different sizes!"); - for (size_t i = 0; i < slicelength; ++i) { - s[start] = value[i]; start += step; - } - }) + .def("__getitem__", + [](const Sequence &s, const py::slice &slice) -> Sequence * { + size_t start, stop, step, slicelength; + if (!slice.compute(s.size(), &start, &stop, &step, &slicelength)) + throw py::error_already_set(); + auto *seq = new Sequence(slicelength); + for (size_t i = 0; i < slicelength; ++i) { + (*seq)[i] = s[start]; + start += step; + } + return seq; + }) + .def("__setitem__", + [](Sequence &s, const py::slice &slice, const Sequence &value) { + size_t start, stop, step, slicelength; + if (!slice.compute(s.size(), &start, &stop, &step, &slicelength)) + throw py::error_already_set(); + if (slicelength != value.size()) + throw std::runtime_error( + "Left and right hand size of slice assignment have different sizes!"); + for (size_t i = 0; i < slicelength; ++i) { + s[start] = value[i]; + start += step; + } + }) /// Comparisons .def(py::self == py::self) .def(py::self != py::self) @@ -231,8 +242,8 @@ TEST_SUBMODULE(sequences_and_iterators, m) { StringMap(std::unordered_map init) : map(std::move(init)) {} - void set(std::string key, std::string val) { map[key] = val; } - std::string get(std::string key) const { return map.at(key); } + void set(const std::string &key, std::string val) { map[key] = std::move(val); } + std::string get(const std::string &key) const { return map.at(key); } size_t size() const { return map.size(); } private: std::unordered_map map; @@ -243,19 +254,24 @@ TEST_SUBMODULE(sequences_and_iterators, m) { py::class_(m, "StringMap") .def(py::init<>()) .def(py::init>()) - .def("__getitem__", [](const StringMap &map, std::string key) { - try { return map.get(key); } - catch (const std::out_of_range&) { - throw py::key_error("key '" + key + "' does not exist"); - } - }) + .def("__getitem__", + [](const StringMap &map, const std::string &key) { + try { + return map.get(key); + } catch (const std::out_of_range &) { + throw py::key_error("key '" + key + "' does not exist"); + } + }) .def("__setitem__", &StringMap::set) .def("__len__", &StringMap::size) - .def("__iter__", [](const StringMap &map) { return py::make_key_iterator(map.begin(), map.end()); }, - py::keep_alive<0, 1>()) - .def("items", [](const StringMap &map) { return py::make_iterator(map.begin(), map.end()); }, - py::keep_alive<0, 1>()) - ; + .def( + "__iter__", + [](const StringMap &map) { return py::make_key_iterator(map.begin(), map.end()); }, + py::keep_alive<0, 1>()) + .def( + "items", + [](const StringMap &map) { return py::make_iterator(map.begin(), map.end()); }, + py::keep_alive<0, 1>()); // test_generalized_iterators class IntPairs { @@ -304,7 +320,7 @@ TEST_SUBMODULE(sequences_and_iterators, m) { #endif // test_python_iterator_in_cpp - m.def("object_to_list", [](py::object o) { + m.def("object_to_list", [](const py::object &o) { auto l = py::list(); for (auto item : o) { l.append(item); @@ -322,22 +338,22 @@ TEST_SUBMODULE(sequences_and_iterators, m) { }); // test_sequence_length: check that Python sequences can be converted to py::sequence. - m.def("sequence_length", [](py::sequence seq) { return seq.size(); }); + m.def("sequence_length", [](const py::sequence &seq) { return seq.size(); }); // Make sure that py::iterator works with std algorithms - m.def("count_none", [](py::object o) { + m.def("count_none", [](const py::object &o) { return std::count_if(o.begin(), o.end(), [](py::handle h) { return h.is_none(); }); }); - m.def("find_none", [](py::object o) { + m.def("find_none", [](const py::object &o) { auto it = std::find_if(o.begin(), o.end(), [](py::handle h) { return h.is_none(); }); return it->is_none(); }); - m.def("count_nonzeros", [](py::dict d) { - return std::count_if(d.begin(), d.end(), [](std::pair p) { - return p.second.cast() != 0; - }); + m.def("count_nonzeros", [](const py::dict &d) { + return std::count_if(d.begin(), d.end(), [](std::pair p) { + return p.second.cast() != 0; + }); }); m.def("tuple_iterator", &test_random_access_iterator); diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 2ece3bf0c..534dec0be 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -170,7 +170,7 @@ struct SharedPtrRef { struct A { A() { print_created(this); } A(const A &) { print_copy_created(this); } - A(A &&) { print_move_created(this); } + A(A &&) noexcept { print_move_created(this); } ~A() { print_destroyed(this); } }; @@ -183,7 +183,7 @@ struct SharedFromThisRef { struct B : std::enable_shared_from_this { B() { print_created(this); } B(const B &) : std::enable_shared_from_this() { print_copy_created(this); } - B(B &&) : std::enable_shared_from_this() { print_move_created(this); } + B(B &&) noexcept : std::enable_shared_from_this() { print_move_created(this); } ~B() { print_destroyed(this); } }; @@ -209,7 +209,9 @@ struct C { struct TypeForHolderWithAddressOf { TypeForHolderWithAddressOf() { print_created(this); } TypeForHolderWithAddressOf(const TypeForHolderWithAddressOf &) { print_copy_created(this); } - TypeForHolderWithAddressOf(TypeForHolderWithAddressOf &&) { print_move_created(this); } + TypeForHolderWithAddressOf(TypeForHolderWithAddressOf &&) noexcept { + print_move_created(this); + } ~TypeForHolderWithAddressOf() { print_destroyed(this); } std::string toString() const { return "TypeForHolderWithAddressOf[" + std::to_string(value) + "]"; @@ -245,7 +247,7 @@ struct ElementA : ElementBase { }; struct ElementList { - void add(std::shared_ptr e) { l.push_back(e); } + void add(const std::shared_ptr &e) { l.push_back(e); } std::vector> l; }; @@ -308,6 +310,7 @@ TEST_SUBMODULE(smart_ptr, m) { m.def("make_myobject2_1", []() { return new MyObject2(6); }); m.def("make_myobject2_2", []() { return std::make_shared(7); }); m.def("print_myobject2_1", [](const MyObject2 *obj) { py::print(obj->toString()); }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("print_myobject2_2", [](std::shared_ptr obj) { py::print(obj->toString()); }); m.def("print_myobject2_3", [](const std::shared_ptr &obj) { py::print(obj->toString()); }); m.def("print_myobject2_4", [](const std::shared_ptr *obj) { py::print((*obj)->toString()); }); @@ -317,6 +320,7 @@ TEST_SUBMODULE(smart_ptr, m) { m.def("make_myobject3_1", []() { return new MyObject3(8); }); m.def("make_myobject3_2", []() { return std::make_shared(9); }); m.def("print_myobject3_1", [](const MyObject3 *obj) { py::print(obj->toString()); }); + // NOLINTNEXTLINE(performance-unnecessary-value-param) m.def("print_myobject3_2", [](std::shared_ptr obj) { py::print(obj->toString()); }); m.def("print_myobject3_3", [](const std::shared_ptr &obj) { py::print(obj->toString()); }); m.def("print_myobject3_4", [](const std::shared_ptr *obj) { py::print((*obj)->toString()); }); @@ -358,12 +362,15 @@ TEST_SUBMODULE(smart_ptr, m) { py::class_>(m, "SharedPtrRef") .def(py::init<>()) .def_readonly("ref", &SharedPtrRef::value) - .def_property_readonly("copy", [](const SharedPtrRef &s) { return s.value; }, - py::return_value_policy::copy) + .def_property_readonly( + "copy", [](const SharedPtrRef &s) { return s.value; }, py::return_value_policy::copy) .def_readonly("holder_ref", &SharedPtrRef::shared) - .def_property_readonly("holder_copy", [](const SharedPtrRef &s) { return s.shared; }, - py::return_value_policy::copy) + .def_property_readonly( + "holder_copy", + [](const SharedPtrRef &s) { return s.shared; }, + py::return_value_policy::copy) .def("set_ref", [](SharedPtrRef &, const A &) { return true; }) + // NOLINTNEXTLINE(performance-unnecessary-value-param) .def("set_holder", [](SharedPtrRef &, std::shared_ptr) { return true; }); // test_shared_ptr_from_this_and_references @@ -372,13 +379,19 @@ TEST_SUBMODULE(smart_ptr, m) { py::class_>(m, "SharedFromThisRef") .def(py::init<>()) .def_readonly("bad_wp", &SharedFromThisRef::value) - .def_property_readonly("ref", [](const SharedFromThisRef &s) -> const B & { return *s.shared; }) - .def_property_readonly("copy", [](const SharedFromThisRef &s) { return s.value; }, - py::return_value_policy::copy) + .def_property_readonly("ref", + [](const SharedFromThisRef &s) -> const B & { return *s.shared; }) + .def_property_readonly( + "copy", + [](const SharedFromThisRef &s) { return s.value; }, + py::return_value_policy::copy) .def_readonly("holder_ref", &SharedFromThisRef::shared) - .def_property_readonly("holder_copy", [](const SharedFromThisRef &s) { return s.shared; }, - py::return_value_policy::copy) + .def_property_readonly( + "holder_copy", + [](const SharedFromThisRef &s) { return s.shared; }, + py::return_value_policy::copy) .def("set_ref", [](SharedFromThisRef &, const B &) { return true; }) + // NOLINTNEXTLINE(performance-unnecessary-value-param) .def("set_holder", [](SharedFromThisRef &, std::shared_ptr) { return true; }); // Issue #865: shared_from_this doesn't work with virtual inheritance @@ -396,10 +409,14 @@ TEST_SUBMODULE(smart_ptr, m) { py::class_(m, "TypeForHolderWithAddressOf") .def_static("make", []() { return HolderWithAddressOf(new TypeForHolderWithAddressOf); }) .def("get", [](const HolderWithAddressOf &self) { return self.get(); }) - .def("print_object_1", [](const TypeForHolderWithAddressOf *obj) { py::print(obj->toString()); }) + .def("print_object_1", + [](const TypeForHolderWithAddressOf *obj) { py::print(obj->toString()); }) + // NOLINTNEXTLINE(performance-unnecessary-value-param) .def("print_object_2", [](HolderWithAddressOf obj) { py::print(obj.get()->toString()); }) - .def("print_object_3", [](const HolderWithAddressOf &obj) { py::print(obj.get()->toString()); }) - .def("print_object_4", [](const HolderWithAddressOf *obj) { py::print((*obj).get()->toString()); }); + .def("print_object_3", + [](const HolderWithAddressOf &obj) { py::print(obj.get()->toString()); }) + .def("print_object_4", + [](const HolderWithAddressOf *obj) { py::print((*obj).get()->toString()); }); // test_move_only_holder_with_addressof_operator using MoveOnlyHolderWithAddressOf = unique_ptr_with_addressof_operator; @@ -411,6 +428,7 @@ TEST_SUBMODULE(smart_ptr, m) { // test_smart_ptr_from_default py::class_>(m, "HeldByDefaultHolder") .def(py::init<>()) + // NOLINTNEXTLINE(performance-unnecessary-value-param) .def_static("load_shared_ptr", [](std::shared_ptr) {}); // test_shared_ptr_gc diff --git a/tests/test_stl.cpp b/tests/test_stl.cpp index 059016277..07899b84e 100644 --- a/tests/test_stl.cpp +++ b/tests/test_stl.cpp @@ -202,6 +202,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; }); using opt_holder = OptionalHolder; @@ -245,13 +246,13 @@ TEST_SUBMODULE(stl, m) { using result_type = const char *; result_type operator()(int) { return "int"; } - result_type operator()(std::string) { return "std::string"; } + result_type operator()(const std::string &) { return "std::string"; } result_type operator()(double) { return "double"; } result_type operator()(std::nullptr_t) { return "std::nullptr_t"; } }; // test_variant - m.def("load_variant", [](variant v) { + m.def("load_variant", [](const variant &v) { return py::detail::visit_helper::call(visitor(), v); }); m.def("load_variant_2pass", [](variant v) { @@ -287,8 +288,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; }); class Placeholder { diff --git a/tests/test_virtual_functions.cpp b/tests/test_virtual_functions.cpp index 685d64a7c..51068f638 100644 --- a/tests/test_virtual_functions.cpp +++ b/tests/test_virtual_functions.cpp @@ -17,7 +17,10 @@ class ExampleVirt { public: ExampleVirt(int state) : state(state) { print_created(this, state); } ExampleVirt(const ExampleVirt &e) : state(e.state) { print_copy_created(this); } - ExampleVirt(ExampleVirt &&e) : state(e.state) { print_move_created(this); e.state = 0; } + ExampleVirt(ExampleVirt &&e) noexcept : state(e.state) { + print_move_created(this); + e.state = 0; + } virtual ~ExampleVirt() { print_destroyed(this); } virtual int run(int value) { @@ -100,7 +103,10 @@ public: class NonCopyable { public: NonCopyable(int a, int b) : value{new int(a*b)} { print_created(this, a, b); } - NonCopyable(NonCopyable &&o) { value = std::move(o.value); print_move_created(this); } + NonCopyable(NonCopyable &&o) noexcept { + value = std::move(o.value); + print_move_created(this); + } NonCopyable(const NonCopyable &) = delete; NonCopyable() = delete; void operator=(const NonCopyable &) = delete; @@ -120,7 +126,10 @@ class Movable { public: Movable(int a, int b) : value{a+b} { print_created(this, a, b); } Movable(const Movable &m) { value = m.value; print_copy_created(this); } - Movable(Movable &&m) { value = std::move(m.value); print_move_created(this); } + Movable(Movable &&m) noexcept { + value = m.value; + print_move_created(this); + } std::string get_value() const { return std::to_string(value); } ~Movable() { print_destroyed(this); } private: