Skip to content

Commit 3893f37

Browse files
Skylion007Skylion007
authored
maint(clang-tidy): Bugprone enable checks (#3166)
* Enable bugprone checks * Reset delta and massage config * Start to apply bugprone fixes * try to fix minor bug * Fix later * Fix perfect forwarding bugprone * Remove nolint * undo constructor delete * Fix bugprone-perfect-forwarding again * Remove TODO * Add another nolint for bugprone-exception-escape in scoped interpreter * Fix remaining bugprone errors * Properly apply bugprone-macro-parantheses * Redo formatting and remove bugprone nolint * Add coment and revert more whitespace changes * Fix typo * Fix parsing bug * Add back comma * Fix clang-tidy issue * Apply remaining clang-tidy fixes
1 parent 089328f commit 3893f37

15 files changed

+85
-68
lines changed

‎.clang-tidy

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

33
Checks: '
4+
*bugprone*,
45
cppcoreguidelines-init-variables,
56
clang-analyzer-optin.cplusplus.VirtualCall,
67
llvm-namespace-comment,
@@ -43,6 +44,9 @@ readability-static-accessed-through-instance,
4344
readability-static-definition-in-anonymous-namespace,
4445
readability-string-compare,
4546
readability-uniqueptr-delete-release,
47+
-bugprone-exception-escape,
48+
-bugprone-reserved-identifier,
49+
-bugprone-unused-raii,
4650
'
4751

4852
CheckOptions:

‎include/pybind11/cast.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,9 @@ public:
102102
} \
103103
return cast(*src, policy, parent); \
104104
} \
105-
operator type *() { return &value; } \
106-
operator type &() { return value; } \
107-
operator type &&() && { return std::move(value); } \
105+
operator type *() { return &value; } /* NOLINT(bugprone-macro-parentheses) */ \
106+
operator type &() { return value; } /* NOLINT(bugprone-macro-parentheses) */ \
107+
operator type &&() && { return std::move(value); } /* NOLINT(bugprone-macro-parentheses) */ \
108108
template <typename T_> \
109109
using cast_op_type = pybind11::detail::movable_cast_op_type<T_>
110110

@@ -145,9 +145,8 @@ struct type_caster<T, enable_if_t<std::is_arithmetic<T>::value && !is_std_char_t
145145
py_value = (py_type) PyFloat_AsDouble(src.ptr());
146146
else
147147
return false;
148-
} else if (PyFloat_Check(src.ptr())) {
149-
return false;
150-
} else if (!convert && !PYBIND11_LONG_CHECK(src.ptr()) && !index_check(src.ptr())) {
148+
} else if (PyFloat_Check(src.ptr())
149+
|| (!convert && !PYBIND11_LONG_CHECK(src.ptr()) && !index_check(src.ptr()))) {
151150
return false;
152151
} else {
153152
handle src_or_index = src;

‎include/pybind11/detail/common.h

Lines changed: 19 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -220,8 +220,8 @@
220220
#define PYBIND11_BYTES_SIZE PyBytes_Size
221221
#define PYBIND11_LONG_CHECK(o) PyLong_Check(o)
222222
#define PYBIND11_LONG_AS_LONGLONG(o) PyLong_AsLongLong(o)
223-
#define PYBIND11_LONG_FROM_SIGNED(o) PyLong_FromSsize_t((ssize_t) o)
224-
#define PYBIND11_LONG_FROM_UNSIGNED(o) PyLong_FromSize_t((size_t) o)
223+
#define PYBIND11_LONG_FROM_SIGNED(o) PyLong_FromSsize_t((ssize_t) (o))
224+
#define PYBIND11_LONG_FROM_UNSIGNED(o) PyLong_FromSize_t((size_t) (o))
225225
#define PYBIND11_BYTES_NAME "bytes"
226226
#define PYBIND11_STRING_NAME "str"
227227
#define PYBIND11_SLICE_OBJECT PyObject
@@ -356,24 +356,23 @@ extern "C" {
356356
});
357357
}
358358
\endrst */
359-
#define PYBIND11_MODULE(name, variable) \
360-
static ::pybind11::module_::module_def \
361-
PYBIND11_CONCAT(pybind11_module_def_, name) PYBIND11_MAYBE_UNUSED; \
362-
PYBIND11_MAYBE_UNUSED \
363-
static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \
364-
PYBIND11_PLUGIN_IMPL(name) { \
365-
PYBIND11_CHECK_PYTHON_VERSION \
366-
PYBIND11_ENSURE_INTERNALS_READY \
367-
auto m = ::pybind11::module_::create_extension_module( \
368-
PYBIND11_TOSTRING(name), nullptr, \
369-
&PYBIND11_CONCAT(pybind11_module_def_, name)); \
370-
try { \
371-
PYBIND11_CONCAT(pybind11_init_, name)(m); \
372-
return m.ptr(); \
373-
} PYBIND11_CATCH_INIT_EXCEPTIONS \
374-
} \
375-
void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &variable)
376-
359+
#define PYBIND11_MODULE(name, variable) \
360+
static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name) \
361+
PYBIND11_MAYBE_UNUSED; \
362+
PYBIND11_MAYBE_UNUSED \
363+
static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \
364+
PYBIND11_PLUGIN_IMPL(name) { \
365+
PYBIND11_CHECK_PYTHON_VERSION \
366+
PYBIND11_ENSURE_INTERNALS_READY \
367+
auto m = ::pybind11::module_::create_extension_module( \
368+
PYBIND11_TOSTRING(name), nullptr, &PYBIND11_CONCAT(pybind11_module_def_, name)); \
369+
try { \
370+
PYBIND11_CONCAT(pybind11_init_, name)(m); \
371+
return m.ptr(); \
372+
} \
373+
PYBIND11_CATCH_INIT_EXCEPTIONS \
374+
} \
375+
void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ & (variable))
377376

378377
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
379378

‎include/pybind11/embed.h

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -45,25 +45,23 @@
4545
});
4646
}
4747
\endrst */
48-
#define PYBIND11_EMBEDDED_MODULE(name, variable) \
49-
static ::pybind11::module_::module_def \
50-
PYBIND11_CONCAT(pybind11_module_def_, name); \
51-
static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \
52-
static PyObject PYBIND11_CONCAT(*pybind11_init_wrapper_, name)() { \
53-
auto m = ::pybind11::module_::create_extension_module( \
54-
PYBIND11_TOSTRING(name), nullptr, \
55-
&PYBIND11_CONCAT(pybind11_module_def_, name)); \
56-
try { \
57-
PYBIND11_CONCAT(pybind11_init_, name)(m); \
58-
return m.ptr(); \
59-
} PYBIND11_CATCH_INIT_EXCEPTIONS \
60-
} \
61-
PYBIND11_EMBEDDED_MODULE_IMPL(name) \
62-
::pybind11::detail::embedded_module PYBIND11_CONCAT(pybind11_module_, name) \
63-
(PYBIND11_TOSTRING(name), \
64-
PYBIND11_CONCAT(pybind11_init_impl_, name)); \
65-
void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &variable)
66-
48+
#define PYBIND11_EMBEDDED_MODULE(name, variable) \
49+
static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name); \
50+
static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \
51+
static PyObject PYBIND11_CONCAT(*pybind11_init_wrapper_, name)() { \
52+
auto m = ::pybind11::module_::create_extension_module( \
53+
PYBIND11_TOSTRING(name), nullptr, &PYBIND11_CONCAT(pybind11_module_def_, name)); \
54+
try { \
55+
PYBIND11_CONCAT(pybind11_init_, name)(m); \
56+
return m.ptr(); \
57+
} \
58+
PYBIND11_CATCH_INIT_EXCEPTIONS \
59+
} \
60+
PYBIND11_EMBEDDED_MODULE_IMPL(name) \
61+
::pybind11::detail::embedded_module PYBIND11_CONCAT(pybind11_module_, name)( \
62+
PYBIND11_TOSTRING(name), PYBIND11_CONCAT(pybind11_init_impl_, name)); \
63+
void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ \
64+
& variable) // NOLINT(bugprone-macro-parentheses)
6765

6866
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
6967
PYBIND11_NAMESPACE_BEGIN(detail)

‎include/pybind11/numpy.h

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,8 +1551,11 @@ struct vectorize_helper {
15511551
"pybind11::vectorize(...) requires a function with at least one vectorizable argument");
15521552

15531553
public:
1554-
template <typename T>
1555-
explicit vectorize_helper(T &&f) : f(std::forward<T>(f)) { }
1554+
template <typename T,
1555+
// SFINAE to prevent shadowing the copy constructor.
1556+
typename = detail::enable_if_t<
1557+
!std::is_same<vectorize_helper, typename std::decay<T>::type>::value>>
1558+
explicit vectorize_helper(T &&f) : f(std::forward<T>(f)) {}
15561559

15571560
object operator()(typename vectorize_arg<Args>::type... args) {
15581561
return run(args...,

‎include/pybind11/pybind11.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1726,7 +1726,7 @@ struct enum_base {
17261726
m_base.attr(op) = cpp_function( \
17271727
[](const object &a, const object &b) { \
17281728
if (!type::handle_of(a).is(type::handle_of(b))) \
1729-
strict_behavior; \
1729+
strict_behavior; /* NOLINT(bugprone-macro-parentheses) */ \
17301730
return expr; \
17311731
}, \
17321732
name(op), \

‎tests/object.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,11 @@ template <typename T> class ref {
110110

111111
/// Overwrite this reference with another reference
112112
ref& operator=(const ref& r) {
113-
print_copy_assigned(this, "pointer", r.m_ptr); track_copy_assigned((ref_tag*) this);
113+
if (this == &r) {
114+
return *this;
115+
}
116+
print_copy_assigned(this, "pointer", r.m_ptr);
117+
track_copy_assigned((ref_tag *) this);
114118

115119
if (m_ptr == r.m_ptr)
116120
return *this;

‎tests/pybind11_tests.h

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,10 @@ class test_initializer {
2323
test_initializer(const char *submodule_name, Initializer init);
2424
};
2525

26-
#define TEST_SUBMODULE(name, variable) \
27-
void test_submodule_##name(py::module_ &); \
28-
test_initializer name(#name, test_submodule_##name); \
29-
void test_submodule_##name(py::module_ &variable)
30-
26+
#define TEST_SUBMODULE(name, variable) \
27+
void test_submodule_##name(py::module_ &); \
28+
test_initializer name(#name, test_submodule_##name); \
29+
void test_submodule_##name(py::module_ &(variable))
3130

3231
/// Dummy type which is not exported anywhere -- something to trigger a conversion error
3332
struct UnregisteredType { };

‎tests/test_buffers.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ TEST_SUBMODULE(buffers, m) {
4040
}
4141

4242
Matrix &operator=(const Matrix &s) {
43-
print_copy_assigned(this, std::to_string(m_rows) + "x" + std::to_string(m_cols) + " matrix");
43+
if (this == &s) {
44+
return *this;
45+
}
46+
print_copy_assigned(this,
47+
std::to_string(m_rows) + "x" + std::to_string(m_cols) + " matrix");
4448
delete[] m_data;
4549
m_rows = s.m_rows;
4650
m_cols = s.m_cols;

‎tests/test_class.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -492,15 +492,15 @@ using DoesntBreak5 = py::class_<BreaksBase<5>>;
492492
using DoesntBreak6 = py::class_<BreaksBase<6>, std::shared_ptr<BreaksBase<6>>, BreaksTramp<6>>;
493493
using DoesntBreak7 = py::class_<BreaksBase<7>, BreaksTramp<7>, std::shared_ptr<BreaksBase<7>>>;
494494
using DoesntBreak8 = py::class_<BreaksBase<8>, std::shared_ptr<BreaksBase<8>>>;
495-
#define CHECK_BASE(N) static_assert(std::is_same<typename DoesntBreak##N::type, BreaksBase<N>>::value, \
495+
#define CHECK_BASE(N) static_assert(std::is_same<typename DoesntBreak##N::type, BreaksBase<(N)>>::value, \
496496
"DoesntBreak" #N " has wrong type!")
497497
CHECK_BASE(1); CHECK_BASE(2); CHECK_BASE(3); CHECK_BASE(4); CHECK_BASE(5); CHECK_BASE(6); CHECK_BASE(7); CHECK_BASE(8);
498-
#define CHECK_ALIAS(N) static_assert(DoesntBreak##N::has_alias && std::is_same<typename DoesntBreak##N::type_alias, BreaksTramp<N>>::value, \
498+
#define CHECK_ALIAS(N) static_assert(DoesntBreak##N::has_alias && std::is_same<typename DoesntBreak##N::type_alias, BreaksTramp<(N)>>::value, \
499499
"DoesntBreak" #N " has wrong type_alias!")
500500
#define CHECK_NOALIAS(N) static_assert(!DoesntBreak##N::has_alias && std::is_void<typename DoesntBreak##N::type_alias>::value, \
501501
"DoesntBreak" #N " has type alias, but shouldn't!")
502502
CHECK_ALIAS(1); CHECK_ALIAS(2); CHECK_NOALIAS(3); CHECK_ALIAS(4); CHECK_NOALIAS(5); CHECK_ALIAS(6); CHECK_ALIAS(7); CHECK_NOALIAS(8);
503-
#define CHECK_HOLDER(N, TYPE) static_assert(std::is_same<typename DoesntBreak##N::holder_type, std::TYPE##_ptr<BreaksBase<N>>>::value, \
503+
#define CHECK_HOLDER(N, TYPE) static_assert(std::is_same<typename DoesntBreak##N::holder_type, std::TYPE##_ptr<BreaksBase<(N)>>>::value, \
504504
"DoesntBreak" #N " has wrong holder_type!")
505505
CHECK_HOLDER(1, unique); CHECK_HOLDER(2, unique); CHECK_HOLDER(3, unique); CHECK_HOLDER(4, unique); CHECK_HOLDER(5, unique);
506506
CHECK_HOLDER(6, shared); CHECK_HOLDER(7, shared); CHECK_HOLDER(8, shared);
@@ -510,7 +510,7 @@ CHECK_HOLDER(6, shared); CHECK_HOLDER(7, shared); CHECK_HOLDER(8, shared);
510510
// failures occurs).
511511

512512
// We have to actually look into the type: the typedef alone isn't enough to instantiate the type:
513-
#define CHECK_BROKEN(N) static_assert(std::is_same<typename Breaks##N::type, BreaksBase<-N>>::value, \
513+
#define CHECK_BROKEN(N) static_assert(std::is_same<typename Breaks##N::type, BreaksBase<-(N)>>::value, \
514514
"Breaks1 has wrong type!");
515515

516516
//// Two holder classes:

‎tests/test_multiple_inheritance.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,10 @@ TEST_SUBMODULE(multiple_inheritance, m) {
108108

109109

110110
// test_multiple_inheritance_python_many_bases
111-
#define PYBIND11_BASEN(N) py::class_<BaseN<N>>(m, "BaseN" #N).def(py::init<int>()).def("f" #N, [](BaseN<N> &b) { return b.i + N; })
111+
#define PYBIND11_BASEN(N) \
112+
py::class_<BaseN<(N)>>(m, "BaseN" #N).def(py::init<int>()).def("f" #N, [](BaseN<N> &b) { \
113+
return b.i + (N); \
114+
})
112115
PYBIND11_BASEN( 1); PYBIND11_BASEN( 2); PYBIND11_BASEN( 3); PYBIND11_BASEN( 4);
113116
PYBIND11_BASEN( 5); PYBIND11_BASEN( 6); PYBIND11_BASEN( 7); PYBIND11_BASEN( 8);
114117
PYBIND11_BASEN( 9); PYBIND11_BASEN(10); PYBIND11_BASEN(11); PYBIND11_BASEN(12);

‎tests/test_numpy_dtypes.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -148,11 +148,13 @@ py::array mkarray_via_buffer(size_t n) {
148148
1, { n }, { sizeof(T) }));
149149
}
150150

151-
#define SET_TEST_VALS(s, i) do { \
152-
s.bool_ = (i) % 2 != 0; \
153-
s.uint_ = (uint32_t) (i); \
154-
s.float_ = (float) (i) * 1.5f; \
155-
s.ldbl_ = (long double) (i) * -2.5L; } while (0)
151+
#define SET_TEST_VALS(s, i) \
152+
do { \
153+
(s).bool_ = (i) % 2 != 0; \
154+
(s).uint_ = (uint32_t) (i); \
155+
(s).float_ = (float) (i) *1.5f; \
156+
(s).ldbl_ = (long double) (i) * -2.5L; \
157+
} while (0)
156158

157159
template <typename S>
158160
py::array_t<S, 0> create_recarray(size_t n) {

‎tests/test_smart_ptr.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ struct SharedPtrRef {
182182
struct SharedFromThisRef {
183183
struct B : std::enable_shared_from_this<B> {
184184
B() { print_created(this); }
185+
// NOLINTNEXTLINE(bugprone-copy-constructor-init)
185186
B(const B &) : std::enable_shared_from_this<B>() { print_copy_created(this); }
186187
B(B &&) noexcept : std::enable_shared_from_this<B>() { print_move_created(this); }
187188
~B() { print_destroyed(this); }

‎tests/test_tagbased_polymorphic.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,13 @@ std::vector<std::unique_ptr<Animal>> create_zoo()
8686
const std::type_info* Animal::type_of_kind(Kind kind)
8787
{
8888
switch (kind) {
89-
case Kind::Unknown: break;
90-
89+
case Kind::Unknown:
9190
case Kind::Dog: break;
91+
9292
case Kind::Labrador: return &typeid(Labrador);
9393
case Kind::Chihuahua: return &typeid(Chihuahua);
94-
case Kind::LastDog: break;
9594

95+
case Kind::LastDog:
9696
case Kind::Cat: break;
9797
case Kind::Panther: return &typeid(Panther);
9898
case Kind::LastCat: break;

‎tests/test_virtual_functions.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ template <class Base = B_Tpl>
454454
class PyB_Tpl : public PyA_Tpl<Base> {
455455
public:
456456
using PyA_Tpl<Base>::PyA_Tpl; // Inherit constructors (via PyA_Tpl's inherited constructors)
457+
// NOLINTNEXTLINE(bugprone-parent-virtual-call)
457458
int unlucky_number() override { PYBIND11_OVERRIDE(int, Base, unlucky_number, ); }
458459
double lucky_number() override { PYBIND11_OVERRIDE(double, Base, lucky_number, ); }
459460
};

0 commit comments

Comments
 (0)