#32 fix warnings and clang-tidy issues everywhere

CMakeLists.txt

@@ -25,7 +25,7 @@ option(ELSA_INSTALL "Enable generating the install targets for make install" ${E
 option(ELSA_BUILD_EXAMPLES "Enable building of examples" ${ELSA_MASTER_PROJECT})
 
 option(ELSA_BUILD_CUDA_PROJECTORS "Enable building (or attempting to) the CUDA projectors" ON)
-option(ELSA_BUILD_WITH_MORE_WARNINGS "Enable all and extra warnings when building (-Wall -Wextra)" OFF)
+option(ELSA_BUILD_WITH_MORE_WARNINGS "Enable all and extra warnings when building (-Wall -Wextra)" ON)
 
 option(ELSA_SANITIZE_THREAD "Build elsa with thread sanitizers (TSAN)" OFF)
 option(ELSA_SANITIZE_ADDRESS "Build elsa with address and undefined-behavior sanitizers (ASAN and UBSAN)" OFF)

elsa/core/BlockDescriptor.cpp

 #include "BlockDescriptor.h"
 
+#include <algorithm>
 #include <stdexcept>
 
 namespace elsa
@@ -31,11 +32,16 @@ namespace elsa
             _numberOfCoefficientsPerDimension.head(_numberOfDimensions - 1).prod();
     }
 
-    index_t BlockDescriptor::getNumberOfBlocks() const { return _blockDescriptors.size(); }
+    index_t BlockDescriptor::getNumberOfBlocks() const
+    {
+        return static_cast<index_t>(_blockDescriptors.size());
+    }
 
     const DataDescriptor& BlockDescriptor::getDescriptorOfBlock(index_t i) const
     {
-        return *_blockDescriptors.at(i);
+        // std::vector is using unsigned indices.. so oblige it
+        auto j = static_cast<decltype(_blockDescriptors)::size_type>(i);
+        return *_blockDescriptors.at(j);
     }
 
     index_t BlockDescriptor::getOffsetOfBlock(elsa::index_t i) const
@@ -69,8 +75,9 @@ namespace elsa
         if (_blockDescriptors.size() != otherBlock->_blockDescriptors.size())
             return false;
 
-        for (index_t i = 0; i < _blockDescriptors.size(); ++i)
-            if (*_blockDescriptors.at(i) != *otherBlock->_blockDescriptors.at(i))
+        if (!std::equal(_blockDescriptors.begin(), _blockDescriptors.end(),
+                        otherBlock->_blockDescriptors.begin(),
+                        [](const auto& i1, const auto& i2) { return *i1 == *i2; }))
             return false;
 
         return _blockOffsets == otherBlock->_blockOffsets;

elsa/core/DataContainer.cpp

@@ -107,14 +107,14 @@ namespace elsa
     template <typename data_t>
     data_t& DataContainer<data_t>::operator()(IndexVector_t coordinate)
     {
-        return (*_dataHandler)[_dataDescriptor->getIndexFromCoordinate(coordinate)];
+        return (*_dataHandler)[_dataDescriptor->getIndexFromCoordinate(std::move(coordinate))];
     }
 
     template <typename data_t>
     const data_t& DataContainer<data_t>::operator()(IndexVector_t coordinate) const
     {
         return static_cast<const DataHandler<data_t>&>(
-            *_dataHandler)[_dataDescriptor->getIndexFromCoordinate(coordinate)];
+            *_dataHandler)[_dataDescriptor->getIndexFromCoordinate(std::move(coordinate))];
     }
 
     template <typename data_t>
@@ -276,7 +276,6 @@ namespace elsa
     DataContainer<data_t> DataContainer<data_t>::getBlock(index_t i)
     {
         const auto blockDesc = dynamic_cast<const BlockDescriptor*>(_dataDescriptor.get());
-
         if (!blockDesc)
             throw std::logic_error("DataContainer: cannot get block from not-blocked container");
 
@@ -291,10 +290,9 @@ namespace elsa
     }
 
     template <typename data_t>
-    const DataContainer<data_t> DataContainer<data_t>::getBlock(index_t i) const
+    DataContainer<data_t> DataContainer<data_t>::getBlock(index_t i) const
     {
         const auto blockDesc = dynamic_cast<const BlockDescriptor*>(_dataDescriptor.get());
-
         if (!blockDesc)
             throw std::logic_error("DataContainer: cannot get block from not-blocked container");
 
@@ -320,8 +318,7 @@ namespace elsa
     }
 
     template <typename data_t>
-    const DataContainer<data_t>
-        DataContainer<data_t>::viewAs(const DataDescriptor& dataDescriptor) const
+    DataContainer<data_t> DataContainer<data_t>::viewAs(const DataDescriptor& dataDescriptor) const
     {
         if (dataDescriptor.getNumberOfCoefficients() != getSize())
             throw std::invalid_argument("DataContainer: view must have same size as container");

elsa/core/DataContainer.h

@@ -201,13 +201,13 @@ namespace elsa
         DataContainer<data_t> getBlock(index_t i);
 
         /// returns a const reference to the i-th block, wrapped in a DataContainer
-        const DataContainer<data_t> getBlock(index_t i) const;
+        DataContainer<data_t> getBlock(index_t i) const;
 
         /// return a view of this DataContainer with a different descriptor
         DataContainer<data_t> viewAs(const DataDescriptor& dataDescriptor);
 
         /// return a const view of this DataContainer with a different descriptor
-        const DataContainer<data_t> viewAs(const DataDescriptor& dataDescriptor) const;
+        DataContainer<data_t> viewAs(const DataDescriptor& dataDescriptor) const;
 
         /// iterator for DataContainer (random access and continuous)
         using iterator = DataContainerIterator<DataContainer<data_t>>;

elsa/core/DataContainerIterator.h

@@ -50,7 +50,7 @@ namespace elsa::detail
             return *this;
         }
         /// postfix increment operator
-        self_type operator++(int)
+        self_type operator++(int) &
         {
             auto i = *this;
             ++_ptr;
@@ -64,7 +64,7 @@ namespace elsa::detail
             return *this;
         }
         /// postfix decrement operator
-        self_type operator--(int)
+        self_type operator--(int) &
         {
             auto i = *this;
             --_ptr;
@@ -165,7 +165,7 @@ namespace elsa::detail
             return *this;
         }
         /// postfix increment operator
-        self_type operator++(int)
+        self_type operator++(int) &
         {
             auto i = *this;
             ++_ptr;
@@ -179,7 +179,7 @@ namespace elsa::detail
             return *this;
         }
         /// postfix decrement operator
-        self_type operator--(int)
+        self_type operator--(int) &
         {
             auto i = *this;
             --_ptr;

elsa/core/DataHandlerCPU.cpp

@@ -398,7 +398,6 @@ namespace elsa
                         _data->data() + (map->_map.data() - oldData), map->getSize());
             }
         }
-        return;
     }
 
     template <typename data_t>

elsa/core/DataHandlerCPU.h

@@ -18,7 +18,7 @@ namespace elsa
     class DataHandlerCPU;
     // forward declaration, used for testing and defined in test file (declared as friend)
     template <typename data_t>
-    int useCount(const DataHandlerCPU<data_t>&);
+    long useCount(const DataHandlerCPU<data_t>&);
 
     /**
      * \brief Class representing and owning a vector stored in CPU main memory (using
@@ -163,7 +163,7 @@ namespace elsa
             getBlock(index_t startIndex, index_t numberOfElements) const override;
 
         /// used for testing only and defined in test file
-        friend int useCount<>(const DataHandlerCPU<data_t>& dh);
+        friend long useCount<>(const DataHandlerCPU<data_t>& dh);
 
     protected:
         /// the vector storing the data

elsa/core/DataHandlerMapCPU.cpp

@@ -49,7 +49,6 @@ namespace elsa
     data_t& DataHandlerMapCPU<data_t>::operator[](index_t index)
     {
         _dataOwner->detach();
-
         return _map[index];
     }
 

elsa/core/LinearOperator.cpp

@@ -14,7 +14,8 @@ namespace elsa
 
     template <typename data_t>
     LinearOperator<data_t>::LinearOperator(const LinearOperator<data_t>& other)
-        : _domainDescriptor{other._domainDescriptor->clone()},
+        : Cloneable<LinearOperator<data_t>>(),
+          _domainDescriptor{other._domainDescriptor->clone()},
           _rangeDescriptor{other._rangeDescriptor->clone()},
           _isLeaf{other._isLeaf},
           _isAdjoint{other._isAdjoint},

elsa/core/elsaDefines.h

@@ -10,18 +10,25 @@ namespace elsa
     using complex_t = std::complex<real_t>; ///< global type for complex numbers
     using index_t = std::ptrdiff_t;         ///< global type for indices
 
-    using RealVector_t =
-        Eigen::Matrix<real_t, Eigen::Dynamic, 1>; ///< global type for vectors of real numbers
-    using ComplexVector_t =
-        Eigen::Matrix<complex_t, Eigen::Dynamic, 1>; ///< global type for vectors of complex numbers
-    using IndexVector_t =
-        Eigen::Matrix<index_t, Eigen::Dynamic, 1>; ///< global type for vectors of indices
-    using BooleanVector_t =
-        Eigen::Matrix<bool, Eigen::Dynamic, 1>; ///< global type for vectors of booleans
-
-    using RealMatrix_t =
-        Eigen::Matrix<real_t, Eigen::Dynamic,
-                      Eigen::Dynamic>; ///< global type for matrices of real numbers
-
-    constexpr double pi = 3.14159265358979323846; ///< global constexpr for the number pi
+    /// global type for vectors of real numbers
+    using RealVector_t = Eigen::Matrix<real_t, Eigen::Dynamic, 1>;
+
+    /// global type for vectors of complex numbers
+    using ComplexVector_t = Eigen::Matrix<complex_t, Eigen::Dynamic, 1>;
+
+    /// global type for vectors of indices
+    using IndexVector_t = Eigen::Matrix<index_t, Eigen::Dynamic, 1>;
+
+    /// global type for vectors of booleans
+    using BooleanVector_t = Eigen::Matrix<bool, Eigen::Dynamic, 1>;
+
+    /// global type for matrices of real numbers
+    using RealMatrix_t = Eigen::Matrix<real_t, Eigen::Dynamic, Eigen::Dynamic>;
+
+    /// template global constexpr for the number pi
+    template <typename T>
+    constexpr auto pi = static_cast<T>(3.14159265358979323846);
+
+    /// global constexpr for the number pi
+    constexpr auto pi_t = pi<real_t>;
 } // namespace elsa

elsa/core/tests/CMakeLists.txt

@@ -5,6 +5,7 @@ include(CTest)
 include(Catch)
 
 # the actual tests
+ELSA_TEST(elsaDefines)
 ELSA_TEST(DataDescriptor)
 ELSA_TEST(BlockDescriptor)
 ELSA_TEST(DataContainer)

elsa/core/tests/test_DataContainer.cpp

@@ -151,12 +151,12 @@ SCENARIO("Element-wise access of DataContainers")
                 REQUIRE(dc(coord) == 0.0_a);
                 REQUIRE(dc(17, 4) == 0.0_a);
 
-                dc[index] = 2.2;
+                dc[index] = 2.2f;
                 REQUIRE(dc[index] == 2.2_a);
                 REQUIRE(dc(coord) == 2.2_a);
                 REQUIRE(dc(17, 4) == 2.2_a);
 
-                dc(coord) = 3.3;
+                dc(coord) = 3.3f;
                 REQUIRE(dc[index] == 3.3_a);
                 REQUIRE(dc(coord) == 3.3_a);
                 REQUIRE(dc(17, 4) == 3.3_a);
@@ -236,7 +236,7 @@ SCENARIO("Testing the element-wise operations of DataContainer")
 
             THEN("the binary in-place addition of a scalar work as expected")
             {
-                float scalar = 923.41;
+                float scalar = 923.41f;
                 dc += scalar;
                 for (index_t i = 0; i < dc.getSize(); ++i)
                     REQUIRE(dc[i] == randVec(i) + scalar);
@@ -244,7 +244,7 @@ SCENARIO("Testing the element-wise operations of DataContainer")
 
             THEN("the binary in-place subtraction of a scalar work as expected")
             {
-                float scalar = 74.165;
+                float scalar = 74.165f;
                 dc -= scalar;
                 for (index_t i = 0; i < dc.getSize(); ++i)
                     REQUIRE(dc[i] == randVec(i) - scalar);
@@ -252,7 +252,7 @@ SCENARIO("Testing the element-wise operations of DataContainer")
 
             THEN("the binary in-place multiplication with a scalar work as expected")
             {
-                float scalar = 12.69;
+                float scalar = 12.69f;
                 dc *= scalar;
                 for (index_t i = 0; i < dc.getSize(); ++i)
                     REQUIRE(dc[i] == randVec(i) * scalar);
@@ -260,7 +260,7 @@ SCENARIO("Testing the element-wise operations of DataContainer")
 
             THEN("the binary in-place division by a scalar work as expected")
             {
-                float scalar = 82.61;
+                float scalar = 82.61f;
                 dc /= scalar;
                 for (index_t i = 0; i < dc.getSize(); ++i)
                     REQUIRE(dc[i] == randVec(i) / scalar);
@@ -268,7 +268,7 @@ SCENARIO("Testing the element-wise operations of DataContainer")
 
             THEN("the element-wise assignment of a scalar works as expected")
             {
-                float scalar = 123.45;
+                float scalar = 123.45f;
                 dc = scalar;
                 for (index_t i = 0; i < dc.getSize(); ++i)
                     REQUIRE(dc[i] == scalar);
@@ -359,7 +359,7 @@ SCENARIO("Testing the arithmetic operations with DataContainer arguments")
 
         THEN("the operations with a scalar work as expected")
         {
-            float scalar = 4.92;
+            float scalar = 4.92f;
 
             auto resultScalarPlus = scalar + dc;
             for (index_t i = 0; i < dc.getSize(); ++i)

elsa/core/tests/test_DataHandlerCPU.cpp

@@ -12,7 +12,7 @@
 #include "DataHandlerMapCPU.h"
 
 template <typename data_t>
-int elsa::useCount(const DataHandlerCPU<data_t>& dh)
+long elsa::useCount(const DataHandlerCPU<data_t>& dh)
 {
     return dh._data.use_count();
 }
@@ -34,7 +34,7 @@ TEMPLATE_TEST_CASE("Scenario: Constructing DataHandlerCPU", "", float, double, i
             THEN("it has a zero vector")
             {
                 for (index_t i = 0; i < size; ++i)
-                    REQUIRE(dh[i] == 0.0);
+                    REQUIRE(dh[i] == static_cast<TestType>(0));
             }
         }
 
@@ -467,9 +467,11 @@ TEMPLATE_TEST_CASE("Scenario: Testing the element-wise operations of DataHandler
                     REQUIRE((*dhExp)[i] == Approx(static_cast<TestType>(std::exp(randVec(i)))));
 
                 auto dhLog = dh.log();
-                for (index_t i = 0; i < size; ++i)
+                for (index_t i = 0; i < size; ++i) {
+                    TestType logVal = static_cast<TestType>(std::log(randVec(i)));
                     if (randVec(i) > 0)
-                        REQUIRE((*dhLog)[i] == Approx(static_cast<TestType>(log(randVec(i)))));
+                        REQUIRE((*dhLog)[i] == Approx(logVal));
+                }
             }
 
             THEN("the element-wise binary vector operations work as expected")
@@ -556,7 +558,7 @@ TEMPLATE_TEST_CASE("Scenario: Testing the element-wise operations of DataHandler
 
             THEN("the element-wise assignment of a scalar works as expected")
             {
-                TestType scalar = std::is_integral_v<TestType> ? 47 : 47.11;
+                auto scalar = std::is_integral_v<TestType> ? TestType(47) : TestType(47.11);
 
                 dh = scalar;
                 for (index_t i = 0; i < size; ++i)
@@ -624,7 +626,7 @@ TEMPLATE_TEST_CASE("Scenario: Testing the arithmetic operations with DataHandler
 
         THEN("the operations with a scalar work as expected")
         {
-            TestType scalar = std::is_integral_v<TestType> ? 5 : 4.7;
+            auto scalar = std::is_integral_v<TestType> ? TestType(5) : TestType(4.7);
 
             auto resultScalarPlus = scalar + dh;
             for (index_t i = 0; i < size; ++i)

elsa/core/tests/test_DataHandlerMapCPU.cpp

@@ -12,7 +12,7 @@
 #include "DataHandlerCPU.h"
 
 template <typename data_t>
-int elsa::useCount(const DataHandlerCPU<data_t>& dh)
+long elsa::useCount(const DataHandlerCPU<data_t>& dh)
 {
     return dh._data.use_count();
 }
@@ -182,7 +182,6 @@ TEMPLATE_TEST_CASE("Scenario: Assigning to DataHandlerMapCPU", "", float, double
 
         WHEN("copy assigning a partial DataHandlerMapCPU through base pointers")
         {
-            DataHandler<TestType>* dhPtr = &dh;
             const auto dhCopy = dh;
             Eigen::VectorX<TestType> randVec{2 * size};
             randVec.setRandom();
@@ -211,7 +210,6 @@ TEMPLATE_TEST_CASE("Scenario: Assigning to DataHandlerMapCPU", "", float, double
 
         WHEN("copy assigning a full DataHandlerMapCPU (aka a view) through base pointers")
         {
-            DataHandler<TestType>* dhPtr = &dh;
             const auto dhCopy = dh;
             Eigen::VectorX<TestType> randVec{size};
             randVec.setRandom();
@@ -269,7 +267,6 @@ TEMPLATE_TEST_CASE("Scenario: Assigning to DataHandlerMapCPU", "", float, double
 
         WHEN("\"move\" assigning a partial DataHandlerMapCPU through base pointers")
         {
-            DataHandler<TestType>* dhPtr = &dh;
             const auto dhCopy = dh;
             Eigen::VectorX<TestType> randVec{2 * size};
             randVec.setRandom();
@@ -298,7 +295,6 @@ TEMPLATE_TEST_CASE("Scenario: Assigning to DataHandlerMapCPU", "", float, double
 
         WHEN("\"move\" assigning a full DataHandlerMapCPU (aka a view) through base pointers")
         {
-            DataHandler<TestType>* dhPtr = &dh;
             const auto dhCopy = dh;
             Eigen::VectorX<TestType> randVec{size};
             randVec.setRandom();
@@ -667,7 +663,7 @@ TEMPLATE_TEST_CASE("Scenario: Testing the element-wise operations of DataHandler
                 auto dhLog = dh.log();
                 for (index_t i = 0; i < size; ++i)
                     if (randVec(i) > 0)
-                        REQUIRE((*dhLog)[i] == Approx(static_cast<TestType>(log(randVec(i)))));
+                        REQUIRE((*dhLog)[i] == Approx(static_cast<TestType>(std::log(randVec(i)))));
             }