From 3b30b0a51e48a3273918f30dfb222db604cea5e9 Mon Sep 17 00:00:00 2001 From: Aaron Gokaslan Date: Mon, 21 Jun 2021 10:37:48 -0400 Subject: [PATCH] fix(clang-tidy): clang-tidy readability and misc fixes, like adding const (#3052) * Enable and apply clang-tidy readability and misc fixes. * Revert deprecated tester * add space to tests/test_constants_and_functions.cpp --- .clang-tidy | 13 +++++++++++++ include/pybind11/detail/common.h | 2 +- include/pybind11/detail/descr.h | 4 ++-- include/pybind11/detail/type_caster_base.h | 6 +++--- include/pybind11/numpy.h | 4 ++-- include/pybind11/pybind11.h | 3 ++- tests/local_bindings.h | 2 +- tests/test_constants_and_functions.cpp | 10 +++++++--- tests/test_factory_constructors.cpp | 4 ++-- tests/test_iostream.cpp | 6 +++--- tests/test_methods_and_attributes.cpp | 12 +++++------- tests/test_modules.cpp | 3 ++- tests/test_multiple_inheritance.cpp | 8 ++++---- tests/test_numpy_vectorize.cpp | 2 +- tests/test_smart_ptr.cpp | 2 +- tests/test_stl_binders.cpp | 4 +++- 16 files changed, 52 insertions(+), 33 deletions(-) diff --git a/.clang-tidy b/.clang-tidy index 9098e77ff..60531abe0 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -2,17 +2,30 @@ FormatStyle: file Checks: ' llvm-namespace-comment, +misc-misplaced-const, +misc-static-assert, +misc-uniqueptr-reset-release, modernize-avoid-bind, modernize-replace-auto-ptr, modernize-replace-disallow-copy-and-assign-macro, modernize-shrink-to-fit, modernize-use-auto, +modernize-use-bool-literals, modernize-use-equals-default, modernize-use-equals-delete, +modernize-use-default-member-init, +modernize-use-noexcept, modernize-use-emplace, modernize-use-override, modernize-use-using, readability-container-size-empty, +readability-make-member-function-const, +readability-redundant-function-ptr-dereference, +readability-redundant-smartptr-get, +readability-redundant-string-cstr, +readability-simplify-subscript-expr, +readability-string-compare, +readability-uniqueptr-delete-release, ' CheckOptions: diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 987f0badf..1f7446103 100644 --- a/include/pybind11/detail/common.h +++ b/include/pybind11/detail/common.h @@ -480,7 +480,7 @@ struct instance { void allocate_layout(); /// Destroys/deallocates all of the above - void deallocate_layout(); + void deallocate_layout() const; /// Returns the value_and_holder wrapper for the given type (or the first, if `find_type` /// omitted). Returns a default-constructed (with `.inst = nullptr`) object on failure if diff --git a/include/pybind11/detail/descr.h b/include/pybind11/detail/descr.h index 92720cd56..7cb8350e7 100644 --- a/include/pybind11/detail/descr.h +++ b/include/pybind11/detail/descr.h @@ -23,9 +23,9 @@ PYBIND11_NAMESPACE_BEGIN(detail) /* Concatenate type signatures at compile time */ template struct descr { - char text[N + 1]; + char text[N + 1]{'\0'}; - constexpr descr() : text{'\0'} { } + constexpr descr() = default; constexpr descr(char const (&s)[N+1]) : descr(s, make_index_sequence()) { } template diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index 8ebe39879..0dd9d48e5 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -241,7 +241,7 @@ struct value_and_holder { ? inst->simple_holder_constructed : inst->nonsimple.status[index] & instance::status_holder_constructed; } - void set_holder_constructed(bool v = true) { + void set_holder_constructed(bool v = true) const { if (inst->simple_layout) inst->simple_holder_constructed = v; else if (v) @@ -254,7 +254,7 @@ struct value_and_holder { ? inst->simple_instance_registered : inst->nonsimple.status[index] & instance::status_instance_registered; } - void set_instance_registered(bool v = true) { + void set_instance_registered(bool v = true) const { if (inst->simple_layout) inst->simple_instance_registered = v; else if (v) @@ -397,7 +397,7 @@ PYBIND11_NOINLINE inline void instance::allocate_layout() { owned = true; } -PYBIND11_NOINLINE inline void instance::deallocate_layout() { +PYBIND11_NOINLINE inline void instance::deallocate_layout() const { if (!simple_layout) PyMem_Free(nonsimple.values_and_holders); } diff --git a/include/pybind11/numpy.h b/include/pybind11/numpy.h index 928784246..fdb7a9794 100644 --- a/include/pybind11/numpy.h +++ b/include/pybind11/numpy.h @@ -1291,7 +1291,7 @@ public: using value_type = container_type::value_type; using size_type = container_type::size_type; - common_iterator() : p_ptr(0), m_strides() {} + common_iterator() : m_strides() {} common_iterator(void* ptr, const container_type& strides, const container_type& shape) : p_ptr(reinterpret_cast(ptr)), m_strides(strides.size()) { @@ -1312,7 +1312,7 @@ public: } private: - char* p_ptr; + char *p_ptr{0}; container_type m_strides; }; diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 2ff88dbf5..497923605 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -395,7 +395,8 @@ protected: rec->def = new PyMethodDef(); std::memset(rec->def, 0, sizeof(PyMethodDef)); rec->def->ml_name = rec->name; - rec->def->ml_meth = reinterpret_cast(reinterpret_cast(*dispatcher)); + rec->def->ml_meth + = reinterpret_cast(reinterpret_cast(dispatcher)); rec->def->ml_flags = METH_VARARGS | METH_KEYWORDS; capsule rec_capsule(unique_rec.release(), [](void *ptr) { diff --git a/tests/local_bindings.h b/tests/local_bindings.h index 22537b13a..91c23fe59 100644 --- a/tests/local_bindings.h +++ b/tests/local_bindings.h @@ -56,7 +56,7 @@ class Pet { public: Pet(std::string name) : name_(name) {} std::string name_; - const std::string &name() { return name_; } + const std::string &name() const { return name_; } }; } // namespace pets diff --git a/tests/test_constants_and_functions.cpp b/tests/test_constants_and_functions.cpp index 5bbbd0c70..04830d8d2 100644 --- a/tests/test_constants_and_functions.cpp +++ b/tests/test_constants_and_functions.cpp @@ -1,5 +1,6 @@ /* - tests/test_constants_and_functions.cpp -- global constants and functions, enumerations, raw byte strings + tests/test_constants_and_functions.cpp -- global constants and functions, enumerations, raw + byte strings Copyright (c) 2016 Wenzel Jakob @@ -60,6 +61,7 @@ int f3(int x) noexcept(false) { return x+3; } # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wdeprecated" #endif +// NOLINTNEXTLINE(modernize-use-noexcept) int f4(int x) throw() { return x+4; } // Deprecated equivalent to noexcept(true) #if defined(__GNUG__) && !defined(__INTEL_COMPILER) # pragma GCC diagnostic pop @@ -75,8 +77,10 @@ struct C { # pragma GCC diagnostic push # pragma GCC diagnostic ignored "-Wdeprecated" #endif - int m7(int x) throw() { return x-7; } - int m8(int x) const throw() { return x-8; } + // NOLINTNEXTLINE(modernize-use-noexcept) + int m7(int x) throw() { return x - 7; } + // NOLINTNEXTLINE(modernize-use-noexcept) + int m8(int x) const throw() { return x - 8; } #if defined(__GNUG__) && !defined(__INTEL_COMPILER) # pragma GCC diagnostic pop #endif diff --git a/tests/test_factory_constructors.cpp b/tests/test_factory_constructors.cpp index 6832ac494..7308ad5fc 100644 --- a/tests/test_factory_constructors.cpp +++ b/tests/test_factory_constructors.cpp @@ -77,7 +77,7 @@ public: TestFactory6(const TestFactory6 &f) { print_copy_created(this); value = f.value; alias = f.alias; } virtual ~TestFactory6() { print_destroyed(this); } virtual int get() { return value; } - bool has_alias() { return alias; } + bool has_alias() const { return alias; } }; class PyTF6 : public TestFactory6 { public: @@ -102,7 +102,7 @@ public: TestFactory7(const TestFactory7 &f) { print_copy_created(this); value = f.value; alias = f.alias; } virtual ~TestFactory7() { print_destroyed(this); } virtual int get() { return value; } - bool has_alias() { return alias; } + bool has_alias() const { return alias; } }; class PyTF7 : public TestFactory7 { public: diff --git a/tests/test_iostream.cpp b/tests/test_iostream.cpp index 1be0655df..f21fed546 100644 --- a/tests/test_iostream.cpp +++ b/tests/test_iostream.cpp @@ -34,7 +34,7 @@ void noisy_funct_dual(std::string msg, std::string emsg) { // simply repeatedly write to std::cerr until stopped // redirect is called at some point to test the safety of scoped_estream_redirect struct TestThread { - TestThread() : t_{nullptr}, stop_{false} { + TestThread() : stop_{false} { auto thread_f = [this] { while (!stop_) { std::cout << "x" << std::flush; @@ -49,7 +49,7 @@ struct TestThread { void stop() { stop_ = true; } - void join() { + void join() const { py::gil_scoped_release gil_lock; t_->join(); } @@ -59,7 +59,7 @@ struct TestThread { std::this_thread::sleep_for(std::chrono::milliseconds(50)); } - std::thread * t_; + std::thread *t_{nullptr}; std::atomic stop_; }; diff --git a/tests/test_methods_and_attributes.cpp b/tests/test_methods_and_attributes.cpp index f99909bda..1f855ef35 100644 --- a/tests/test_methods_and_attributes.cpp +++ b/tests/test_methods_and_attributes.cpp @@ -25,9 +25,7 @@ public: ExampleMandA(ExampleMandA &&e) : value(e.value) { print_move_created(this); } ~ExampleMandA() { print_destroyed(this); } - std::string toString() { - return "ExampleMandA[value=" + std::to_string(value) + "]"; - } + std::string toString() const { return "ExampleMandA[value=" + std::to_string(value) + "]"; } void operator=(const ExampleMandA &e) { print_copy_assigned(this); value = e.value; } void operator=(ExampleMandA &&e) { print_move_assigned(this); value = e.value; } @@ -48,13 +46,13 @@ public: ExampleMandA self1() { return *this; } // return by value ExampleMandA &self2() { return *this; } // return by reference - const ExampleMandA &self3() { return *this; } // return by const reference + const ExampleMandA &self3() const { return *this; } // return by const reference ExampleMandA *self4() { return this; } // return by pointer - const ExampleMandA *self5() { return this; } // return by const pointer + const ExampleMandA *self5() const { return this; } // return by const pointer - int internal1() { return value; } // return by value + int internal1() const { return value; } // return by value int &internal2() { return value; } // return by reference - const int &internal3() { return value; } // return by const reference + const int &internal3() const { return value; } // return by const reference int *internal4() { return &value; } // return by pointer const int *internal5() { return &value; } // return by const pointer diff --git a/tests/test_modules.cpp b/tests/test_modules.cpp index 67387e809..586713540 100644 --- a/tests/test_modules.cpp +++ b/tests/test_modules.cpp @@ -24,7 +24,8 @@ TEST_SUBMODULE(modules, m) { ~A() { print_destroyed(this); } A(const A&) { print_copy_created(this); } A& operator=(const A ©) { print_copy_assigned(this); v = copy.v; return *this; } - std::string toString() { return "A[" + std::to_string(v) + "]"; } + std::string toString() const { return "A[" + std::to_string(v) + "]"; } + private: int v; }; diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index ad801fe19..daecb9564 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -48,12 +48,12 @@ int VanillaStaticMix2::static_value = 12; // test_multiple_inheritance_virtbase struct Base1a { Base1a(int i) : i(i) { } - int foo() { return i; } + int foo() const { return i; } int i; }; struct Base2a { Base2a(int i) : i(i) { } - int bar() { return i; } + int bar() const { return i; } int i; }; struct Base12a : Base1a, Base2a { @@ -78,7 +78,7 @@ TEST_SUBMODULE(multiple_inheritance, m) { // test_multiple_inheritance_mix2 struct Base1 { Base1(int i) : i(i) { } - int foo() { return i; } + int foo() const { return i; } int i; }; py::class_ b1(m, "Base1"); @@ -87,7 +87,7 @@ TEST_SUBMODULE(multiple_inheritance, m) { struct Base2 { Base2(int i) : i(i) { } - int bar() { return i; } + int bar() const { return i; } int i; }; py::class_ b2(m, "Base2"); diff --git a/tests/test_numpy_vectorize.cpp b/tests/test_numpy_vectorize.cpp index 274b7558a..8ed9b27e1 100644 --- a/tests/test_numpy_vectorize.cpp +++ b/tests/test_numpy_vectorize.cpp @@ -62,7 +62,7 @@ TEST_SUBMODULE(numpy_vectorize, m) { // test_method_vectorization struct VectorizeTestClass { VectorizeTestClass(int v) : value{v} {}; - float method(int x, float y) { return y + (float) (x + value); } + float method(int x, float y) const { return y + (float) (x + value); } int value = 0; }; py::class_ vtc(m, "VectorizeTestClass"); diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index e0af24979..2ece3bf0c 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -240,7 +240,7 @@ struct ElementBase { struct ElementA : ElementBase { ElementA(int v) : v(v) { } - int value() { return v; } + int value() const { return v; } int v; }; diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp index c791477e3..22847eb7a 100644 --- a/tests/test_stl_binders.cpp +++ b/tests/test_stl_binders.cpp @@ -125,5 +125,7 @@ TEST_SUBMODULE(stl_binders, m) { PYBIND11_NUMPY_DTYPE(VStruct, w, x, y, z); py::class_(m, "VStruct").def_readwrite("x", &VStruct::x); py::bind_vector>(m, "VectorStruct", py::buffer_protocol()); - m.def("get_vectorstruct", [] {return std::vector {{0, 5, 3.0, 1}, {1, 30, -1e4, 0}};}); + m.def("get_vectorstruct", [] { + return std::vector{{false, 5, 3.0, true}, {true, 30, -1e4, false}}; + }); }