Skip to content

Commit b5357d1

Browse files
Skylion007Skylion007
authored
fix(clang-tidy): Enable clang-tidy else-after-return and redundant void checks (#3080)
* Enable clang-tidy else-after-return and redundant void checks * Fix remaining else-after * Address reviewer comments * Fix indentation * Rerun clang-tidy post merge
1 parent 6d1b197 commit b5357d1

17 files changed

+114
-115
lines changed

‎.clang-tidy

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
FormatStyle: file
22

33
Checks: '
4+
clang-analyzer-optin.cplusplus.VirtualCall,
45
llvm-namespace-comment,
56
misc-misplaced-const,
67
misc-static-assert,
78
misc-uniqueptr-reset-release,
89
modernize-avoid-bind,
10+
modernize-redundant-void-arg,
911
modernize-replace-auto-ptr,
1012
modernize-replace-disallow-copy-and-assign-macro,
1113
modernize-shrink-to-fit,
@@ -20,6 +22,7 @@ modernize-use-override,
2022
modernize-use-using,
2123
*performance*,
2224
readability-container-size-empty,
25+
readability-else-after-return,
2326
readability-make-member-function-const,
2427
readability-redundant-function-ptr-dereference,
2528
readability-redundant-smartptr-get,

‎include/pybind11/cast.h

Lines changed: 47 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -85,25 +85,28 @@ template <typename type> class type_caster<std::reference_wrapper<type>> {
8585
operator std::reference_wrapper<type>() { return cast_op<type &>(subcaster); }
8686
};
8787

88-
#define PYBIND11_TYPE_CASTER(type, py_name) \
89-
protected: \
90-
type value; \
91-
public: \
92-
static constexpr auto name = py_name; \
93-
template <typename T_, enable_if_t<std::is_same<type, remove_cv_t<T_>>::value, int> = 0> \
94-
static handle cast(T_ *src, return_value_policy policy, handle parent) { \
95-
if (!src) return none().release(); \
96-
if (policy == return_value_policy::take_ownership) { \
97-
auto h = cast(std::move(*src), policy, parent); delete src; return h; \
98-
} else { \
99-
return cast(*src, policy, parent); \
100-
} \
101-
} \
102-
operator type*() { return &value; } \
103-
operator type&() { return value; } \
104-
operator type&&() && { return std::move(value); } \
105-
template <typename T_> using cast_op_type = pybind11::detail::movable_cast_op_type<T_>
106-
88+
#define PYBIND11_TYPE_CASTER(type, py_name) \
89+
protected: \
90+
type value; \
91+
\
92+
public: \
93+
static constexpr auto name = py_name; \
94+
template <typename T_, enable_if_t<std::is_same<type, remove_cv_t<T_>>::value, int> = 0> \
95+
static handle cast(T_ *src, return_value_policy policy, handle parent) { \
96+
if (!src) \
97+
return none().release(); \
98+
if (policy == return_value_policy::take_ownership) { \
99+
auto h = cast(std::move(*src), policy, parent); \
100+
delete src; \
101+
return h; \
102+
} \
103+
return cast(*src, policy, parent); \
104+
} \
105+
operator type *() { return &value; } \
106+
operator type &() { return value; } \
107+
operator type &&() && { return std::move(value); } \
108+
template <typename T_> \
109+
using cast_op_type = pybind11::detail::movable_cast_op_type<T_>
107110

108111
template <typename CharT> using is_std_char_type = any_of<
109112
std::is_same<CharT, char>, /* std::string */
@@ -247,7 +250,8 @@ template <> class type_caster<void> : public type_caster<void_type> {
247250
bool load(handle h, bool) {
248251
if (!h) {
249252
return false;
250-
} else if (h.is_none()) {
253+
}
254+
if (h.is_none()) {
251255
value = nullptr;
252256
return true;
253257
}
@@ -272,8 +276,7 @@ template <> class type_caster<void> : public type_caster<void_type> {
272276
static handle cast(const void *ptr, return_value_policy /* policy */, handle /* parent */) {
273277
if (ptr)
274278
return capsule(ptr).release();
275-
else
276-
return none().inc_ref();
279+
return none().inc_ref();
277280
}
278281

279282
template <typename T> using cast_op_type = void*&;
@@ -289,9 +292,15 @@ template <> class type_caster<bool> {
289292
public:
290293
bool load(handle src, bool convert) {
291294
if (!src) return false;
292-
else if (src.ptr() == Py_True) { value = true; return true; }
293-
else if (src.ptr() == Py_False) { value = false; return true; }
294-
else if (convert || !std::strcmp("numpy.bool_", Py_TYPE(src.ptr())->tp_name)) {
295+
if (src.ptr() == Py_True) {
296+
value = true;
297+
return true;
298+
}
299+
if (src.ptr() == Py_False) {
300+
value = false;
301+
return true;
302+
}
303+
if (convert || !std::strcmp("numpy.bool_", Py_TYPE(src.ptr())->tp_name)) {
295304
// (allow non-implicit conversion for numpy booleans)
296305

297306
Py_ssize_t res = -1;
@@ -315,9 +324,8 @@ template <> class type_caster<bool> {
315324
if (res == 0 || res == 1) {
316325
value = (bool) res;
317326
return true;
318-
} else {
319-
PyErr_Clear();
320327
}
328+
PyErr_Clear();
321329
}
322330
return false;
323331
}
@@ -351,7 +359,8 @@ template <typename StringType, bool IsView = false> struct string_caster {
351359
handle load_src = src;
352360
if (!src) {
353361
return false;
354-
} else if (!PyUnicode_Check(load_src.ptr())) {
362+
}
363+
if (!PyUnicode_Check(load_src.ptr())) {
355364
#if PY_MAJOR_VERSION >= 3
356365
return load_bytes(load_src);
357366
#else
@@ -554,10 +563,11 @@ template <template<typename...> class Tuple, typename... Ts> class tuple_caster
554563
static handle cast(T *src, return_value_policy policy, handle parent) {
555564
if (!src) return none().release();
556565
if (policy == return_value_policy::take_ownership) {
557-
auto h = cast(std::move(*src), policy, parent); delete src; return h;
558-
} else {
559-
return cast(*src, policy, parent);
566+
auto h = cast(std::move(*src), policy, parent);
567+
delete src;
568+
return h;
560569
}
570+
return cast(*src, policy, parent);
561571
}
562572

563573
static constexpr auto name = _("Tuple[") + concat(make_caster<Ts>::name...) + _("]");
@@ -664,14 +674,14 @@ struct copyable_holder_caster : public type_caster_base<type> {
664674
value = v_h.value_ptr();
665675
holder = v_h.template holder<holder_type>();
666676
return true;
667-
} else {
668-
throw cast_error("Unable to cast from non-held to held instance (T& to Holder<T>) "
677+
}
678+
throw cast_error("Unable to cast from non-held to held instance (T& to Holder<T>) "
669679
#if defined(NDEBUG)
670-
"(compile in debug mode for type information)");
680+
"(compile in debug mode for type information)");
671681
#else
672-
"of type '" + type_id<holder_type>() + "''");
682+
"of type '"
683+
+ type_id<holder_type>() + "''");
673684
#endif
674-
}
675685
}
676686

677687
template <typename T = holder_type, detail::enable_if_t<!std::is_constructible<T, const T &, type*>::value, int> = 0>
@@ -917,8 +927,7 @@ template <typename T> detail::enable_if_t<detail::move_always<T>::value, T> cast
917927
template <typename T> detail::enable_if_t<detail::move_if_unreferenced<T>::value, T> cast(object &&object) {
918928
if (object.ref_count() > 1)
919929
return cast<T>(object);
920-
else
921-
return move<T>(std::move(object));
930+
return move<T>(std::move(object));
922931
}
923932
template <typename T> detail::enable_if_t<detail::move_never<T>::value, T> cast(object &&object) {
924933
return cast<T>(object);

‎include/pybind11/chrono.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,11 @@ template <typename type> class duration_caster {
5353
return true;
5454
}
5555
// If invoked with a float we assume it is seconds and convert
56-
else if (PyFloat_Check(src.ptr())) {
56+
if (PyFloat_Check(src.ptr())) {
5757
value = type(duration_cast<duration<rep, period>>(duration<double>(PyFloat_AsDouble(src.ptr()))));
5858
return true;
5959
}
60-
else return false;
60+
return false;
6161
}
6262

6363
// If this is a duration just return it back

‎include/pybind11/detail/class.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -162,9 +162,7 @@ extern "C" inline PyObject *pybind11_meta_getattro(PyObject *obj, PyObject *name
162162
Py_INCREF(descr);
163163
return descr;
164164
}
165-
else {
166-
return PyType_Type.tp_getattro(obj, name);
167-
}
165+
return PyType_Type.tp_getattro(obj, name);
168166
}
169167
#endif
170168

‎include/pybind11/detail/type_caster_base.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -670,7 +670,7 @@ class type_caster_generic {
670670
return true;
671671
}
672672
// Case 2: We have a derived class
673-
else if (PyType_IsSubtype(srctype, typeinfo->type)) {
673+
if (PyType_IsSubtype(srctype, typeinfo->type)) {
674674
auto &bases = all_type_info(srctype);
675675
bool no_cpp_mi = typeinfo->simple_type;
676676

@@ -687,7 +687,7 @@ class type_caster_generic {
687687
// Case 2b: the python type inherits from multiple C++ bases. Check the bases to see if
688688
// we can find an exact match (or, for a simple C++ type, an inherited match); if so, we
689689
// can safely reinterpret_cast to the relevant pointer.
690-
else if (bases.size() > 1) {
690+
if (bases.size() > 1) {
691691
for (auto base : bases) {
692692
if (no_cpp_mi ? PyType_IsSubtype(base->type, typeinfo->type) : base->type == typeinfo->type) {
693693
this_.load_value(reinterpret_cast<instance *>(src.ptr())->get_value_and_holder(base));

‎include/pybind11/eigen.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -169,21 +169,18 @@ template <typename Type_> struct EigenProps {
169169
return false; // Vector size mismatch
170170
return {rows == 1 ? 1 : n, cols == 1 ? 1 : n, stride};
171171
}
172-
else if (fixed) {
172+
if (fixed) {
173173
// The type has a fixed size, but is not a vector: abort
174174
return false;
175175
}
176-
else if (fixed_cols) {
176+
if (fixed_cols) {
177177
// Since this isn't a vector, cols must be != 1. We allow this only if it exactly
178178
// equals the number of elements (rows is Dynamic, and so 1 row is allowed).
179179
if (cols != n) return false;
180180
return {1, n, stride};
181-
}
182-
else {
183-
// Otherwise it's either fully dynamic, or column dynamic; both become a column vector
181+
} // Otherwise it's either fully dynamic, or column dynamic; both become a column vector
184182
if (fixed_rows && rows != n) return false;
185183
return {n, 1, stride};
186-
}
187184
}
188185

189186
static constexpr bool show_writeable = is_eigen_dense_map<Type>::value && is_eigen_mutable_map<Type>::value;

‎include/pybind11/functional.h

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -98,8 +98,7 @@ struct type_caster<std::function<Return(Args...)>> {
9898
auto result = f_.template target<function_type>();
9999
if (result)
100100
return cpp_function(*result, policy).release();
101-
else
102-
return cpp_function(std::forward<Func>(f_), policy).release();
101+
return cpp_function(std::forward<Func>(f_), policy).release();
103102
}
104103

105104
PYBIND11_TYPE_CASTER(type, _("Callable[[") + concat(make_caster<Args>::name...) + _("], ")

‎include/pybind11/numpy.h

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,9 +1340,8 @@ template <size_t N> class multi_array_iterator {
13401340
if (++m_index[i] != m_shape[i]) {
13411341
increment_common_iterator(i);
13421342
break;
1343-
} else {
1344-
m_index[i] = 0;
13451343
}
1344+
m_index[i] = 0;
13461345
}
13471346
return *this;
13481347
}
@@ -1493,8 +1492,7 @@ struct vectorize_returned_array {
14931492
static Type create(broadcast_trivial trivial, const std::vector<ssize_t> &shape) {
14941493
if (trivial == broadcast_trivial::f_trivial)
14951494
return array_t<Return, array::f_style>(shape);
1496-
else
1497-
return array_t<Return>(shape);
1495+
return array_t<Return>(shape);
14981496
}
14991497

15001498
static Return *mutable_data(Type &array) {

‎include/pybind11/pybind11.h

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -396,7 +396,7 @@ class cpp_function : public function {
396396
std::memset(rec->def, 0, sizeof(PyMethodDef));
397397
rec->def->ml_name = rec->name;
398398
rec->def->ml_meth
399-
= reinterpret_cast<PyCFunction>(reinterpret_cast<void (*)(void)>(dispatcher));
399+
= reinterpret_cast<PyCFunction>(reinterpret_cast<void (*)()>(dispatcher));
400400
rec->def->ml_flags = METH_VARARGS | METH_KEYWORDS;
401401

402402
capsule rec_capsule(unique_rec.release(), [](void *ptr) {
@@ -924,20 +924,20 @@ class cpp_function : public function {
924924
append_note_if_missing_header_is_suspected(msg);
925925
PyErr_SetString(PyExc_TypeError, msg.c_str());
926926
return nullptr;
927-
} else if (!result) {
927+
}
928+
if (!result) {
928929
std::string msg = "Unable to convert function return value to a "
929930
"Python type! The signature was\n\t";
930931
msg += it->signature;
931932
append_note_if_missing_header_is_suspected(msg);
932933
PyErr_SetString(PyExc_TypeError, msg.c_str());
933934
return nullptr;
934-
} else {
935-
if (overloads->is_constructor && !self_value_and_holder.holder_constructed()) {
936-
auto *pi = reinterpret_cast<instance *>(parent.ptr());
937-
self_value_and_holder.type->init_instance(pi, nullptr);
938-
}
939-
return result.ptr();
940935
}
936+
if (overloads->is_constructor && !self_value_and_holder.holder_constructed()) {
937+
auto *pi = reinterpret_cast<instance *>(parent.ptr());
938+
self_value_and_holder.type->init_instance(pi, nullptr);
939+
}
940+
return result.ptr();
941941
}
942942
};
943943

@@ -1860,9 +1860,9 @@ PYBIND11_NOINLINE inline void keep_alive_impl(size_t Nurse, size_t Patient, func
18601860
auto get_arg = [&](size_t n) {
18611861
if (n == 0)
18621862
return ret;
1863-
else if (n == 1 && call.init_self)
1863+
if (n == 1 && call.init_self)
18641864
return call.init_self;
1865-
else if (n <= call.args.size())
1865+
if (n <= call.args.size())
18661866
return call.args[n - 1];
18671867
return handle();
18681868
};
@@ -2186,18 +2186,19 @@ template <class T> function get_override(const T *this_ptr, const char *name) {
21862186
return tinfo ? detail::get_type_override(this_ptr, tinfo, name) : function();
21872187
}
21882188

2189-
#define PYBIND11_OVERRIDE_IMPL(ret_type, cname, name, ...) \
2190-
do { \
2191-
pybind11::gil_scoped_acquire gil; \
2192-
pybind11::function override = pybind11::get_override(static_cast<const cname *>(this), name); \
2193-
if (override) { \
2194-
auto o = override(__VA_ARGS__); \
2195-
if (pybind11::detail::cast_is_temporary_value_reference<ret_type>::value) { \
2196-
static pybind11::detail::override_caster_t<ret_type> caster; \
2197-
return pybind11::detail::cast_ref<ret_type>(std::move(o), caster); \
2198-
} \
2199-
else return pybind11::detail::cast_safe<ret_type>(std::move(o)); \
2200-
} \
2189+
#define PYBIND11_OVERRIDE_IMPL(ret_type, cname, name, ...) \
2190+
do { \
2191+
pybind11::gil_scoped_acquire gil; \
2192+
pybind11::function override \
2193+
= pybind11::get_override(static_cast<const cname *>(this), name); \
2194+
if (override) { \
2195+
auto o = override(__VA_ARGS__); \
2196+
if (pybind11::detail::cast_is_temporary_value_reference<ret_type>::value) { \
2197+
static pybind11::detail::override_caster_t<ret_type> caster; \
2198+
return pybind11::detail::cast_ref<ret_type>(std::move(o), caster); \
2199+
} \
2200+
return pybind11::detail::cast_safe<ret_type>(std::move(o)); \
2201+
} \
22012202
} while (false)
22022203

22032204
/** \rst

0 commit comments

Comments
 (0)