Improve warnings

ports-of-call/array.hpp

@@ -194,9 +194,13 @@ struct array {
 
   //! swap the array contents with another
   PORTABLE_FUNCTION constexpr void swap(array &other) {
-    using std::swap;
     for (size_type i = 0; i < N; ++i) {
-      swap(arr[i], other.arr[i]);
+      // C++20 makes std::swap constexpr, so this should probably be replaced
+      // at that point.  For now, this is required if a user wants to call swap
+      // on a GPU.
+      T t = arr[i];
+      arr[i] = other.arr[i];
+      other.arr[i] = t;
     }
   }
 };

ports-of-call/portability.hpp

@@ -153,7 +153,8 @@ void portableCopyToHost(T *const to, T const *const from, size_t const size_byte
 }
 
 template <typename Function>
-void portableFor(const char *name, int start, int stop, Function function) {
+void portableFor([[maybe_unused]] const char *name, int start, int stop,
+                 Function function) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using policy = Kokkos::RangePolicy<>;
   Kokkos::parallel_for(name, policy(start, stop), function);
@@ -165,8 +166,8 @@ void portableFor(const char *name, int start, int stop, Function function) {
 }
 
 template <typename Function>
-void portableFor(const char *name, int starty, int stopy, int startx, int stopx,
-                 Function function) {
+void portableFor([[maybe_unused]] const char *name, int starty, int stopy, int startx,
+                 int stopx, Function function) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using Policy2D = Kokkos::MDRangePolicy<Kokkos::Rank<2>>;
   Kokkos::parallel_for(name, Policy2D({starty, startx}, {stopy, stopx}), function);
@@ -180,8 +181,8 @@ void portableFor(const char *name, int starty, int stopy, int startx, int stopx,
 }
 
 template <typename Function>
-void portableFor(const char *name, int startz, int stopz, int starty, int stopy,
-                 int startx, int stopx, Function function) {
+void portableFor([[maybe_unused]] const char *name, int startz, int stopz, int starty,
+                 int stopy, int startx, int stopx, Function function) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using Policy3D = Kokkos::MDRangePolicy<Kokkos::Rank<3>>;
   Kokkos::parallel_for(name, Policy3D({startz, starty, startx}, {stopz, stopy, stopx}),
@@ -198,8 +199,9 @@ void portableFor(const char *name, int startz, int stopz, int starty, int stopy,
 }
 
 template <typename Function>
-void portableFor(const char *name, int starta, int stopa, int startz, int stopz,
-                 int starty, int stopy, int startx, int stopx, Function function) {
+void portableFor([[maybe_unused]] const char *name, int starta, int stopa, int startz,
+                 int stopz, int starty, int stopy, int startx, int stopx,
+                 Function function) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using Policy4D = Kokkos::MDRangePolicy<Kokkos::Rank<4>>;
   Kokkos::parallel_for(
@@ -219,9 +221,9 @@ void portableFor(const char *name, int starta, int stopa, int startz, int stopz,
 }
 
 template <typename Function>
-void portableFor(const char *name, int startb, int stopb, int starta, int stopa,
-                 int startz, int stopz, int starty, int stopy, int startx, int stopx,
-                 Function function) {
+void portableFor([[maybe_unused]] const char *name, int startb, int stopb, int starta,
+                 int stopa, int startz, int stopz, int starty, int stopy, int startx,
+                 int stopx, Function function) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using Policy5D = Kokkos::MDRangePolicy<Kokkos::Rank<5>>;
   Kokkos::parallel_for(name,
@@ -244,8 +246,8 @@ void portableFor(const char *name, int startb, int stopb, int starta, int stopa,
 }
 
 template <typename Function, typename T>
-void portableReduce(const char *name, int start, int stop, Function function,
-                    T &reduced) {
+void portableReduce([[maybe_unused]] const char *name, int start, int stop,
+                    Function function, T &reduced) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using Policy = Kokkos::RangePolicy<>;
   Kokkos::parallel_reduce(name, Policy(start, stop), function, reduced);
@@ -257,8 +259,8 @@ void portableReduce(const char *name, int start, int stop, Function function,
 }
 
 template <typename Function, typename T>
-void portableReduce(const char *name, int starty, int stopy, int startx, int stopx,
-                    Function function, T &reduced) {
+void portableReduce([[maybe_unused]] const char *name, int starty, int stopy, int startx,
+                    int stopx, Function function, T &reduced) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using Policy2D = Kokkos::MDRangePolicy<Kokkos::Rank<2>>;
   Kokkos::parallel_reduce(name, Policy2D({starty, startx}, {stopy, stopx}), function,
@@ -273,8 +275,8 @@ void portableReduce(const char *name, int starty, int stopy, int startx, int sto
 }
 
 template <typename Function, typename T>
-void portableReduce(const char *name, int startz, int stopz, int starty, int stopy,
-                    int startx, int stopx, Function function, T &reduced) {
+void portableReduce([[maybe_unused]] const char *name, int startz, int stopz, int starty,
+                    int stopy, int startx, int stopx, Function function, T &reduced) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using Policy3D = Kokkos::MDRangePolicy<Kokkos::Rank<3>>;
   Kokkos::parallel_reduce(name, Policy3D({startz, starty, startx}, {stopz, stopy, stopx}),
@@ -291,9 +293,9 @@ void portableReduce(const char *name, int startz, int stopz, int starty, int sto
 }
 
 template <typename Function, typename T>
-void portableReduce(const char *name, int starta, int stopa, int startz, int stopz,
-                    int starty, int stopy, int startx, int stopx, Function function,
-                    T &reduced) {
+void portableReduce([[maybe_unused]] const char *name, int starta, int stopa, int startz,
+                    int stopz, int starty, int stopy, int startx, int stopx,
+                    Function function, T &reduced) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using Policy4D = Kokkos::MDRangePolicy<Kokkos::Rank<4>>;
   Kokkos::parallel_reduce(
@@ -313,9 +315,9 @@ void portableReduce(const char *name, int starta, int stopa, int startz, int sto
 }
 
 template <typename Function, typename T>
-void portableReduce(const char *name, int startb, int stopb, int starta, int stopa,
-                    int startz, int stopz, int starty, int stopy, int startx, int stopx,
-                    Function function, T &reduced) {
+void portableReduce([[maybe_unused]] const char *name, int startb, int stopb, int starta,
+                    int stopa, int startz, int stopz, int starty, int stopy, int startx,
+                    int stopx, Function function, T &reduced) {
 #ifdef PORTABILITY_STRATEGY_KOKKOS
   using Policy5D = Kokkos::MDRangePolicy<Kokkos::Rank<5>>;
   Kokkos::parallel_reduce(name,

ports-of-call/span.hpp

@@ -55,6 +55,16 @@ namespace detail {
 using std::data;
 using std::size;
 
+// type-safe check against 0 (prevents warnings about comparing an unsigned against 0)
+template <typename T, std::enable_if_t<std::is_unsigned<T>::value, bool> = true>
+PORTABLE_FUNCTION constexpr bool check_nonnegative(const T) {
+  return true;
+}
+template <typename T, std::enable_if_t<!std::is_unsigned<T>::value, bool> = true>
+PORTABLE_FUNCTION constexpr bool check_nonnegative(const T t) {
+  return t >= 0;
+}
+
 // object to handle storage of span
 template <class E, std::size_t S>
 struct span_storage {
@@ -195,7 +205,8 @@ class span {
   // explicit: extent != dynamic_extent
   // NB: iterator concepts to further restrict overload resolution
   constexpr span(pointer ptr, size_type count) : storage_(ptr, count) {
-    span_EXPECTS((ptr == nullptr && count == 0) || (ptr != nullptr && count >= 0));
+    span_EXPECTS((ptr == nullptr && count == 0) ||
+                 (ptr != nullptr && detail::check_nonnegative(count)));
   }
 
   // constructs a span that is a view over the range [first, last)
@@ -205,7 +216,7 @@ class span {
   // NB: iterator concepts to further restrict overload resolution
   constexpr span(pointer first_elem, pointer last_elem)
       : storage_(first_elem, last_elem - first_elem) {
-    span_EXPECTS(last_elem - first_elem >= 0);
+    span_EXPECTS(detail::check_nonnegative(last_elem - first_elem));
   }
 
   // constructs a span that is a view over arr
@@ -272,25 +283,25 @@ class span {
   // https://en.cppreference.com/w/cpp/container/span/first
   template <std::size_t Count>
   constexpr span<element_type, Count> first() const {
-    span_EXPECTS(Count >= 0 && Count <= size());
+    span_EXPECTS(detail::check_nonnegative(Count) && Count <= size());
     return {data(), Count};
   }
 
   constexpr span<element_type, dynamic_extent> first(size_type count) const {
-    span_EXPECTS(count >= 0 && count <= size());
+    span_EXPECTS(detail::check_nonnegative(count) && count <= size());
     return {data(), count};
   }
 
   // obtains a span that is a view over the last Count elements of this span
   // https://en.cppreference.com/w/cpp/container/span/last
   template <std::size_t Count>
   constexpr span<element_type, Count> last() const {
-    span_EXPECTS(Count >= 0 && Count <= size());
+    span_EXPECTS(detail::check_nonnegative(Count) && Count <= size());
     return {data() + (size() - Count), Count};
   }
 
   constexpr span<element_type, dynamic_extent> last(size_type count) const {
-    span_EXPECTS(count >= 0 && count <= size());
+    span_EXPECTS(detail::check_nonnegative(count) && count <= size());
     return {data() + (size() - count), count};
   }
 
@@ -305,16 +316,17 @@ class span {
 
   template <std::size_t Offset, std::size_t Count = dynamic_extent>
   constexpr subspan_return_t<Offset, Count> subspan() const {
-    span_EXPECTS((Offset >= 0 && Offset <= size()) &&
-                 (Count == dynamic_extent || (Count >= 0 && Count + Offset <= size())));
+    span_EXPECTS((detail::check_nonnegative(Offset) && Offset <= size()) &&
+                 (Count == dynamic_extent ||
+                  (detail::check_nonnegative(Count) && Count + Offset <= size())));
     return {data() + Offset, Count != dynamic_extent ? Count : size() - Offset};
   }
 
   constexpr span<element_type, dynamic_extent>
   subspan(size_type offset, size_type count = dynamic_extent) const {
-    span_EXPECTS((offset >= 0 && offset <= size()) &&
+    span_EXPECTS((detail::check_nonnegative(offset) && offset <= size()) &&
                  (count == static_cast<size_type>(dynamic_extent) ||
-                  (count >= 0 && count + offset <= size())));
+                  (detail::check_nonnegative(count) && count + offset <= size())));
 
     return {data() + offset, count == dynamic_extent ? size() - offset : count};
   }
@@ -337,7 +349,7 @@ class span {
 
   // [span.element_access]--
   constexpr reference operator[](size_type idx) const {
-    span_EXPECTS(idx >= 0 && idx < size());
+    span_EXPECTS(detail::check_nonnegative(idx) && idx < size());
     return *(data() + idx);
   }
 
@@ -381,7 +393,7 @@ span(const std::array<T, N> &) -> span<const T, N>;
 
 template <class Container>
 span(Container &) -> span<typename std::remove_reference<
-                      decltype(*detail::data(std::declval<Container &>()))>::type>;
+    decltype(*detail::data(std::declval<Container &>()))>::type>;
 
 template <class Container>
 span(const Container &) -> span<const typename Container::value_type>;

test/CMakeLists.txt

@@ -56,6 +56,41 @@ elseif (PORTS_OF_CALL_TEST_PORTABILITY_STRATEGY STREQUAL "Cuda")
   target_compile_definitions(portsofcall_iface INTERFACE PORTABILITY_STRATEGY_CUDA)
 endif()
 
+if(NOT PORTS_OF_CALL_SILENCE_WARNINGS)
+    # turn on strict warnings and error
+    # NOTE: The ROCm 6.2.1 installation on some machines (RZAdams and RZVernal at least) will
+    #       inject a `--gcc-toolchain` command-line argument into Clang, which is a deprecated
+    #       option.  Clang will warn about unused command-line arguments, so you get multiple
+    #       instances of that warning.  Then `-Werror` promotes those to errors.  We can turn these
+    #       back to non-fatal warnings with `-Wno-error=unused-command-line-argument` or we can
+    #       turn off that particular warning with `-Wno-unused-command-line-argument`.
+    # TODO: find comiler flags for non-GNU compatible compilers
+    target_compile_options(portsofcall_iface INTERFACE
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},GNU>:-Werror;-Wall;-pedantic-errors;-Wunused-parameter>
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},AppleClang>:-Werror;-Wall;-Wunused-parameter>
+        $<$<NOT:$<BOOL:${WIN32}>>:$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Clang>:-Werror;-Wall;-Wno-unused-command-line-argument>>
+        $<$<BOOL:${WIN32}>:$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Clang>:-Wall;-Wno-c++98-compat;-Wno-c++98-compat-pedantic>>
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Intel>:-Werror;-w2;-Wunused-variable;-Wunused-parameter;-diag-disable=remark>
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},PGI>:>
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},MSVC>:/permissive->
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},XL>:>
+    )
+else()
+    # turn off warnings
+    # TODO: find comiler flags for non-GNU compatible compilers
+    target_compile_options(portsofcall_iface INTERFACE
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},GNU>:-w>
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},AppleClang>:>
+        $<$<NOT:$<BOOL:${WIN32}>>:$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Clang>:>>
+        $<$<BOOL:${WIN32}>:$<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Clang>:>>
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},Intel>:>
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},PGI>:>
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},MSVC>:>
+        $<$<IN_LIST:${CMAKE_CXX_COMPILER_ID},XL>:>
+    )
+endif()
+
+
 target_link_libraries(portsofcall_iface INTERFACE Catch2::Catch2)
 
 add_executable(test_portsofcall test_main.cpp)

test/test_array.cpp

@@ -119,7 +119,7 @@ TEST_CASE("array begins and ends", "[array]") {
   using std::end;
 
   SECTION("with non-const array") {
-    array<int, 3> arr{{1, 2, 3}};
+    [[maybe_unused]] array<int, 3> arr{{1, 2, 3}};
 
     CHECK(std::is_same<int *, decltype(begin(arr))>::value);
     CHECK(std::is_same<int const *, decltype(cbegin(arr))>::value);
@@ -129,7 +129,7 @@ TEST_CASE("array begins and ends", "[array]") {
   }
 
   SECTION("with const array") {
-    array<int, 3> const arr{{1, 2, 3}};
+    [[maybe_unused]] array<int, 3> const arr{{1, 2, 3}};
 
     CHECK(std::is_same<int const *, decltype(begin(arr))>::value);
     CHECK(std::is_same<int const *, decltype(cbegin(arr))>::value);
@@ -139,13 +139,13 @@ TEST_CASE("array begins and ends", "[array]") {
   }
 
   SECTION("with non-const zero-sized array") {
-    array<int, 0> arr;
+    [[maybe_unused]] array<int, 0> arr;
 
     CHECK(begin(arr) == end(arr));
   }
 
   SECTION("with const zero-sized array") {
-    array<int, 0> const arr{};
+    [[maybe_unused]] array<int, 0> const arr{};
 
     CHECK(begin(arr) == end(arr));
   }
@@ -208,7 +208,7 @@ TEST_CASE("array sizes", "[array]") {
 TEST_CASE("array fill (GPU)", "[array][GPU]") {
   constexpr std::size_t N = 42;
   std::size_t count = 0;
-  auto func = PORTABLE_LAMBDA(const int i, std::size_t &count) {
+  auto func = PORTABLE_LAMBDA(const int /*i*/, std::size_t &count) {
     constexpr double value = 3.14;
     array<double, N> arr;
     arr.fill(value);
@@ -260,21 +260,21 @@ TEST_CASE("array swap", "[array]") {
 }
 
 TEST_CASE("array tuple_size", "[array]") {
-  array<double, 5> arr;
+  [[maybe_unused]] array<double, 5> arr;
 
   CHECK(std::tuple_size<decltype(arr)>::value == 5);
 }
 
 TEST_CASE("array tuple_element", "[array]") {
   struct Foo {};
 
-  array<Foo, 3> foos;
+  [[maybe_unused]] array<Foo, 3> foos;
 
   CHECK(std::is_same<Foo, std::tuple_element<1, decltype(foos)>::type>::value);
 }
 
 TEST_CASE("make_array function", "[array]") {
-  auto arr = make_array(1.0, 2.0, 3.0);
+  [[maybe_unused]] auto arr = make_array(1.0, 2.0, 3.0);
 
   CHECK(std::is_same<decltype(arr), array<double, 3>>::value);
 }

test/test_portability.cpp

@@ -102,7 +102,7 @@ TEST_CASE("portableCopy works with all portability strategies", "[portableCopy]"
   int sum{0};
   portableReduce(
       "check portableCopy", 0, N, 0, 0, 0, 0,
-      PORTABLE_LAMBDA(const int &i, const int &j, const int &k, int &isum) {
+      PORTABLE_LAMBDA(const int &i, const int & /*j*/, const int & /*k*/, int &isum) {
         if (a[i] != index_func(i)) {
           isum += 1;
         }
@@ -121,7 +121,7 @@ TEST_CASE("portableCopy works with all portability strategies", "[portableCopy]"
 
   // count elements that don't match reference
   sum = 0;
-  for (int i = 0; i < N; ++i) {
+  for (size_t i = 0; i < N; ++i) {
     if (b[i] != index_func(i)) {
       sum += 1;
     }

test/test_span.cpp

@@ -61,8 +61,7 @@ constexpr void generate(ForwardIt first, ForwardIt last, Generator g) {
 }
 
 template <class T, std::size_t N>
-[[nodiscard]]
-constexpr auto slide(span<T, N> s, std::size_t offset, std::size_t width) {
+[[nodiscard]] constexpr auto slide(span<T, N> s, std::size_t offset, std::size_t width) {
   return s.subspan(offset, offset + width <= s.size() ? width : 0U);
 }
 
@@ -569,7 +568,7 @@ TEST_CASE("span portability", "[PortsOfCall::span]") {
   constexpr Real d_gp = 2. * pi / static_cast<Real>(N);
 
   std::vector<Real> h_gp(N);
-  for (auto i = 0; i < N; ++i) {
+  for (std::size_t i = 0; i < N; ++i) {
     h_gp[i] = -pi + static_cast<Real>(i) * d_gp;
   }