style: clang-tidy checks (#455)

* style: clang-tidy checks

* style: a few more checks

* style: casting

Author: henryiii

.clang-tidy

@@ -0,0 +1,12 @@
+FormatStyle: file
+
+Checks: '
+modernize-*,
+-modernize-use-trailing-return-type,
+-modernize-avoid-c-arrays,
+llvm-namespace-comment,
+readability-container-size-empty,
+google-readability-casting,
+'
+
+HeaderFilterRegex: 'bh_python/.*hpp'

.github/workflows/tests.yml

@@ -21,6 +21,29 @@ jobs:
       with:
         extra_args: --hook-stage manual
 
+  clang-tidy:
+    name: Clang-Tidy
+    runs-on: ubuntu-latest
+    container: silkeh/clang:10
+
+    steps:
+    - uses: actions/checkout@v1
+      with:
+        submodules: true
+
+    - name: Install requirements
+      run: apt-get update && apt-get install -y python3-dev python3-pip
+
+    - name: Install extra requirements
+      run: python3 -m pip install setuptools_scm
+
+    - name: Configure
+      run: cmake -S . -B build -DCMAKE_CXX_CLANG_TIDY="$(which clang-tidy);--warnings-as-errors=*"
+
+    - name: Build
+      run: cmake --build build -j 2
+
+
   cmake:
     runs-on: ubuntu-latest
     strategy:

CMakeLists.txt

@@ -105,8 +105,15 @@ option(BOOST_HISTOGRAM_ERRORS "Make warnings errors (for CI mostly)")
 
 # Adding warnings
 if("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang" OR "${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
-  target_compile_options(_core PRIVATE -Wall -Wextra -pedantic-errors -Wconversion
-                                       -Wsign-conversion -Wsign-compare)
+  target_compile_options(
+    _core
+    PRIVATE -Wall
+            -Wextra
+            -pedantic-errors
+            -Wconversion
+            -Wsign-conversion
+            -Wsign-compare
+            -Wno-unused-value)
   if(BOOST_HISTOGRAM_ERRORS)
     target_compile_options(_core PRIVATE -Werror)
   endif()
@@ -182,7 +189,7 @@ function(python_import)
   if(FAILED_NAMES)
     list(JOIN FAILED_NAMES " " FAILED_NAMES)
     message(
-      FATAL_ERROR
+      WARNING
         "Dependencies missing, try installing:\n  ${PYTHON_EXECUTABLE} -m pip install ${FAILED_NAMES}\n"
     )
   endif()

CONTRIBUTING.md

@@ -176,6 +176,30 @@ pre-commit autoupdate
 > when running pre-commit, and instead run `clang-format -style=file -i
 > <files>` yourself.
 
+## Clang-Tidy
+
+To run Clang tidy, the following recipe should work. Files will be modified in
+place, so you can use git to monitor the changes.
+
+```bash
+docker run --rm -v $PWD:/pybind11 -it silkeh/clang:10
+apt-get update && apt-get install python3-dev
+cmake -S pybind11/ -B build -DCMAKE_CXX_CLANG_TIDY="$(which clang-tidy);-fix"
+cmake --build build
+```
+
+Remember to build single-threaded if applying fixes!
+
+## Include what you use
+
+To run include what you use, install (`brew install include-what-you-use` on
+macOS), then run:
+
+```bash
+cmake -S . -B build-iwyu -DCMAKE_CXX_INCLUDE_WHAT_YOU_USE=$(which include-what-you-use)
+cmake --build build
+```
+
 ## Common tasks
 
 

include/bh_python/array_like.hpp

@@ -14,11 +14,11 @@
 template <class T>
 py::array_t<T> array_like(py::object obj) {
     if(!py::isinstance<py::array>(obj)) {
-        ssize_t shape[1] = {0}; // if scalar
+        py::ssize_t shape[1] = {0}; // if scalar
         if(py::isinstance<py::sequence>(obj) && !py::isinstance<py::str>(obj)) {
             // if sequence
             auto seq = py::cast<py::sequence>(obj);
-            shape[0] = static_cast<ssize_t>(seq.size());
+            shape[0] = static_cast<py::ssize_t>(seq.size());
         }
         return py::array_t<T>(shape);
     }

include/bh_python/axis.hpp

@@ -117,8 +117,7 @@ class boolean : public bh::axis::integer<int, metadata_t, option::none_t> {
                      bh::axis::index_type end,
                      unsigned merge)
         : integer(src, begin, end, merge) {}
-    boolean(const boolean& other)
-        : integer(other) {}
+    boolean(const boolean& other) = default;
 
     bh::axis::index_type index(int x) const noexcept {
         return integer::index(x == 0 ? 0 : 1);
@@ -206,11 +205,9 @@ decltype(auto) unchecked_bin(const A& ax, bh::axis::index_type i) {
 template <class A>
 py::array_t<double> edges(const A& ax, bool flow = false, bool numpy_upper = false) {
     auto continuous = [flow, numpy_upper](const auto& ax) {
-        using AX = std::decay_t<decltype(ax)>;
-        using index_type
-            = std::conditional_t<bh::axis::traits::is_continuous<AX>::value,
-                                 bh::axis::real_index_type,
-                                 bh::axis::index_type>;
+        using AX         = std::decay_t<decltype(ax)>;
+        using index_type = bh::axis::index_type;
+
         const index_type underflow
             = flow && (bh::axis::traits::get_options<AX>::test(option::underflow));
         const index_type overflow

include/bh_python/histogram.hpp

@@ -101,7 +101,7 @@ py::buffer_info make_buffer(bh::histogram<A, bh::unlimited_storage<Allocator>>&
 template <class F, int Opt>
 const F& pyarray_at(const py::array_t<F, Opt>& input,
                     std::vector<py::ssize_t> indexes) {
-    const py::ssize_t rank = static_cast<py::ssize_t>(indexes.size());
+    const auto rank = static_cast<py::ssize_t>(indexes.size());
     if(input.ndim() != rank)
         throw py::value_error(
             "The input array dimensions must match the index dimensions");

include/bh_python/kwargs.hpp

@@ -47,7 +47,7 @@ inline void none_only_arg(py::kwargs& kwargs, const char* name) {
 
 /// Run this last; it will provide an error if other keyword are still present
 inline void finalize_args(const py::kwargs& kwargs) {
-    if(kwargs.size() > 0) {
+    if(!kwargs.empty()) {
         auto keys = py::str(", ").attr("join")(kwargs.attr("keys")());
         throw py::type_error(py::str("Keyword(s) {0} not expected").format(keys));
     }

include/bh_python/register_histogram.hpp

@@ -49,7 +49,7 @@ auto register_histogram(py::module& m, const char* name, const char* desc) {
         .def("__copy__", [](const histogram_t& self) { return histogram_t(self); })
         .def("__deepcopy__",
              [](const histogram_t& self, py::object memo) {
-                 histogram_t* a  = new histogram_t(self);
+                 auto* a         = new histogram_t(self);
                  py::module copy = py::module::import("copy");
                  for(unsigned i = 0; i < a->rank(); i++) {
                      bh::unsafe_access::axis(*a, i).metadata()
@@ -135,7 +135,8 @@ auto register_histogram(py::module& m, const char* name, const char* desc) {
         .def(
             "axis",
             [](const histogram_t& self, int i) -> py::object {
-                unsigned ii = i < 0 ? self.rank() - (unsigned)std::abs(i) : (unsigned)i;
+                unsigned ii = i < 0 ? self.rank() - static_cast<unsigned>(std::abs(i))
+                                    : static_cast<unsigned>(i);
 
                 if(ii < self.rank()) {
                     const axis_variant& var = self.axis(ii);

include/bh_python/regular_numpy.hpp

@@ -17,16 +17,15 @@ namespace axis {
 /// Mimics the numpy behavoir exactly.
 class regular_numpy : public bh::axis::regular<double, bh::use_default, metadata_t> {
     using value_type = double;
-    double stop_;
+    double stop_{0};
 
   public:
     regular_numpy(unsigned n, value_type start, value_type stop, metadata_t meta = {})
         : regular(n, start, stop, meta)
         , stop_(stop) {}
 
     regular_numpy()
-        : regular()
-        , stop_(0) {}
+        : regular() {}
 
     bh::axis::index_type index(value_type v) const {
         return v <= stop_ ? std::min(regular::index(v), size() - 1) : regular::index(v);