Skip to content

Enable and apply clang-tidy readability and misc fixes. #3052

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

Merged
merged 3 commits into from
Jun 21, 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
13 changes: 13 additions & 0 deletions .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion include/pybind11/detail/common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/detail/descr.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ PYBIND11_NAMESPACE_BEGIN(detail)
/* Concatenate type signatures at compile time */
template <size_t N, typename... Ts>
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<N>()) { }

template <size_t... Is>
Expand Down
6 changes: 3 additions & 3 deletions include/pybind11/detail/type_caster_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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);
}
Expand Down
4 changes: 2 additions & 2 deletions include/pybind11/numpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -1291,7 +1291,7 @@ class common_iterator {
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<char*>(ptr)), m_strides(strides.size()) {
Expand All @@ -1312,7 +1312,7 @@ class common_iterator {
}

private:
char* p_ptr;
char *p_ptr{0};
container_type m_strides;
};

Expand Down
3 changes: 2 additions & 1 deletion include/pybind11/pybind11.h
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,8 @@ class cpp_function : public function {
rec->def = new PyMethodDef();
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));
rec->def->ml_meth
= reinterpret_cast<PyCFunction>(reinterpret_cast<void (*)(void)>(dispatcher));
rec->def->ml_flags = METH_VARARGS | METH_KEYWORDS;

capsule rec_capsule(unique_rec.release(), [](void *ptr) {
Expand Down
2 changes: 1 addition & 1 deletion tests/local_bindings.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 7 additions & 3 deletions tests/test_constants_and_functions.cpp
Original file line number Diff line number Diff line change
@@ -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 <[email protected]>

Expand Down Expand Up @@ -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
Expand All @@ -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)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check discourages the user of throw, but since these are explicilty deprecated tests, I add nolints to prevent them from being changed.

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
Expand Down
4 changes: 2 additions & 2 deletions tests/test_factory_constructors.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class TestFactory6 {
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:
Expand All @@ -102,7 +102,7 @@ class TestFactory7 {
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:
Expand Down
6 changes: 3 additions & 3 deletions tests/test_iostream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -49,7 +49,7 @@ struct TestThread {

void stop() { stop_ = true; }

void join() {
void join() const {
py::gil_scoped_release gil_lock;
t_->join();
}
Expand All @@ -59,7 +59,7 @@ struct TestThread {
std::this_thread::sleep_for(std::chrono::milliseconds(50));
}

std::thread * t_;
std::thread *t_{nullptr};
std::atomic<bool> stop_;
};

Expand Down
12 changes: 5 additions & 7 deletions tests/test_methods_and_attributes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ class ExampleMandA {
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; }
Expand All @@ -48,13 +46,13 @@ class ExampleMandA {

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

Expand Down
3 changes: 2 additions & 1 deletion tests/test_modules.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ TEST_SUBMODULE(modules, m) {
~A() { print_destroyed(this); }
A(const A&) { print_copy_created(this); }
A& operator=(const A &copy) { 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;
};
Expand Down
8 changes: 4 additions & 4 deletions tests/test_multiple_inheritance.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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_<Base1> b1(m, "Base1");
Expand All @@ -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_<Base2> b2(m, "Base2");
Expand Down
2 changes: 1 addition & 1 deletion tests/test_numpy_vectorize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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_<VectorizeTestClass> vtc(m, "VectorizeTestClass");
Expand Down
2 changes: 1 addition & 1 deletion tests/test_smart_ptr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
4 changes: 3 additions & 1 deletion tests/test_stl_binders.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,5 +125,7 @@ TEST_SUBMODULE(stl_binders, m) {
PYBIND11_NUMPY_DTYPE(VStruct, w, x, y, z);
py::class_<VStruct>(m, "VStruct").def_readwrite("x", &VStruct::x);
py::bind_vector<std::vector<VStruct>>(m, "VectorStruct", py::buffer_protocol());
m.def("get_vectorstruct", [] {return std::vector<VStruct> {{0, 5, 3.0, 1}, {1, 30, -1e4, 0}};});
m.def("get_vectorstruct", [] {
return std::vector<VStruct>{{false, 5, 3.0, true}, {true, 30, -1e4, false}};
});
}