Skip to content
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

fix(clang-tidy): Enable clang-tidy else-after-return and redundant void checks #3080

Merged
merged 6 commits into from
Jul 9, 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
3 changes: 3 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
FormatStyle: file

Checks: '
clang-analyzer-optin.cplusplus.VirtualCall,
llvm-namespace-comment,
misc-misplaced-const,
misc-static-assert,
misc-uniqueptr-reset-release,
modernize-avoid-bind,
modernize-redundant-void-arg,
modernize-replace-auto-ptr,
modernize-replace-disallow-copy-and-assign-macro,
modernize-shrink-to-fit,
Expand All @@ -20,6 +22,7 @@ modernize-use-override,
modernize-use-using,
*performance*,
readability-container-size-empty,
readability-else-after-return,
readability-make-member-function-const,
readability-redundant-function-ptr-dereference,
readability-redundant-smartptr-get,
Expand Down
85 changes: 47 additions & 38 deletions include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,25 +85,28 @@ template <typename type> class type_caster<std::reference_wrapper<type>> {
operator std::reference_wrapper<type>() { return cast_op<type &>(subcaster); }
};

#define PYBIND11_TYPE_CASTER(type, py_name) \
protected: \
type value; \
public: \
static constexpr auto name = py_name; \
template <typename T_, enable_if_t<std::is_same<type, remove_cv_t<T_>>::value, int> = 0> \
static handle cast(T_ *src, return_value_policy policy, handle parent) { \
if (!src) return none().release(); \
if (policy == return_value_policy::take_ownership) { \
auto h = cast(std::move(*src), policy, parent); delete src; return h; \
} else { \
return cast(*src, policy, parent); \
} \
} \
operator type*() { return &value; } \
operator type&() { return value; } \
operator type&&() && { return std::move(value); } \
template <typename T_> using cast_op_type = pybind11::detail::movable_cast_op_type<T_>

#define PYBIND11_TYPE_CASTER(type, py_name) \
protected: \
type value; \
\
public: \
static constexpr auto name = py_name; \
template <typename T_, enable_if_t<std::is_same<type, remove_cv_t<T_>>::value, int> = 0> \
static handle cast(T_ *src, return_value_policy policy, handle parent) { \
if (!src) \
return none().release(); \
if (policy == return_value_policy::take_ownership) { \
auto h = cast(std::move(*src), policy, parent); \
delete src; \
return h; \
} \
return cast(*src, policy, parent); \
} \
operator type *() { return &value; } \
operator type &() { return value; } \
operator type &&() && { return std::move(value); } \
template <typename T_> \
using cast_op_type = pybind11::detail::movable_cast_op_type<T_>

template <typename CharT> using is_std_char_type = any_of<
std::is_same<CharT, char>, /* std::string */
Expand Down Expand Up @@ -247,7 +250,8 @@ template <> class type_caster<void> : public type_caster<void_type> {
bool load(handle h, bool) {
if (!h) {
return false;
} else if (h.is_none()) {
}
if (h.is_none()) {
value = nullptr;
return true;
}
Expand All @@ -272,8 +276,7 @@ template <> class type_caster<void> : public type_caster<void_type> {
static handle cast(const void *ptr, return_value_policy /* policy */, handle /* parent */) {
if (ptr)
return capsule(ptr).release();
else
return none().inc_ref();
return none().inc_ref();
}

template <typename T> using cast_op_type = void*&;
Expand All @@ -289,9 +292,15 @@ template <> class type_caster<bool> {
public:
bool load(handle src, bool convert) {
if (!src) return false;
else if (src.ptr() == Py_True) { value = true; return true; }
else if (src.ptr() == Py_False) { value = false; return true; }
else if (convert || !std::strcmp("numpy.bool_", Py_TYPE(src.ptr())->tp_name)) {
if (src.ptr() == Py_True) {
value = true;
return true;
}
if (src.ptr() == Py_False) {
value = false;
return true;
}
if (convert || !std::strcmp("numpy.bool_", Py_TYPE(src.ptr())->tp_name)) {
// (allow non-implicit conversion for numpy booleans)

Py_ssize_t res = -1;
Expand All @@ -315,9 +324,8 @@ template <> class type_caster<bool> {
if (res == 0 || res == 1) {
value = (bool) res;
return true;
} else {
PyErr_Clear();
}
PyErr_Clear();
}
return false;
}
Expand Down Expand Up @@ -351,7 +359,8 @@ template <typename StringType, bool IsView = false> struct string_caster {
handle load_src = src;
if (!src) {
return false;
} else if (!PyUnicode_Check(load_src.ptr())) {
}
if (!PyUnicode_Check(load_src.ptr())) {
#if PY_MAJOR_VERSION >= 3
return load_bytes(load_src);
#else
Expand Down Expand Up @@ -554,10 +563,11 @@ template <template<typename...> class Tuple, typename... Ts> class tuple_caster
static handle cast(T *src, return_value_policy policy, handle parent) {
if (!src) return none().release();
if (policy == return_value_policy::take_ownership) {
auto h = cast(std::move(*src), policy, parent); delete src; return h;
} else {
return cast(*src, policy, parent);
auto h = cast(std::move(*src), policy, parent);
delete src;
return h;
}
return cast(*src, policy, parent);
}

static constexpr auto name = _("Tuple[") + concat(make_caster<Ts>::name...) + _("]");
Expand Down Expand Up @@ -664,14 +674,14 @@ struct copyable_holder_caster : public type_caster_base<type> {
value = v_h.value_ptr();
holder = v_h.template holder<holder_type>();
return true;
} else {
throw cast_error("Unable to cast from non-held to held instance (T& to Holder<T>) "
}
throw cast_error("Unable to cast from non-held to held instance (T& to Holder<T>) "
#if defined(NDEBUG)
"(compile in debug mode for type information)");
"(compile in debug mode for type information)");
#else
"of type '" + type_id<holder_type>() + "''");
"of type '"
+ type_id<holder_type>() + "''");
#endif
}
}

template <typename T = holder_type, detail::enable_if_t<!std::is_constructible<T, const T &, type*>::value, int> = 0>
Expand Down Expand Up @@ -917,8 +927,7 @@ template <typename T> detail::enable_if_t<detail::move_always<T>::value, T> cast
template <typename T> detail::enable_if_t<detail::move_if_unreferenced<T>::value, T> cast(object &&object) {
if (object.ref_count() > 1)
return cast<T>(object);
else
return move<T>(std::move(object));
return move<T>(std::move(object));
}
template <typename T> detail::enable_if_t<detail::move_never<T>::value, T> cast(object &&object) {
return cast<T>(object);
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/chrono.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,11 @@ template <typename type> class duration_caster {
return true;
}
// If invoked with a float we assume it is seconds and convert
else if (PyFloat_Check(src.ptr())) {
if (PyFloat_Check(src.ptr())) {
value = type(duration_cast<duration<rep, period>>(duration<double>(PyFloat_AsDouble(src.ptr()))));
return true;
}
else return false;
return false;
}

// If this is a duration just return it back
Expand Down
4 changes: 1 addition & 3 deletions include/pybind11/detail/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,7 @@ extern "C" inline PyObject *pybind11_meta_getattro(PyObject *obj, PyObject *name
Py_INCREF(descr);
return descr;
}
else {
return PyType_Type.tp_getattro(obj, name);
}
return PyType_Type.tp_getattro(obj, name);
}
#endif

Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/detail/type_caster_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ class type_caster_generic {
return true;
}
// Case 2: We have a derived class
else if (PyType_IsSubtype(srctype, typeinfo->type)) {
if (PyType_IsSubtype(srctype, typeinfo->type)) {
auto &bases = all_type_info(srctype);
bool no_cpp_mi = typeinfo->simple_type;

Expand All @@ -687,7 +687,7 @@ class type_caster_generic {
// Case 2b: the python type inherits from multiple C++ bases. Check the bases to see if
// we can find an exact match (or, for a simple C++ type, an inherited match); if so, we
// can safely reinterpret_cast to the relevant pointer.
else if (bases.size() > 1) {
if (bases.size() > 1) {
for (auto base : bases) {
if (no_cpp_mi ? PyType_IsSubtype(base->type, typeinfo->type) : base->type == typeinfo->type) {
this_.load_value(reinterpret_cast<instance *>(src.ptr())->get_value_and_holder(base));
Expand Down
9 changes: 3 additions & 6 deletions include/pybind11/eigen.h
Original file line number Diff line number Diff line change
Expand Up @@ -169,21 +169,18 @@ template <typename Type_> struct EigenProps {
return false; // Vector size mismatch
return {rows == 1 ? 1 : n, cols == 1 ? 1 : n, stride};
}
else if (fixed) {
if (fixed) {
// The type has a fixed size, but is not a vector: abort
return false;
}
else if (fixed_cols) {
if (fixed_cols) {
// Since this isn't a vector, cols must be != 1. We allow this only if it exactly
// equals the number of elements (rows is Dynamic, and so 1 row is allowed).
if (cols != n) return false;
return {1, n, stride};
}
else {
// Otherwise it's either fully dynamic, or column dynamic; both become a column vector
} // Otherwise it's either fully dynamic, or column dynamic; both become a column vector
if (fixed_rows && rows != n) return false;
return {n, 1, stride};
}
}

static constexpr bool show_writeable = is_eigen_dense_map<Type>::value && is_eigen_mutable_map<Type>::value;
Expand Down
3 changes: 1 addition & 2 deletions include/pybind11/functional.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,7 @@ struct type_caster<std::function<Return(Args...)>> {
auto result = f_.template target<function_type>();
if (result)
return cpp_function(*result, policy).release();
else
return cpp_function(std::forward<Func>(f_), policy).release();
return cpp_function(std::forward<Func>(f_), policy).release();
}

PYBIND11_TYPE_CASTER(type, _("Callable[[") + concat(make_caster<Args>::name...) + _("], ")
Expand Down
6 changes: 2 additions & 4 deletions include/pybind11/numpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -1340,9 +1340,8 @@ template <size_t N> class multi_array_iterator {
if (++m_index[i] != m_shape[i]) {
increment_common_iterator(i);
break;
} else {
m_index[i] = 0;
}
m_index[i] = 0;
}
return *this;
}
Expand Down Expand Up @@ -1493,8 +1492,7 @@ struct vectorize_returned_array {
static Type create(broadcast_trivial trivial, const std::vector<ssize_t> &shape) {
if (trivial == broadcast_trivial::f_trivial)
return array_t<Return, array::f_style>(shape);
else
return array_t<Return>(shape);
return array_t<Return>(shape);
}

static Return *mutable_data(Type &array) {
Expand Down
45 changes: 23 additions & 22 deletions include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ class cpp_function : public function {
std::memset(rec->def, 0, sizeof(PyMethodDef));
rec->def->ml_name = rec->name;
rec->def->ml_meth
= reinterpret_cast<PyCFunction>(reinterpret_cast<void (*)(void)>(dispatcher));
= reinterpret_cast<PyCFunction>(reinterpret_cast<void (*)()>(dispatcher));
rec->def->ml_flags = METH_VARARGS | METH_KEYWORDS;

capsule rec_capsule(unique_rec.release(), [](void *ptr) {
Expand Down Expand Up @@ -924,20 +924,20 @@ class cpp_function : public function {
append_note_if_missing_header_is_suspected(msg);
PyErr_SetString(PyExc_TypeError, msg.c_str());
return nullptr;
} else if (!result) {
}
if (!result) {
std::string msg = "Unable to convert function return value to a "
"Python type! The signature was\n\t";
msg += it->signature;
append_note_if_missing_header_is_suspected(msg);
PyErr_SetString(PyExc_TypeError, msg.c_str());
return nullptr;
} else {
if (overloads->is_constructor && !self_value_and_holder.holder_constructed()) {
auto *pi = reinterpret_cast<instance *>(parent.ptr());
self_value_and_holder.type->init_instance(pi, nullptr);
}
return result.ptr();
}
if (overloads->is_constructor && !self_value_and_holder.holder_constructed()) {
auto *pi = reinterpret_cast<instance *>(parent.ptr());
self_value_and_holder.type->init_instance(pi, nullptr);
}
return result.ptr();
}
};

Expand Down Expand Up @@ -1860,9 +1860,9 @@ PYBIND11_NOINLINE inline void keep_alive_impl(size_t Nurse, size_t Patient, func
auto get_arg = [&](size_t n) {
if (n == 0)
return ret;
else if (n == 1 && call.init_self)
if (n == 1 && call.init_self)
return call.init_self;
else if (n <= call.args.size())
if (n <= call.args.size())
return call.args[n - 1];
return handle();
};
Expand Down Expand Up @@ -2186,18 +2186,19 @@ template <class T> function get_override(const T *this_ptr, const char *name) {
return tinfo ? detail::get_type_override(this_ptr, tinfo, name) : function();
}

#define PYBIND11_OVERRIDE_IMPL(ret_type, cname, name, ...) \
do { \
pybind11::gil_scoped_acquire gil; \
pybind11::function override = pybind11::get_override(static_cast<const cname *>(this), name); \
if (override) { \
auto o = override(__VA_ARGS__); \
if (pybind11::detail::cast_is_temporary_value_reference<ret_type>::value) { \
static pybind11::detail::override_caster_t<ret_type> caster; \
return pybind11::detail::cast_ref<ret_type>(std::move(o), caster); \
} \
else return pybind11::detail::cast_safe<ret_type>(std::move(o)); \
} \
#define PYBIND11_OVERRIDE_IMPL(ret_type, cname, name, ...) \
do { \
pybind11::gil_scoped_acquire gil; \
pybind11::function override \
= pybind11::get_override(static_cast<const cname *>(this), name); \
if (override) { \
auto o = override(__VA_ARGS__); \
if (pybind11::detail::cast_is_temporary_value_reference<ret_type>::value) { \
static pybind11::detail::override_caster_t<ret_type> caster; \
return pybind11::detail::cast_ref<ret_type>(std::move(o), caster); \
} \
return pybind11::detail::cast_safe<ret_type>(std::move(o)); \
} \
} while (false)

/** \rst
Expand Down
Loading