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
This commit is contained in:
Aaron Gokaslan 2021-08-06 14:30:28 -04:00 committed by GitHub
parent 089328f779
commit 3893f37bce
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 85 additions and 68 deletions

View File

@ -1,6 +1,7 @@
FormatStyle: file FormatStyle: file
Checks: ' Checks: '
*bugprone*,
cppcoreguidelines-init-variables, cppcoreguidelines-init-variables,
clang-analyzer-optin.cplusplus.VirtualCall, clang-analyzer-optin.cplusplus.VirtualCall,
llvm-namespace-comment, llvm-namespace-comment,
@ -43,6 +44,9 @@ readability-static-accessed-through-instance,
readability-static-definition-in-anonymous-namespace, readability-static-definition-in-anonymous-namespace,
readability-string-compare, readability-string-compare,
readability-uniqueptr-delete-release, readability-uniqueptr-delete-release,
-bugprone-exception-escape,
-bugprone-reserved-identifier,
-bugprone-unused-raii,
' '
CheckOptions: CheckOptions:

View File

@ -102,9 +102,9 @@ public:
} \ } \
return cast(*src, policy, parent); \ return cast(*src, policy, parent); \
} \ } \
operator type *() { return &value; } \ operator type *() { return &value; } /* NOLINT(bugprone-macro-parentheses) */ \
operator type &() { return value; } \ operator type &() { return value; } /* NOLINT(bugprone-macro-parentheses) */ \
operator type &&() && { return std::move(value); } \ operator type &&() && { return std::move(value); } /* NOLINT(bugprone-macro-parentheses) */ \
template <typename T_> \ template <typename T_> \
using cast_op_type = pybind11::detail::movable_cast_op_type<T_> using cast_op_type = pybind11::detail::movable_cast_op_type<T_>
@ -145,9 +145,8 @@ public:
py_value = (py_type) PyFloat_AsDouble(src.ptr()); py_value = (py_type) PyFloat_AsDouble(src.ptr());
else else
return false; return false;
} else if (PyFloat_Check(src.ptr())) { } else if (PyFloat_Check(src.ptr())
return false; || (!convert && !PYBIND11_LONG_CHECK(src.ptr()) && !index_check(src.ptr()))) {
} else if (!convert && !PYBIND11_LONG_CHECK(src.ptr()) && !index_check(src.ptr())) {
return false; return false;
} else { } else {
handle src_or_index = src; handle src_or_index = src;

View File

@ -220,8 +220,8 @@
#define PYBIND11_BYTES_SIZE PyBytes_Size #define PYBIND11_BYTES_SIZE PyBytes_Size
#define PYBIND11_LONG_CHECK(o) PyLong_Check(o) #define PYBIND11_LONG_CHECK(o) PyLong_Check(o)
#define PYBIND11_LONG_AS_LONGLONG(o) PyLong_AsLongLong(o) #define PYBIND11_LONG_AS_LONGLONG(o) PyLong_AsLongLong(o)
#define PYBIND11_LONG_FROM_SIGNED(o) PyLong_FromSsize_t((ssize_t) o) #define PYBIND11_LONG_FROM_SIGNED(o) PyLong_FromSsize_t((ssize_t) (o))
#define PYBIND11_LONG_FROM_UNSIGNED(o) PyLong_FromSize_t((size_t) o) #define PYBIND11_LONG_FROM_UNSIGNED(o) PyLong_FromSize_t((size_t) (o))
#define PYBIND11_BYTES_NAME "bytes" #define PYBIND11_BYTES_NAME "bytes"
#define PYBIND11_STRING_NAME "str" #define PYBIND11_STRING_NAME "str"
#define PYBIND11_SLICE_OBJECT PyObject #define PYBIND11_SLICE_OBJECT PyObject
@ -356,24 +356,23 @@ extern "C" {
}); });
} }
\endrst */ \endrst */
#define PYBIND11_MODULE(name, variable) \ #define PYBIND11_MODULE(name, variable) \
static ::pybind11::module_::module_def \ static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name) \
PYBIND11_CONCAT(pybind11_module_def_, name) PYBIND11_MAYBE_UNUSED; \ PYBIND11_MAYBE_UNUSED; \
PYBIND11_MAYBE_UNUSED \ PYBIND11_MAYBE_UNUSED \
static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \ static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \
PYBIND11_PLUGIN_IMPL(name) { \ PYBIND11_PLUGIN_IMPL(name) { \
PYBIND11_CHECK_PYTHON_VERSION \ PYBIND11_CHECK_PYTHON_VERSION \
PYBIND11_ENSURE_INTERNALS_READY \ PYBIND11_ENSURE_INTERNALS_READY \
auto m = ::pybind11::module_::create_extension_module( \ auto m = ::pybind11::module_::create_extension_module( \
PYBIND11_TOSTRING(name), nullptr, \ PYBIND11_TOSTRING(name), nullptr, &PYBIND11_CONCAT(pybind11_module_def_, name)); \
&PYBIND11_CONCAT(pybind11_module_def_, name)); \ try { \
try { \ PYBIND11_CONCAT(pybind11_init_, name)(m); \
PYBIND11_CONCAT(pybind11_init_, name)(m); \ return m.ptr(); \
return m.ptr(); \ } \
} PYBIND11_CATCH_INIT_EXCEPTIONS \ PYBIND11_CATCH_INIT_EXCEPTIONS \
} \ } \
void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &variable) void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ & (variable))
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)

View File

@ -45,25 +45,23 @@
}); });
} }
\endrst */ \endrst */
#define PYBIND11_EMBEDDED_MODULE(name, variable) \ #define PYBIND11_EMBEDDED_MODULE(name, variable) \
static ::pybind11::module_::module_def \ static ::pybind11::module_::module_def PYBIND11_CONCAT(pybind11_module_def_, name); \
PYBIND11_CONCAT(pybind11_module_def_, name); \ static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \
static void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &); \ static PyObject PYBIND11_CONCAT(*pybind11_init_wrapper_, name)() { \
static PyObject PYBIND11_CONCAT(*pybind11_init_wrapper_, name)() { \ auto m = ::pybind11::module_::create_extension_module( \
auto m = ::pybind11::module_::create_extension_module( \ PYBIND11_TOSTRING(name), nullptr, &PYBIND11_CONCAT(pybind11_module_def_, name)); \
PYBIND11_TOSTRING(name), nullptr, \ try { \
&PYBIND11_CONCAT(pybind11_module_def_, name)); \ PYBIND11_CONCAT(pybind11_init_, name)(m); \
try { \ return m.ptr(); \
PYBIND11_CONCAT(pybind11_init_, name)(m); \ } \
return m.ptr(); \ PYBIND11_CATCH_INIT_EXCEPTIONS \
} PYBIND11_CATCH_INIT_EXCEPTIONS \ } \
} \ PYBIND11_EMBEDDED_MODULE_IMPL(name) \
PYBIND11_EMBEDDED_MODULE_IMPL(name) \ ::pybind11::detail::embedded_module PYBIND11_CONCAT(pybind11_module_, name)( \
::pybind11::detail::embedded_module PYBIND11_CONCAT(pybind11_module_, name) \ PYBIND11_TOSTRING(name), PYBIND11_CONCAT(pybind11_init_impl_, name)); \
(PYBIND11_TOSTRING(name), \ void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ \
PYBIND11_CONCAT(pybind11_init_impl_, name)); \ & variable) // NOLINT(bugprone-macro-parentheses)
void PYBIND11_CONCAT(pybind11_init_, name)(::pybind11::module_ &variable)
PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE) PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
PYBIND11_NAMESPACE_BEGIN(detail) PYBIND11_NAMESPACE_BEGIN(detail)

View File

@ -1551,8 +1551,11 @@ private:
"pybind11::vectorize(...) requires a function with at least one vectorizable argument"); "pybind11::vectorize(...) requires a function with at least one vectorizable argument");
public: public:
template <typename T> template <typename T,
explicit vectorize_helper(T &&f) : f(std::forward<T>(f)) { } // SFINAE to prevent shadowing the copy constructor.
typename = detail::enable_if_t<
!std::is_same<vectorize_helper, typename std::decay<T>::type>::value>>
explicit vectorize_helper(T &&f) : f(std::forward<T>(f)) {}
object operator()(typename vectorize_arg<Args>::type... args) { object operator()(typename vectorize_arg<Args>::type... args) {
return run(args..., return run(args...,

View File

@ -1726,7 +1726,7 @@ struct enum_base {
m_base.attr(op) = cpp_function( \ m_base.attr(op) = cpp_function( \
[](const object &a, const 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; /* NOLINT(bugprone-macro-parentheses) */ \
return expr; \ return expr; \
}, \ }, \
name(op), \ name(op), \

View File

@ -110,7 +110,11 @@ public:
/// Overwrite this reference with another reference /// Overwrite this reference with another reference
ref& operator=(const ref& r) { ref& operator=(const ref& r) {
print_copy_assigned(this, "pointer", r.m_ptr); track_copy_assigned((ref_tag*) this); if (this == &r) {
return *this;
}
print_copy_assigned(this, "pointer", r.m_ptr);
track_copy_assigned((ref_tag *) this);
if (m_ptr == r.m_ptr) if (m_ptr == r.m_ptr)
return *this; return *this;

View File

@ -23,11 +23,10 @@ public:
test_initializer(const char *submodule_name, Initializer init); test_initializer(const char *submodule_name, Initializer init);
}; };
#define TEST_SUBMODULE(name, variable) \ #define TEST_SUBMODULE(name, variable) \
void test_submodule_##name(py::module_ &); \ void test_submodule_##name(py::module_ &); \
test_initializer name(#name, test_submodule_##name); \ test_initializer name(#name, test_submodule_##name); \
void test_submodule_##name(py::module_ &variable) void test_submodule_##name(py::module_ &(variable))
/// Dummy type which is not exported anywhere -- something to trigger a conversion error /// Dummy type which is not exported anywhere -- something to trigger a conversion error
struct UnregisteredType { }; struct UnregisteredType { };

View File

@ -40,7 +40,11 @@ TEST_SUBMODULE(buffers, m) {
} }
Matrix &operator=(const Matrix &s) { Matrix &operator=(const Matrix &s) {
print_copy_assigned(this, std::to_string(m_rows) + "x" + std::to_string(m_cols) + " matrix"); if (this == &s) {
return *this;
}
print_copy_assigned(this,
std::to_string(m_rows) + "x" + std::to_string(m_cols) + " matrix");
delete[] m_data; delete[] m_data;
m_rows = s.m_rows; m_rows = s.m_rows;
m_cols = s.m_cols; m_cols = s.m_cols;

View File

@ -492,15 +492,15 @@ using DoesntBreak5 = py::class_<BreaksBase<5>>;
using DoesntBreak6 = py::class_<BreaksBase<6>, std::shared_ptr<BreaksBase<6>>, BreaksTramp<6>>; using DoesntBreak6 = py::class_<BreaksBase<6>, std::shared_ptr<BreaksBase<6>>, BreaksTramp<6>>;
using DoesntBreak7 = py::class_<BreaksBase<7>, BreaksTramp<7>, std::shared_ptr<BreaksBase<7>>>; using DoesntBreak7 = py::class_<BreaksBase<7>, BreaksTramp<7>, std::shared_ptr<BreaksBase<7>>>;
using DoesntBreak8 = py::class_<BreaksBase<8>, std::shared_ptr<BreaksBase<8>>>; using DoesntBreak8 = py::class_<BreaksBase<8>, std::shared_ptr<BreaksBase<8>>>;
#define CHECK_BASE(N) static_assert(std::is_same<typename DoesntBreak##N::type, BreaksBase<N>>::value, \ #define CHECK_BASE(N) static_assert(std::is_same<typename DoesntBreak##N::type, BreaksBase<(N)>>::value, \
"DoesntBreak" #N " has wrong type!") "DoesntBreak" #N " has wrong type!")
CHECK_BASE(1); CHECK_BASE(2); CHECK_BASE(3); CHECK_BASE(4); CHECK_BASE(5); CHECK_BASE(6); CHECK_BASE(7); CHECK_BASE(8); CHECK_BASE(1); CHECK_BASE(2); CHECK_BASE(3); CHECK_BASE(4); CHECK_BASE(5); CHECK_BASE(6); CHECK_BASE(7); CHECK_BASE(8);
#define CHECK_ALIAS(N) static_assert(DoesntBreak##N::has_alias && std::is_same<typename DoesntBreak##N::type_alias, BreaksTramp<N>>::value, \ #define CHECK_ALIAS(N) static_assert(DoesntBreak##N::has_alias && std::is_same<typename DoesntBreak##N::type_alias, BreaksTramp<(N)>>::value, \
"DoesntBreak" #N " has wrong type_alias!") "DoesntBreak" #N " has wrong type_alias!")
#define CHECK_NOALIAS(N) static_assert(!DoesntBreak##N::has_alias && std::is_void<typename DoesntBreak##N::type_alias>::value, \ #define CHECK_NOALIAS(N) static_assert(!DoesntBreak##N::has_alias && std::is_void<typename DoesntBreak##N::type_alias>::value, \
"DoesntBreak" #N " has type alias, but shouldn't!") "DoesntBreak" #N " has type alias, but shouldn't!")
CHECK_ALIAS(1); CHECK_ALIAS(2); CHECK_NOALIAS(3); CHECK_ALIAS(4); CHECK_NOALIAS(5); CHECK_ALIAS(6); CHECK_ALIAS(7); CHECK_NOALIAS(8); CHECK_ALIAS(1); CHECK_ALIAS(2); CHECK_NOALIAS(3); CHECK_ALIAS(4); CHECK_NOALIAS(5); CHECK_ALIAS(6); CHECK_ALIAS(7); CHECK_NOALIAS(8);
#define CHECK_HOLDER(N, TYPE) static_assert(std::is_same<typename DoesntBreak##N::holder_type, std::TYPE##_ptr<BreaksBase<N>>>::value, \ #define CHECK_HOLDER(N, TYPE) static_assert(std::is_same<typename DoesntBreak##N::holder_type, std::TYPE##_ptr<BreaksBase<(N)>>>::value, \
"DoesntBreak" #N " has wrong holder_type!") "DoesntBreak" #N " has wrong holder_type!")
CHECK_HOLDER(1, unique); CHECK_HOLDER(2, unique); CHECK_HOLDER(3, unique); CHECK_HOLDER(4, unique); CHECK_HOLDER(5, unique); CHECK_HOLDER(1, unique); CHECK_HOLDER(2, unique); CHECK_HOLDER(3, unique); CHECK_HOLDER(4, unique); CHECK_HOLDER(5, unique);
CHECK_HOLDER(6, shared); CHECK_HOLDER(7, shared); CHECK_HOLDER(8, shared); 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);
// failures occurs). // failures occurs).
// We have to actually look into the type: the typedef alone isn't enough to instantiate the type: // We have to actually look into the type: the typedef alone isn't enough to instantiate the type:
#define CHECK_BROKEN(N) static_assert(std::is_same<typename Breaks##N::type, BreaksBase<-N>>::value, \ #define CHECK_BROKEN(N) static_assert(std::is_same<typename Breaks##N::type, BreaksBase<-(N)>>::value, \
"Breaks1 has wrong type!"); "Breaks1 has wrong type!");
//// Two holder classes: //// Two holder classes:

View File

@ -108,7 +108,10 @@ TEST_SUBMODULE(multiple_inheritance, m) {
// test_multiple_inheritance_python_many_bases // test_multiple_inheritance_python_many_bases
#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; }) #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); \
})
PYBIND11_BASEN( 1); PYBIND11_BASEN( 2); PYBIND11_BASEN( 3); PYBIND11_BASEN( 4); PYBIND11_BASEN( 1); PYBIND11_BASEN( 2); PYBIND11_BASEN( 3); PYBIND11_BASEN( 4);
PYBIND11_BASEN( 5); PYBIND11_BASEN( 6); PYBIND11_BASEN( 7); PYBIND11_BASEN( 8); PYBIND11_BASEN( 5); PYBIND11_BASEN( 6); PYBIND11_BASEN( 7); PYBIND11_BASEN( 8);
PYBIND11_BASEN( 9); PYBIND11_BASEN(10); PYBIND11_BASEN(11); PYBIND11_BASEN(12); PYBIND11_BASEN( 9); PYBIND11_BASEN(10); PYBIND11_BASEN(11); PYBIND11_BASEN(12);

View File

@ -148,11 +148,13 @@ py::array mkarray_via_buffer(size_t n) {
1, { n }, { sizeof(T) })); 1, { n }, { sizeof(T) }));
} }
#define SET_TEST_VALS(s, i) do { \ #define SET_TEST_VALS(s, i) \
s.bool_ = (i) % 2 != 0; \ do { \
s.uint_ = (uint32_t) (i); \ (s).bool_ = (i) % 2 != 0; \
s.float_ = (float) (i) * 1.5f; \ (s).uint_ = (uint32_t) (i); \
s.ldbl_ = (long double) (i) * -2.5L; } while (0) (s).float_ = (float) (i) *1.5f; \
(s).ldbl_ = (long double) (i) * -2.5L; \
} while (0)
template <typename S> template <typename S>
py::array_t<S, 0> create_recarray(size_t n) { py::array_t<S, 0> create_recarray(size_t n) {

View File

@ -182,6 +182,7 @@ struct SharedPtrRef {
struct SharedFromThisRef { struct SharedFromThisRef {
struct B : std::enable_shared_from_this<B> { struct B : std::enable_shared_from_this<B> {
B() { print_created(this); } B() { print_created(this); }
// NOLINTNEXTLINE(bugprone-copy-constructor-init)
B(const B &) : std::enable_shared_from_this<B>() { print_copy_created(this); } B(const B &) : std::enable_shared_from_this<B>() { print_copy_created(this); }
B(B &&) noexcept : std::enable_shared_from_this<B>() { print_move_created(this); } B(B &&) noexcept : std::enable_shared_from_this<B>() { print_move_created(this); }
~B() { print_destroyed(this); } ~B() { print_destroyed(this); }

View File

@ -86,13 +86,13 @@ std::vector<std::unique_ptr<Animal>> create_zoo()
const std::type_info* Animal::type_of_kind(Kind kind) const std::type_info* Animal::type_of_kind(Kind kind)
{ {
switch (kind) { switch (kind) {
case Kind::Unknown: break; case Kind::Unknown:
case Kind::Dog: break; case Kind::Dog: break;
case Kind::Labrador: return &typeid(Labrador); case Kind::Labrador: return &typeid(Labrador);
case Kind::Chihuahua: return &typeid(Chihuahua); case Kind::Chihuahua: return &typeid(Chihuahua);
case Kind::LastDog: break;
case Kind::LastDog:
case Kind::Cat: break; case Kind::Cat: break;
case Kind::Panther: return &typeid(Panther); case Kind::Panther: return &typeid(Panther);
case Kind::LastCat: break; case Kind::LastCat: break;

View File

@ -454,6 +454,7 @@ template <class Base = B_Tpl>
class PyB_Tpl : public PyA_Tpl<Base> { class PyB_Tpl : public PyA_Tpl<Base> {
public: public:
using PyA_Tpl<Base>::PyA_Tpl; // Inherit constructors (via PyA_Tpl's inherited constructors) using PyA_Tpl<Base>::PyA_Tpl; // Inherit constructors (via PyA_Tpl's inherited constructors)
// NOLINTNEXTLINE(bugprone-parent-virtual-call)
int unlucky_number() override { PYBIND11_OVERRIDE(int, Base, unlucky_number, ); } int unlucky_number() override { PYBIND11_OVERRIDE(int, Base, unlucky_number, ); }
double lucky_number() override { PYBIND11_OVERRIDE(double, Base, lucky_number, ); } double lucky_number() override { PYBIND11_OVERRIDE(double, Base, lucky_number, ); }
}; };