fix(clang-tidy): Apply performance fixes from clang-tidy (#3046)

Skylion007 · web-flow · commit af6218ff78d4 · 2021-06-19T10:53:27.000-07:00
* Apply performance fixes from clang-tidy

* 2nd Round of Perf Optimizations

* 3rd round of fixes &amp; handle false-positive

* Apply missing fix and clang-format

* Apply reviewer comment
diff --git a/.clang-tidy b/.clang-tidy
@@ -10,4 +10,8 @@ modernize-use-auto,
 modernize-use-emplace,
 '
 
+CheckOptions:
+- key:             performance-unnecessary-value-param.AllowedTypes
+  value:           'exception_ptr$;'
+
 HeaderFilterRegex: 'pybind11/.*h'
diff --git a/include/pybind11/buffer_info.h b/include/pybind11/buffer_info.h
@@ -91,11 +91,9 @@ struct buffer_info {
     buffer_info(const buffer_info &) = delete;
     buffer_info& operator=(const buffer_info &) = delete;
 
-    buffer_info(buffer_info &&other) {
-        (*this) = std::move(other);
-    }
+    buffer_info(buffer_info &&other) noexcept { (*this) = std::move(other); }
 
-    buffer_info& operator=(buffer_info &&rhs) {
+    buffer_info &operator=(buffer_info &&rhs) noexcept {
         ptr = rhs.ptr;
         itemsize = rhs.itemsize;
         size = rhs.size;
diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h
@@ -1064,7 +1064,9 @@ struct kw_only {};
 struct pos_only {};
 
 template <typename T>
-arg_v arg::operator=(T &&value) const { return {std::move(*this), std::forward<T>(value)}; }
+arg_v arg::operator=(T &&value) const {
+    return {*this, std::forward<T>(value)};
+}
 
 /// Alias for backward compatibility -- to be removed in version 2.0
 template <typename /*unused*/> using arg_t = arg_v;
@@ -1286,7 +1288,7 @@ class unpacking_collector {
                          "may be passed via py::arg() to a python function call. "
                          "(compile in debug mode for details)");
     }
-    [[noreturn]] static void nameless_argument_error(std::string type) {
+    [[noreturn]] static void nameless_argument_error(const std::string &type) {
         throw type_error("Got kwargs without a name of type '" + type + "'; only named "
                          "arguments may be passed via py::arg() to a python function call. ");
     }
@@ -1295,7 +1297,7 @@ class unpacking_collector {
                          "(compile in debug mode for details)");
     }
 
-    [[noreturn]] static void multiple_values_error(std::string name) {
+    [[noreturn]] static void multiple_values_error(const std::string &name) {
         throw type_error("Got multiple values for keyword argument '" + name + "'");
     }
 
@@ -1304,7 +1306,8 @@ class unpacking_collector {
                          "(compile in debug mode for details)");
     }
 
-    [[noreturn]] static void argument_cast_error(std::string name, std::string type) {
+    [[noreturn]] static void argument_cast_error(const std::string &name,
+                                                 const std::string &type) {
         throw cast_error("Unable to convert call argument '" + name
                          + "' of type '" + type + "' to Python object");
     }
diff --git a/include/pybind11/eval.h b/include/pybind11/eval.h
@@ -11,6 +11,8 @@
 
 #pragma once
 
+#include <utility>
+
 #include "pybind11.h"
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
@@ -43,7 +45,7 @@ enum eval_mode {
 };
 
 template <eval_mode mode = eval_expr>
-object eval(str expr, object global = globals(), object local = object()) {
+object eval(const str &expr, object global = globals(), object local = object()) {
     if (!local)
         local = global;
 
@@ -75,8 +77,8 @@ object eval(const char (&s)[N], object global = globals(), object local = object
     return eval<mode>(expr, global, local);
 }
 
-inline void exec(str expr, object global = globals(), object local = object()) {
-    eval<eval_statements>(expr, global, local);
+inline void exec(const str &expr, object global = globals(), object local = object()) {
+    eval<eval_statements>(expr, std::move(global), std::move(local));
 }
 
 template <size_t N>
diff --git a/include/pybind11/iostream.h b/include/pybind11/iostream.h
@@ -11,14 +11,15 @@
 
 #include "pybind11.h"
 
-#include <streambuf>
-#include <ostream>
-#include <string>
-#include <memory>
-#include <iostream>
+#include <algorithm>
 #include <cstring>
+#include <iostream>
 #include <iterator>
-#include <algorithm>
+#include <memory>
+#include <ostream>
+#include <streambuf>
+#include <string>
+#include <utility>
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 PYBIND11_NAMESPACE_BEGIN(detail)
@@ -117,11 +118,8 @@ class pythonbuf : public std::streambuf {
     }
 
 public:
-
-    pythonbuf(object pyostream, size_t buffer_size = 1024)
-        : buf_size(buffer_size),
-          d_buffer(new char[buf_size]),
-          pywrite(pyostream.attr("write")),
+    pythonbuf(const object &pyostream, size_t buffer_size = 1024)
+        : buf_size(buffer_size), d_buffer(new char[buf_size]), pywrite(pyostream.attr("write")),
           pyflush(pyostream.attr("flush")) {
         setp(d_buffer.get(), d_buffer.get() + buf_size - 1);
     }
@@ -168,9 +166,8 @@ class scoped_ostream_redirect {
     detail::pythonbuf buffer;
 
 public:
-    scoped_ostream_redirect(
-            std::ostream &costream = std::cout,
-            object pyostream = module_::import("sys").attr("stdout"))
+    scoped_ostream_redirect(std::ostream &costream  = std::cout,
+                            const object &pyostream = module_::import("sys").attr("stdout"))
         : costream(costream), buffer(pyostream) {
         old = costream.rdbuf(&buffer);
     }
@@ -199,10 +196,9 @@ class scoped_ostream_redirect {
 \endrst */
 class scoped_estream_redirect : public scoped_ostream_redirect {
 public:
-    scoped_estream_redirect(
-            std::ostream &costream = std::cerr,
-            object pyostream = module_::import("sys").attr("stderr"))
-        : scoped_ostream_redirect(costream,pyostream) {}
+    scoped_estream_redirect(std::ostream &costream  = std::cerr,
+                            const object &pyostream = module_::import("sys").attr("stderr"))
+        : scoped_ostream_redirect(costream, pyostream) {}
 };
 
 
@@ -261,9 +257,10 @@ PYBIND11_NAMESPACE_END(detail)
             m.noisy_function_with_error_printing()
 
  \endrst */
-inline class_<detail::OstreamRedirect> add_ostream_redirect(module_ m, std::string name = "ostream_redirect") {
-    return class_<detail::OstreamRedirect>(m, name.c_str(), module_local())
-        .def(init<bool,bool>(), arg("stdout")=true, arg("stderr")=true)
+inline class_<detail::OstreamRedirect>
+add_ostream_redirect(module_ m, const std::string &name = "ostream_redirect") {
+    return class_<detail::OstreamRedirect>(std::move(m), name.c_str(), module_local())
+        .def(init<bool, bool>(), arg("stdout") = true, arg("stderr") = true)
         .def("__enter__", &detail::OstreamRedirect::enter)
         .def("__exit__", [](detail::OstreamRedirect &self_, args) { self_.exit(); });
 }
diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h
@@ -164,10 +164,10 @@ struct npy_api {
             NPY_ULONG_, NPY_ULONGLONG_, NPY_UINT_),
     };
 
-    typedef struct {
+    struct PyArray_Dims {
         Py_intptr_t *ptr;
         int len;
-    } PyArray_Dims;
+    };
 
     static npy_api& get() {
         static npy_api api = lookup();
diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h
@@ -1412,15 +1412,15 @@ class class_ : public detail::generic_type {
 
     template <typename D, typename... Extra>
     class_ &def_readwrite_static(const char *name, D *pm, const Extra& ...extra) {
-        cpp_function fget([pm](object) -> const D &{ return *pm; }, scope(*this)),
-                     fset([pm](object, const D &value) { *pm = value; }, scope(*this));
+        cpp_function fget([pm](const object &) -> const D & { return *pm; }, scope(*this)),
+            fset([pm](const object &, const D &value) { *pm = value; }, scope(*this));
         def_property_static(name, fget, fset, return_value_policy::reference, extra...);
         return *this;
     }
 
     template <typename D, typename... Extra>
     class_ &def_readonly_static(const char *name, const D *pm, const Extra& ...extra) {
-        cpp_function fget([pm](object) -> const D &{ return *pm; }, scope(*this));
+        cpp_function fget([pm](const object &) -> const D & { return *pm; }, scope(*this));
         def_property_readonly_static(name, fget, return_value_policy::reference, extra...);
         return *this;
     }
@@ -1671,30 +1671,36 @@ struct enum_base {
             }, name("__members__")), none(), none(), ""
         );
 
-        #define PYBIND11_ENUM_OP_STRICT(op, expr, strict_behavior)                     \
-            m_base.attr(op) = cpp_function(                                            \
-                [](object a, object b) {                                               \
-                    if (!type::handle_of(a).is(type::handle_of(b)))                    \
-                        strict_behavior;                                               \
-                    return expr;                                                       \
-                },                                                                     \
-                name(op), is_method(m_base), arg("other"))
-
-        #define PYBIND11_ENUM_OP_CONV(op, expr)                                        \
-            m_base.attr(op) = cpp_function(                                            \
-                [](object a_, object b_) {                                             \
-                    int_ a(a_), b(b_);                                                 \
-                    return expr;                                                       \
-                },                                                                     \
-                name(op), is_method(m_base), arg("other"))
-
-        #define PYBIND11_ENUM_OP_CONV_LHS(op, expr)                                    \
-            m_base.attr(op) = cpp_function(                                            \
-                [](object a_, object b) {                                              \
-                    int_ a(a_);                                                        \
-                    return expr;                                                       \
-                },                                                                     \
-                name(op), is_method(m_base), arg("other"))
+#define PYBIND11_ENUM_OP_STRICT(op, expr, strict_behavior)                                        \
+    m_base.attr(op) = cpp_function(                                                               \
+        [](const object &a, const object &b) {                                                    \
+            if (!type::handle_of(a).is(type::handle_of(b)))                                       \
+                strict_behavior;                                                                  \
+            return expr;                                                                          \
+        },                                                                                        \
+        name(op),                                                                                 \
+        is_method(m_base),                                                                        \
+        arg("other"))
+
+#define PYBIND11_ENUM_OP_CONV(op, expr)                                                           \
+    m_base.attr(op) = cpp_function(                                                               \
+        [](const object &a_, const object &b_) {                                                  \
+            int_ a(a_), b(b_);                                                                    \
+            return expr;                                                                          \
+        },                                                                                        \
+        name(op),                                                                                 \
+        is_method(m_base),                                                                        \
+        arg("other"))
+
+#define PYBIND11_ENUM_OP_CONV_LHS(op, expr)                                                       \
+    m_base.attr(op) = cpp_function(                                                               \
+        [](const object &a_, const object &b) {                                                   \
+            int_ a(a_);                                                                           \
+            return expr;                                                                          \
+        },                                                                                        \
+        name(op),                                                                                 \
+        is_method(m_base),                                                                        \
+        arg("other"))
 
         if (is_convertible) {
             PYBIND11_ENUM_OP_CONV_LHS("__eq__", !b.is_none() &&  a.equal(b));
@@ -2054,7 +2060,7 @@ exception<CppException> &register_exception(handle scope,
 }
 
 PYBIND11_NAMESPACE_BEGIN(detail)
-PYBIND11_NOINLINE inline void print(tuple args, dict kwargs) {
+PYBIND11_NOINLINE inline void print(const tuple &args, const dict &kwargs) {
     auto strings = tuple(args.size());
     for (size_t i = 0; i < args.size(); ++i) {
         strings[i] = str(args[i]);
diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h
@@ -503,7 +503,7 @@ class accessor : public object_api<accessor<Policy>> {
 public:
     accessor(handle obj, key_type key) : obj(obj), key(std::move(key)) { }
     accessor(const accessor &) = default;
-    accessor(accessor &&) = default;
+    accessor(accessor &&) noexcept = default;
 
     // accessor overload required to override default assignment operator (templates are not allowed
     // to replace default compiler-generated assignments).
@@ -1508,7 +1508,7 @@ class memoryview : public object {
         detail::any_container<ssize_t> shape,
         detail::any_container<ssize_t> strides) {
         return memoryview::from_buffer(
-            const_cast<void*>(ptr), itemsize, format, shape, strides, true);
+            const_cast<void *>(ptr), itemsize, format, std::move(shape), std::move(strides), true);
     }
 
     template<typename T>
diff --git a/include/pybind11/stl.h b/include/pybind11/stl.h
@@ -159,10 +159,13 @@ template <typename Type, typename Value> struct list_caster {
     }
 
 private:
-    template <typename T = Type,
-              enable_if_t<std::is_same<decltype(std::declval<T>().reserve(0)), void>::value, int> = 0>
-    void reserve_maybe(sequence s, Type *) { value.reserve(s.size()); }
-    void reserve_maybe(sequence, void *) { }
+    template <
+        typename T                                                                          = Type,
+        enable_if_t<std::is_same<decltype(std::declval<T>().reserve(0)), void>::value, int> = 0>
+    void reserve_maybe(const sequence &s, Type *) {
+        value.reserve(s.size());
+    }
+    void reserve_maybe(const sequence &, void *) {}
 
 public:
     template <typename T>
diff --git a/include/pybind11/stl_bind.h b/include/pybind11/stl_bind.h
@@ -128,11 +128,11 @@ void vector_modifiers(enable_if_t<is_copy_constructible<typename Vector::value_t
            arg("x"),
            "Add an item to the end of the list");
 
-    cl.def(init([](iterable it) {
+    cl.def(init([](const iterable &it) {
         auto v = std::unique_ptr<Vector>(new Vector());
         v->reserve(len_hint(it));
         for (handle h : it)
-           v->push_back(h.cast<T>());
+            v->push_back(h.cast<T>());
         return v.release();
     }));
 
@@ -151,27 +151,28 @@ void vector_modifiers(enable_if_t<is_copy_constructible<typename Vector::value_t
        "Extend the list by appending all the items in the given list"
     );
 
-    cl.def("extend",
-       [](Vector &v, iterable it) {
-           const size_t old_size = v.size();
-           v.reserve(old_size + len_hint(it));
-           try {
-               for (handle h : it) {
-                   v.push_back(h.cast<T>());
-               }
-           } catch (const cast_error &) {
-               v.erase(v.begin() + static_cast<typename Vector::difference_type>(old_size), v.end());
-               try {
-                   v.shrink_to_fit();
-               } catch (const std::exception &) {
-                   // Do nothing
-               }
-               throw;
-           }
-       },
-       arg("L"),
-       "Extend the list by appending all the items in the given list"
-    );
+    cl.def(
+        "extend",
+        [](Vector &v, const iterable &it) {
+            const size_t old_size = v.size();
+            v.reserve(old_size + len_hint(it));
+            try {
+                for (handle h : it) {
+                    v.push_back(h.cast<T>());
+                }
+            } catch (const cast_error &) {
+                v.erase(v.begin() + static_cast<typename Vector::difference_type>(old_size),
+                        v.end());
+                try {
+                    v.shrink_to_fit();
+                } catch (const std::exception &) {
+                    // Do nothing
+                }
+                throw;
+            }
+        },
+        arg("L"),
+        "Extend the list by appending all the items in the given list");
 
     cl.def("insert",
         [](Vector &v, DiffType i, const T &x) {
@@ -400,7 +401,7 @@ void vector_buffer_impl(Class_& cl, std::true_type) {
         return buffer_info(v.data(), static_cast<ssize_t>(sizeof(T)), format_descriptor<T>::format(), 1, {v.size()}, {sizeof(T)});
     });
 
-    cl.def(init([](buffer buf) {
+    cl.def(init([](const buffer &buf) {
         auto info = buf.request();
         if (info.ndim != 1 || info.strides[0] % static_cast<ssize_t>(sizeof(T)))
             throw type_error("Only valid 1D buffers can be copied to a vector");

Original file line number	Diff line number	Diff line change
`@@ -10,4 +10,8 @@ modernize-use-auto,`
`10`	`10`	`modernize-use-emplace,`
`11`	`11`	`'`
`12`	`12`
	`13`	`+CheckOptions:`
	`14`	`+- key: performance-unnecessary-value-param.AllowedTypes`
	`15`	`+ value: 'exception_ptr$;'`
	`16`	`+`
`13`	`17`	`HeaderFilterRegex: 'pybind11/.*h'`
Original file line number	Diff line number	Diff line change
`@@ -1064,7 +1064,9 @@ struct kw_only {};`
`1064`	`1064`	`struct pos_only {};`
`1065`	`1065`
`1066`	`1066`	`template <typename T>`
`1067`		`-arg_v arg::operator=(T &&value) const { return {std::move(*this), std::forward<T>(value)}; }`
	`1067`	`+arg_v arg::operator=(T &&value) const {`
	`1068`	`+ return {*this, std::forward<T>(value)};`
	`1069`	`+}`
`1068`	`1070`
`1069`	`1071`	`/// Alias for backward compatibility -- to be removed in version 2.0`
`1070`	`1072`	`template <typename /unused/> using arg_t = arg_v;`
`@@ -1286,7 +1288,7 @@ class unpacking_collector {`
`1286`	`1288`	`"may be passed via py::arg() to a python function call. "`
`1287`	`1289`	`"(compile in debug mode for details)");`
`1288`	`1290`	`}`
`1289`		`- [[noreturn]] static void nameless_argument_error(std::string type) {`
	`1291`	`+ [[noreturn]] static void nameless_argument_error(const std::string &type) {`
`1290`	`1292`	`throw type_error("Got kwargs without a name of type '" + type + "'; only named "`
`1291`	`1293`	`"arguments may be passed via py::arg() to a python function call. ");`
`1292`	`1294`	`}`
`@@ -1295,7 +1297,7 @@ class unpacking_collector {`
`1295`	`1297`	`"(compile in debug mode for details)");`
`1296`	`1298`	`}`
`1297`	`1299`
`1298`		`- [[noreturn]] static void multiple_values_error(std::string name) {`
	`1300`	`+ [[noreturn]] static void multiple_values_error(const std::string &name) {`
`1299`	`1301`	`throw type_error("Got multiple values for keyword argument '" + name + "'");`
`1300`	`1302`	`}`
`1301`	`1303`
`@@ -1304,7 +1306,8 @@ class unpacking_collector {`
`1304`	`1306`	`"(compile in debug mode for details)");`
`1305`	`1307`	`}`
`1306`	`1308`
`1307`		`- [[noreturn]] static void argument_cast_error(std::string name, std::string type) {`
	`1309`	`+ [[noreturn]] static void argument_cast_error(const std::string &name,`
	`1310`	`+ const std::string &type) {`
`1308`	`1311`	`throw cast_error("Unable to convert call argument '" + name`
`1309`	`1312`	`+ "' of type '" + type + "' to Python object");`
`1310`	`1313`	`}`