Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@ modernize-use-auto,
modernize-use-emplace,
'

CheckOptions:
- key: performance-unnecessary-value-param.AllowedTypes
value: 'exception_ptr$;'

HeaderFilterRegex: 'pybind11/.*h'
6 changes: 2 additions & 4 deletions include/pybind11/buffer_info.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 7 additions & 4 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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. ");
}
Expand All @@ -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 + "'");
}

Expand All @@ -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");
}
Expand Down
8 changes: 5 additions & 3 deletions include/pybind11/eval.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

#pragma once

#include <utility>

#include "pybind11.h"

PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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>
Expand Down
39 changes: 18 additions & 21 deletions include/pybind11/iostream.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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) {}
};


Expand Down Expand Up @@ -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(); });
}
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/numpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
62 changes: 34 additions & 28 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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) \
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format required this change.

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));
Expand Down Expand Up @@ -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]);
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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>
Expand Down
11 changes: 7 additions & 4 deletions include/pybind11/stl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
49 changes: 25 additions & 24 deletions include/pybind11/stl_bind.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}));

Expand All @@ -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) {
Expand Down Expand Up @@ -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");
Expand Down