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

* Apply performance fixes from clang-tidy

* 2nd Round of Perf Optimizations

* 3rd round of fixes & handle false-positive

* Apply missing fix and clang-format

* Apply reviewer comment
This commit is contained in:
Aaron Gokaslan 2021-06-19 13:53:27 -04:00 committed by GitHub
parent 79178e713d
commit af6218ff78
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 105 additions and 91 deletions

View File

@ -10,4 +10,8 @@ modernize-use-auto,
modernize-use-emplace, modernize-use-emplace,
' '
CheckOptions:
- key: performance-unnecessary-value-param.AllowedTypes
value: 'exception_ptr$;'
HeaderFilterRegex: 'pybind11/.*h' HeaderFilterRegex: 'pybind11/.*h'

View File

@ -91,11 +91,9 @@ struct buffer_info {
buffer_info(const buffer_info &) = delete; buffer_info(const buffer_info &) = delete;
buffer_info& operator=(const buffer_info &) = delete; buffer_info& operator=(const buffer_info &) = delete;
buffer_info(buffer_info &&other) { buffer_info(buffer_info &&other) noexcept { (*this) = std::move(other); }
(*this) = std::move(other);
}
buffer_info& operator=(buffer_info &&rhs) { buffer_info &operator=(buffer_info &&rhs) noexcept {
ptr = rhs.ptr; ptr = rhs.ptr;
itemsize = rhs.itemsize; itemsize = rhs.itemsize;
size = rhs.size; size = rhs.size;

View File

@ -1064,7 +1064,9 @@ struct kw_only {};
struct pos_only {}; struct pos_only {};
template <typename T> 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 /// Alias for backward compatibility -- to be removed in version 2.0
template <typename /*unused*/> using arg_t = arg_v; template <typename /*unused*/> using arg_t = arg_v;
@ -1286,7 +1288,7 @@ private:
"may be passed via py::arg() to a python function call. " "may be passed via py::arg() to a python function call. "
"(compile in debug mode for details)"); "(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 " 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. "); "arguments may be passed via py::arg() to a python function call. ");
} }
@ -1295,7 +1297,7 @@ private:
"(compile in debug mode for details)"); "(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 + "'"); throw type_error("Got multiple values for keyword argument '" + name + "'");
} }
@ -1304,7 +1306,8 @@ private:
"(compile in debug mode for details)"); "(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 throw cast_error("Unable to convert call argument '" + name
+ "' of type '" + type + "' to Python object"); + "' of type '" + type + "' to Python object");
} }

View File

@ -11,6 +11,8 @@
#pragma once #pragma once
#include <utility>
#include "pybind11.h" #include "pybind11.h"
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
@ -43,7 +45,7 @@ enum eval_mode {
}; };
template <eval_mode mode = eval_expr> 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) if (!local)
local = global; local = global;
@ -75,8 +77,8 @@ object eval(const char (&s)[N], object global = globals(), object local = object
return eval<mode>(expr, global, local); return eval<mode>(expr, global, local);
} }
inline void exec(str expr, object global = globals(), object local = object()) { inline void exec(const str &expr, object global = globals(), object local = object()) {
eval<eval_statements>(expr, global, local); eval<eval_statements>(expr, std::move(global), std::move(local));
} }
template <size_t N> template <size_t N>

View File

@ -11,14 +11,15 @@
#include "pybind11.h" #include "pybind11.h"
#include <streambuf>
#include <ostream>
#include <string>
#include <memory>
#include <iostream>
#include <cstring>
#include <iterator>
#include <algorithm> #include <algorithm>
#include <cstring>
#include <iostream>
#include <iterator>
#include <memory>
#include <ostream>
#include <streambuf>
#include <string>
#include <utility>
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
PYBIND11_NAMESPACE_BEGIN(detail) PYBIND11_NAMESPACE_BEGIN(detail)
@ -117,11 +118,8 @@ private:
} }
public: public:
pythonbuf(const object &pyostream, size_t buffer_size = 1024)
pythonbuf(object pyostream, size_t buffer_size = 1024) : buf_size(buffer_size), d_buffer(new char[buf_size]), pywrite(pyostream.attr("write")),
: buf_size(buffer_size),
d_buffer(new char[buf_size]),
pywrite(pyostream.attr("write")),
pyflush(pyostream.attr("flush")) { pyflush(pyostream.attr("flush")) {
setp(d_buffer.get(), d_buffer.get() + buf_size - 1); setp(d_buffer.get(), d_buffer.get() + buf_size - 1);
} }
@ -168,9 +166,8 @@ protected:
detail::pythonbuf buffer; detail::pythonbuf buffer;
public: public:
scoped_ostream_redirect( scoped_ostream_redirect(std::ostream &costream = std::cout,
std::ostream &costream = std::cout, const object &pyostream = module_::import("sys").attr("stdout"))
object pyostream = module_::import("sys").attr("stdout"))
: costream(costream), buffer(pyostream) { : costream(costream), buffer(pyostream) {
old = costream.rdbuf(&buffer); old = costream.rdbuf(&buffer);
} }
@ -199,10 +196,9 @@ public:
\endrst */ \endrst */
class scoped_estream_redirect : public scoped_ostream_redirect { class scoped_estream_redirect : public scoped_ostream_redirect {
public: public:
scoped_estream_redirect( scoped_estream_redirect(std::ostream &costream = std::cerr,
std::ostream &costream = std::cerr, const object &pyostream = module_::import("sys").attr("stderr"))
object pyostream = module_::import("sys").attr("stderr")) : scoped_ostream_redirect(costream, pyostream) {}
: scoped_ostream_redirect(costream,pyostream) {}
}; };
@ -261,9 +257,10 @@ PYBIND11_NAMESPACE_END(detail)
m.noisy_function_with_error_printing() m.noisy_function_with_error_printing()
\endrst */ \endrst */
inline class_<detail::OstreamRedirect> add_ostream_redirect(module_ m, std::string name = "ostream_redirect") { inline class_<detail::OstreamRedirect>
return class_<detail::OstreamRedirect>(m, name.c_str(), module_local()) add_ostream_redirect(module_ m, const std::string &name = "ostream_redirect") {
.def(init<bool,bool>(), arg("stdout")=true, arg("stderr")=true) 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("__enter__", &detail::OstreamRedirect::enter)
.def("__exit__", [](detail::OstreamRedirect &self_, args) { self_.exit(); }); .def("__exit__", [](detail::OstreamRedirect &self_, args) { self_.exit(); });
} }

View File

@ -164,10 +164,10 @@ struct npy_api {
NPY_ULONG_, NPY_ULONGLONG_, NPY_UINT_), NPY_ULONG_, NPY_ULONGLONG_, NPY_UINT_),
}; };
typedef struct { struct PyArray_Dims {
Py_intptr_t *ptr; Py_intptr_t *ptr;
int len; int len;
} PyArray_Dims; };
static npy_api& get() { static npy_api& get() {
static npy_api api = lookup(); static npy_api api = lookup();

View File

@ -1412,15 +1412,15 @@ public:
template <typename D, typename... Extra> template <typename D, typename... Extra>
class_ &def_readwrite_static(const char *name, D *pm, const Extra& ...extra) { class_ &def_readwrite_static(const char *name, 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)),
fset([pm](object, const D &value) { *pm = value; }, scope(*this)); fset([pm](const object &, const D &value) { *pm = value; }, scope(*this));
def_property_static(name, fget, fset, return_value_policy::reference, extra...); def_property_static(name, fget, fset, return_value_policy::reference, extra...);
return *this; return *this;
} }
template <typename D, typename... Extra> template <typename D, typename... Extra>
class_ &def_readonly_static(const char *name, const D *pm, const Extra& ...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...); def_property_readonly_static(name, fget, return_value_policy::reference, extra...);
return *this; return *this;
} }
@ -1671,30 +1671,36 @@ struct enum_base {
}, name("__members__")), none(), none(), "" }, name("__members__")), none(), none(), ""
); );
#define PYBIND11_ENUM_OP_STRICT(op, expr, strict_behavior) \ #define PYBIND11_ENUM_OP_STRICT(op, expr, strict_behavior) \
m_base.attr(op) = cpp_function( \ m_base.attr(op) = cpp_function( \
[](object a, object b) { \ [](const object &a, const object &b) { \
if (!type::handle_of(a).is(type::handle_of(b))) \ if (!type::handle_of(a).is(type::handle_of(b))) \
strict_behavior; \ strict_behavior; \
return expr; \ return expr; \
}, \ }, \
name(op), is_method(m_base), arg("other")) name(op), \
is_method(m_base), \
arg("other"))
#define PYBIND11_ENUM_OP_CONV(op, expr) \ #define PYBIND11_ENUM_OP_CONV(op, expr) \
m_base.attr(op) = cpp_function( \ m_base.attr(op) = cpp_function( \
[](object a_, object b_) { \ [](const object &a_, const object &b_) { \
int_ a(a_), b(b_); \ int_ a(a_), b(b_); \
return expr; \ return expr; \
}, \ }, \
name(op), is_method(m_base), arg("other")) name(op), \
is_method(m_base), \
arg("other"))
#define PYBIND11_ENUM_OP_CONV_LHS(op, expr) \ #define PYBIND11_ENUM_OP_CONV_LHS(op, expr) \
m_base.attr(op) = cpp_function( \ m_base.attr(op) = cpp_function( \
[](object a_, object b) { \ [](const object &a_, const object &b) { \
int_ a(a_); \ int_ a(a_); \
return expr; \ return expr; \
}, \ }, \
name(op), is_method(m_base), arg("other")) name(op), \
is_method(m_base), \
arg("other"))
if (is_convertible) { if (is_convertible) {
PYBIND11_ENUM_OP_CONV_LHS("__eq__", !b.is_none() && a.equal(b)); 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_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()); auto strings = tuple(args.size());
for (size_t i = 0; i < args.size(); ++i) { for (size_t i = 0; i < args.size(); ++i) {
strings[i] = str(args[i]); strings[i] = str(args[i]);

View File

@ -503,7 +503,7 @@ class accessor : public object_api<accessor<Policy>> {
public: public:
accessor(handle obj, key_type key) : obj(obj), key(std::move(key)) { } accessor(handle obj, key_type key) : obj(obj), key(std::move(key)) { }
accessor(const accessor &) = default; accessor(const accessor &) = default;
accessor(accessor &&) = default; accessor(accessor &&) noexcept = default;
// accessor overload required to override default assignment operator (templates are not allowed // accessor overload required to override default assignment operator (templates are not allowed
// to replace default compiler-generated assignments). // to replace default compiler-generated assignments).
@ -1508,7 +1508,7 @@ public:
detail::any_container<ssize_t> shape, detail::any_container<ssize_t> shape,
detail::any_container<ssize_t> strides) { detail::any_container<ssize_t> strides) {
return memoryview::from_buffer( 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> template<typename T>

View File

@ -159,10 +159,13 @@ template <typename Type, typename Value> struct list_caster {
} }
private: private:
template <typename T = Type, template <
enable_if_t<std::is_same<decltype(std::declval<T>().reserve(0)), void>::value, int> = 0> typename T = Type,
void reserve_maybe(sequence s, Type *) { value.reserve(s.size()); } enable_if_t<std::is_same<decltype(std::declval<T>().reserve(0)), void>::value, int> = 0>
void reserve_maybe(sequence, void *) { } void reserve_maybe(const sequence &s, Type *) {
value.reserve(s.size());
}
void reserve_maybe(const sequence &, void *) {}
public: public:
template <typename T> template <typename T>

View File

@ -128,11 +128,11 @@ void vector_modifiers(enable_if_t<is_copy_constructible<typename Vector::value_t
arg("x"), arg("x"),
"Add an item to the end of the list"); "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()); auto v = std::unique_ptr<Vector>(new Vector());
v->reserve(len_hint(it)); v->reserve(len_hint(it));
for (handle h : it) for (handle h : it)
v->push_back(h.cast<T>()); v->push_back(h.cast<T>());
return v.release(); 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" "Extend the list by appending all the items in the given list"
); );
cl.def("extend", cl.def(
[](Vector &v, iterable it) { "extend",
const size_t old_size = v.size(); [](Vector &v, const iterable &it) {
v.reserve(old_size + len_hint(it)); const size_t old_size = v.size();
try { v.reserve(old_size + len_hint(it));
for (handle h : it) { try {
v.push_back(h.cast<T>()); 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()); } catch (const cast_error &) {
try { v.erase(v.begin() + static_cast<typename Vector::difference_type>(old_size),
v.shrink_to_fit(); v.end());
} catch (const std::exception &) { try {
// Do nothing v.shrink_to_fit();
} } catch (const std::exception &) {
throw; // Do nothing
} }
}, throw;
arg("L"), }
"Extend the list by appending all the items in the given list" },
); arg("L"),
"Extend the list by appending all the items in the given list");
cl.def("insert", cl.def("insert",
[](Vector &v, DiffType i, const T &x) { [](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)}); 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(); auto info = buf.request();
if (info.ndim != 1 || info.strides[0] % static_cast<ssize_t>(sizeof(T))) 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"); throw type_error("Only valid 1D buffers can be copied to a vector");