added bounds checking to CArray (#40)

* added bounds checking to CArray

* add operator[] access and slicing

* doxygen, cppcheck, and some clangTidy

* add test of shivaAssertionFailed() for coverage

Author: rrsettgast

cmake/CMakeBasics.cmake

@@ -9,6 +9,8 @@ endif()
 set( SHIVA_BUILD_OBJ_LIBS OFF CACHE BOOL "" )
 
 
+option( SHIVA_ENABLE_BOUNDS_CHECK "Enable bounds checking in shiva::CArray" ON )
+
 if( CMAKE_CXX_STANDARD IN_LIST "98; 11; 14" )
     MESSAGE(FATAL_ERROR "Shiva requires at least c++17")
 endif()
@@ -25,4 +27,5 @@ blt_append_custom_compiler_flag( FLAGS_VAR CMAKE_CXX_FLAGS_DEBUG
                                  CLANG "-fstandalone-debug"
                                 )
 
+
 set( CAMP_ENABLE_TESTS OFF CACHE BOOL "")

cmake/Config.cmake

@@ -1,11 +1,12 @@
 #
 set( PREPROCESSOR_DEFINES CUDA
                           HIP
+                          BOUNDS_CHECK
                         )
 
 set( USE_CONFIGFILE ON CACHE BOOL "" )
 foreach( DEP in ${PREPROCESSOR_DEFINES})
-    if( ${DEP}_FOUND OR ENABLE_${DEP} )
+    if( ${DEP}_FOUND OR ENABLE_${DEP} OR SHIVA_ENABLE_${DEP} )
         set( SHIVA_USE_${DEP} TRUE )
     endif()
 endforeach()

cmake/Macros.cmake

@@ -34,14 +34,16 @@ macro(shiva_add_code_checks)
 
     set( CPPCHECK_FLAGS --std=c++17 
                         --enable=all 
+                        --quiet
                         --suppress=missingIncludeSystem
                         --suppress=unmatchedSuppression
                         --suppress=missingInclude 
                         --suppress=noConstructor 
                         --suppress=noExplicitConstructor
                         --suppress=unusedFunction 
                         --suppress=constStatement 
-                        --suppress=unusedStructMember )
+                        --suppress=unusedStructMember
+                        --suppress=unknownMacro )
 
     blt_add_code_checks( PREFIX    ${arg_PREFIX}
                          SOURCES   ${_sources}

cmake/blt

@@ -13,3 +13,5 @@
 /* #undef SHIVA_USE_HIP */
 
 /* #undef SHIVA_USE_CALIPER */
+
+#define SHIVA_USE_BOUNDS_CHECK

docs/doxygen/ShivaConfig.hpp

@@ -26,7 +26,7 @@ set( DOXYGEN_EXECUTABLE "${HOMEBREW_DIR}/bin/doxygen" CACHE PATH "" FORCE)
 set( CPPCHECK_EXECUTABLE "${HOMEBREW_DIR}/bin/cppcheck" CACHE PATH "" FORCE)
 set( CLANGTIDY_EXECUTABLE "${HOMEBREW_DIR}/opt/llvm/bin/clang-tidy" CACHE PATH "" FORCE)
 
-set( ENABLE_COVERAGE ON CACHE BOOL "" FORCE)
+set( ENABLE_COVERAGE OFF CACHE BOOL "" FORCE)
 set( GCOV_EXECUTABLE "/usr/bin/gcov" CACHE PATH "" FORCE)
 set( LCOV_EXECUTABLE "${HOMEBREW_DIR}/bin/lcov" CACHE PATH "" FORCE)
 

hostconfigs/apple/macOS_base.cmake

@@ -12,4 +12,6 @@
 
 #cmakedefine SHIVA_USE_HIP
 
-#cmakedefine SHIVA_USE_CALIPER
+#cmakedefine SHIVA_USE_CALIPER
+
+#cmakedefine SHIVA_USE_BOUNDS_CHECK

src/ShivaConfig.hpp.in

@@ -24,7 +24,7 @@
 #include "CArrayHelper.hpp"
 
 #include <utility>
-
+#include <cmath>
 
 
 namespace shiva
@@ -94,12 +94,14 @@ struct CArray
    * @brief accessor for m_data
    * @return reference to m_data
    */
+  SHIVA_CONSTEXPR_HOSTDEVICE_FORCEINLINE
   DATA_BUFFER & data() { return m_data; }
 
   /**
    * @brief const accessor for m_data
    * @return reference to const m_data
    */
+  SHIVA_CONSTEXPR_HOSTDEVICE_FORCEINLINE
   DATA_BUFFER const & data() const { return m_data; }
 
   /**
@@ -156,11 +158,98 @@ struct CArray
     return m_data[ CArrayHelper::linearIndexHelper< DIMS... >::eval( indices ... ) ];
   }
 
+private:
+  /**
+   * @struct SubArrayHelper
+   * @brief This struct is used to help with the creation of subarrays.
+   */
+  struct SubArrayHelper
+  {
+    /// This alias is used to create a const subarray type.
+
+    template< int ... SUB_DIMS >
+    using const_type = CArray< T const, T const * const, SUB_DIMS... >;
+
+    /// This alias is used to create a subarray type.
+    template< int ... SUB_DIMS >
+    using type = CArray< T, T * const, SUB_DIMS... >;
+  };
+
+  /**
+   * @brief Helper function to enable operator[].
+   * @tparam CARRAY_TYPE The type of the array to create. Either const_type or type.
+   * @param index The index to access.
+   * @return A reference to the data at the specified index or the subarray.
+   */
+  template< template< int ... > typename CARRAY_TYPE >
+  SHIVA_CONSTEXPR_HOSTDEVICE_FORCEINLINE
+  decltype(auto) squareBracketOperatorHelper( index_type index ) const;
+
+public:
+  /**
+   * @brief operator[] to access the data in the array or to slice a multidimensional array.
+   * @param index The index to access.
+   * @return A reference to the data at the specified index or the subarray.
+   */
+  SHIVA_CONSTEXPR_HOSTDEVICE_FORCEINLINE
+  decltype(auto) operator[]( index_type index ) const
+  {
+    return squareBracketOperatorHelper< SubArrayHelper::template const_type >( index );
+  }
+
+  /**
+   * @brief operator[] to access the data in the array or to slice a mutlidimensional array.
+   * @param index The index to access.
+   * @return A reference to the data at the specified index or the subarray.
+   */
+  SHIVA_CONSTEXPR_HOSTDEVICE_FORCEINLINE
+  decltype(auto) operator[]( index_type index )
+  {
+    if constexpr ( std::is_reference_v< decltype(squareBracketOperatorHelper< SubArrayHelper::template type >( index )) > )
+    {
+      return const_cast< T & >(squareBracketOperatorHelper< SubArrayHelper::template type >( index ));
+    }
+    else
+    {
+      return squareBracketOperatorHelper< SubArrayHelper::template type >( index );
+    }
+  }
+
+
 private:
   /// The data in the array.
   DATA_BUFFER m_data;
 };
 
+/// @copydoc CArray::squareBracketOperatorHelper
+template< typename T, typename DATA_BUFFER, int ... DIMS >
+template< template< int ... > typename CARRAY_TYPE >
+SHIVA_CONSTEXPR_HOSTDEVICE_FORCEINLINE
+decltype(auto)
+CArray< T, DATA_BUFFER, DIMS ... >::squareBracketOperatorHelper( index_type index ) const
+{
+  static_assert( sizeof...(DIMS) >= 1, "operator[] is only valid for sizeof...(DIMS) >= 1" );
+
+#if defined( SHIVA_USE_BOUNDS_CHECK )
+  constexpr int DIM = CArrayHelper::IntPeeler< DIMS... >::first;
+  SHIVA_ASSERT_MSG( index >= 0 && index < DIM,
+                    "Index out of bounds: 0 < index(%jd) < dim(%jd)",
+                    static_cast< intmax_t >( index ),
+                    static_cast< intmax_t >( DIM ) );
+#endif
+
+  if constexpr ( sizeof...(DIMS) > 1 )
+  {
+    using SubArrayDims = typename CArrayHelper::IntPeeler< DIMS... >::rest;
+    using SubArrayType = typename CArrayHelper::ApplyDims< SubArrayDims, CARRAY_TYPE >::type;
+    return SubArrayType( const_cast< T * >( &m_data[ index * CArrayHelper::stride< DIMS..., 1 >() ] ) );
+  }
+  else
+  {
+    return operator()( index );
+  }
+}
+
 /**
  * @brief Alias for a scalar.
  * @tparam T Type held in the scalar

src/common/CArray.hpp

@@ -19,10 +19,11 @@
 #pragma once
 
 
-#include "common/ShivaMacros.hpp"
+#include "common/ShivaErrorHandling.hpp"
+#include "common/types.hpp"
 #include <utility>
-
-
+#include <cstdint>
+#include <cinttypes>
 
 namespace shiva
 {
@@ -93,6 +94,7 @@ struct linearIndexHelper
   SHIVA_STATIC_CONSTEXPR_HOSTDEVICE_FORCEINLINE int
   level()
   {
+    static_assert( INDEX >= 0 && INDEX < DIM );
     constexpr int thisStride = strideHelper< DIMS ..., 1 >();
     if constexpr ( sizeof ... ( DIMS ) == 0 )
     {
@@ -108,17 +110,23 @@ struct linearIndexHelper
    * @copydoc level()
    */
   template< typename INDEX_TYPE, typename ... INDICES_TYPE >
-  SHIVA_STATIC_CONSTEXPR_HOSTDEVICE_FORCEINLINE int
+  static constexpr SHIVA_HOST_DEVICE SHIVA_FORCE_INLINE int
   level( INDEX_TYPE const index, INDICES_TYPE const ... indices )
   {
+#if defined( SHIVA_USE_BOUNDS_CHECK )
+    SHIVA_ASSERT_MSG( index >= 0 && index < DIM,
+                      "Index out of bounds: 0 < index(%jd) < dim(%jd)",
+                      static_cast< intmax_t >( index ),
+                      static_cast< intmax_t >( DIM ) );
+#endif
     constexpr int thisStride = strideHelper< DIMS ..., 1 >();
     if constexpr ( sizeof ... ( DIMS ) == 0 )
     {
       return index * thisStride;
     }
     else
     {
-      return index * thisStride + linearIndexHelper< DIMS ... >::template level( std::forward< INDICES_TYPE const >( indices )... );
+      return index * thisStride + linearIndexHelper< DIMS ... >::template level< INDICES_TYPE ... >( std::forward< INDICES_TYPE const >( indices )... );
     }
   }
 
@@ -191,6 +199,44 @@ struct Peeler
 {
   /// The type of the first value in the pack.
   using type = T;
+
+  /// The type of the remaining values in the pack.
+  using types = tuple< Ts ... >;
+};
+
+/**
+ * @brief function to peel an integer from a parameter pack.
+ * @tparam INT The first value in the pack.
+ * @tparam INTS The remaining values in the pack.
+ */
+template< int INT, int ... INTS >
+struct IntPeeler
+{
+  /// The type of the first value in the pack.
+  static constexpr int first = INT;
+
+  /// The type of the remaining values in the pack.
+  using rest = std::integer_sequence< int, INTS... >;
+};
+
+/**
+ * @brief function to apply a parameter pack to a templated type.
+ * @tparam Ints The integer sequence to apply.
+ * @tparam Target The target type to apply the integer sequence to.
+ */
+template< typename Seq, template< int... > class Target >
+struct ApplyDims;
+
+/**
+ * @brief function to apply a parameter pack to a templated type.
+ * @tparam Ints The integer sequence to apply.
+ * @tparam Target The target type to apply the integer sequence to.
+ */
+template< int... Ints, template< int... > class Target >
+struct ApplyDims< std::integer_sequence< int, Ints... >, Target >
+{
+  /// The type of the target that has the pack applied its varaidic pack.
+  using type = Target< Ints... >;
 };
 
 } // namespace CArrayHelper

src/common/CArrayHelper.hpp

@@ -9,6 +9,7 @@ set( common_headers
      pmpl.hpp
      SequenceUtilities.hpp
      NestedSequenceUtilities.hpp
+     ShivaErrorHandling.hpp
      ShivaMacros.hpp
      types.hpp
      )