build: enable -Werror on TA translation units (opt-in via TA_WERROR)

.github/workflows/ci.yml

@@ -39,6 +39,7 @@ jobs:
         -DCMAKE_PREFIX_PATH="/usr/local/opt/bison;/usr/local/opt/scalapack;/usr/local/opt/boost"
         -DTA_ASSERT_POLICY=TA_ASSERT_THROW
         -DTA_SCALAPACK=ON
+        -DTA_WERROR=ON
 
     steps:
     - uses: actions/checkout@v4

.gitlab-ci.yml

@@ -12,6 +12,7 @@ variables:
   TA_CONFIG : >
     CMAKE_BUILD_TYPE=${BUILD_TYPE}
     TA_ASSERT_POLICY=TA_ASSERT_THROW
+    TA_WERROR=ON
     TA_UT_CTEST_TIMEOUT=3000
     ${TA_PYTHON}
     ${TA_CUDA}

CMakeLists.txt

@@ -149,6 +149,10 @@ add_feature_info(TENSOR_ASSERT_NO_MUTABLE_OPS_WHILE_SHARED TA_TENSOR_ASSERT_NO_M
 
 option(TA_EXPERT "TiledArray Expert mode: disables automatically downloading or building dependencies" OFF)
 
+redefaultable_option(TA_WERROR "Treat compiler warnings as errors when compiling TiledArray's own translation units (does not propagate to consumers of installed TiledArray targets)" OFF)
+add_feature_info(WERROR TA_WERROR "Treat compiler warnings as errors on TiledArray's own translation units")
+include(TiledArrayWarnings)
+
 option(TA_SIGNED_1INDEX_TYPE "Enables the use of signed 1-index coordinate type (OFF in 1.0.0-alpha.2 and older)" ON)
 add_feature_info(SIGNED_1INDEX_TYPE TA_SIGNED_1INDEX_TYPE "Use of signed 1-index coordinate type in TiledArray")
 

INSTALL.md

@@ -428,6 +428,7 @@ support may be added.
 * `BUILD_TESTING` -- Set of `OFF` to disable building unit tests. The default is `ON`.
 * `TA_TRACE_TASKS` -- Set to `ON` to enable tracing of MADNESS tasks using custom task tracer. Note that standard profilers/tracers are generally useless (except in the trivial cases) with MADWorld-based programs since the submission context of tasks is not captured by standard tracing tools; this makes it impossible in a nontrivial program to attribute tasks to source code. WARNING: task tracing his will greatly increase the memory requirements. [Default=OFF].
 * `TA_TTG` -- Set to `ON` to find or fetch the TTG library. [Default=OFF].
+* `TA_WERROR` -- Set to `ON` to treat compiler warnings as errors when compiling TiledArray's own translation units (the `tiledarray` library and in-tree tests/examples). Also implies `MADNESS_WERROR=ON` for the MADworld translation units built as part of TA's FetchContent tree. Does **not** propagate to consumers of the installed `tiledarray` target (i.e. `find_package(tiledarray)` users do not inherit `-Werror`). Honored on GNU/Clang/AppleClang/IntelLLVM. [Default=OFF].
 * `TA_SIGNED_1INDEX_TYPE` -- Set to `OFF` to use unsigned 1-index coordinate type (default for TiledArray 1.0.0-alpha.2 and older). The default is `ON`, which enables the use of negative indices in coordinates.
 * `TA_MAX_SOO_RANK_METADATA` -- Specifies the maximum rank for which to use Small Object Optimization (hence, avoid the use of the heap) for metadata. The default is `8`.
 * `TA_TENSOR_MEM_PROFILE` -- Set to `ON` to profile host memory allocations used by TA::Tensor. This causes the use of Umpire for host memory allocation. This also enables additional tracing facilities provided by Umpire; these can be controlled via [environment variable `UMPIRE_LOG_LEVEL`](https://umpire.readthedocs.io/en/develop/sphinx/features/logging_and_replay.html), but note that the default is to log Umpire info into a file rather than stdout.

cmake/modules/AddTAExecutable.cmake

@@ -2,5 +2,6 @@ macro(add_ta_executable _name _source_files _libs)
 
   add_executable(${_name} EXCLUDE_FROM_ALL "${_source_files}")
   target_link_libraries(${_name} PRIVATE "${_libs}")
+  target_link_libraries(${_name} PRIVATE tiledarray_internal_warnings)
 
 endmacro()

cmake/modules/FindOrFetchBTAS.cmake

@@ -75,3 +75,20 @@ endif(NOT TARGET BTAS::BTAS)
 if (NOT TARGET BTAS::BTAS)
   message(FATAL_ERROR "FindOrFetchBTAS could not make BTAS::BTAS target available")
 endif(NOT TARGET BTAS::BTAS)
+
+# Treat BTAS headers as system: header-only library, no include-order
+# risk against TA's headers, and BTAS upstream trips warnings TA itself
+# can't fix (e.g. btas/generic/converge_class.h -Wreturn-type on gcc).
+# Carved out specifically here despite the top-level
+# CMAKE_NO_SYSTEM_FROM_IMPORTED=TRUE.
+get_target_property(_btas_aliased BTAS::BTAS ALIASED_TARGET)
+if (NOT _btas_aliased)
+  set(_btas_aliased BTAS::BTAS)
+endif()
+get_target_property(_btas_inc ${_btas_aliased} INTERFACE_INCLUDE_DIRECTORIES)
+if (_btas_inc)
+  set_target_properties(${_btas_aliased} PROPERTIES
+      INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${_btas_inc}")
+endif()
+unset(_btas_inc)
+unset(_btas_aliased)

cmake/modules/FindOrFetchMADWorld.cmake

@@ -22,6 +22,13 @@ if (NOT TARGET MADworld)
     endif()
   endif()
   set(MPI_THREAD "multiple" CACHE INTERNAL "MADNESS requires MPI_THREAD_MULTIPLE")
+  # TA_WERROR=ON implies MADNESS_WERROR=ON: warnings-as-errors should cover
+  # the MADNESS translation units built as part of TA's FetchContent tree.
+  # (Requires a MADNESS pin that includes the MADNESS_WERROR option; otherwise
+  #  this cache variable is harmlessly ignored.)
+  if (TA_WERROR)
+    set(MADNESS_WERROR ON CACHE BOOL "Treat compiler warnings as errors when compiling MADNESS's own translation units" FORCE)
+  endif()
   set(MADNESS_ASSUMES_ASLR_DISABLED ${TA_ASSUMES_ASLR_DISABLED} CACHE BOOL "Whether MADNESS assumes ASLR to be disabled")
   set(MPI_CXX_SKIP_MPICXX ON CACHE BOOL "Whether to disable search for C++ MPI-2 bindings")
   set(DISABLE_WORLD_GET_DEFAULT ON CACHE INTERNAL "Whether to disable madness::World::get_default()")

cmake/modules/FindOrFetchRangeV3.cmake

@@ -31,3 +31,23 @@ endif(NOT TARGET range-v3::range-v3)
 if (NOT TARGET range-v3::range-v3)
   message(FATAL_ERROR "FindOrFetchRangeV3 could not make range-v3::range-v3 target available")
 endif(NOT TARGET range-v3::range-v3)
+
+# Treat range-v3 headers as system: range-v3 is header-only with no
+# ordering risk against TA's headers, and it self-triggers
+# -Wdeprecated-declarations (e.g. ranges::compressed_tuple used inside
+# compressed_pair.hpp). The blanket CMAKE_NO_SYSTEM_FROM_IMPORTED=TRUE
+# at the top-level avoids -isystem for general imported targets due to
+# include-dir ordering; carve out range-v3 specifically here.
+# range-v3::range-v3 is an ALIAS — resolve to the underlying target before
+# touching properties.
+get_target_property(_rv3_aliased range-v3::range-v3 ALIASED_TARGET)
+if (NOT _rv3_aliased)
+  set(_rv3_aliased range-v3::range-v3)
+endif()
+get_target_property(_rv3_inc ${_rv3_aliased} INTERFACE_INCLUDE_DIRECTORIES)
+if (_rv3_inc)
+  set_target_properties(${_rv3_aliased} PROPERTIES
+      INTERFACE_SYSTEM_INCLUDE_DIRECTORIES "${_rv3_inc}")
+endif()
+unset(_rv3_inc)
+unset(_rv3_aliased)

cmake/modules/TiledArrayWarnings.cmake

@@ -0,0 +1,53 @@
+include_guard(GLOBAL)
+
+# TiledArray internal warning-policy target.
+#
+# Owns an INTERFACE library `tiledarray_internal_warnings` that carries
+# the warning flags applied to TiledArray's own translation units. The
+# target is linked PRIVATE-ly to the `tiledarray` library and to
+# in-tree executables (via add_ta_executable). PRIVATE scope is
+# load-bearing: it keeps these flags out of INTERFACE_COMPILE_OPTIONS
+# on the installed/exported `tiledarray` target, so downstream
+# consumers (MPQC, ...) do not inherit -Werror through
+# find_package(tiledarray).
+#
+# The target is also NOT installed/exported, which keeps it out of the
+# package.
+
+add_library(tiledarray_internal_warnings INTERFACE)
+
+if (TA_WERROR)
+  if (CMAKE_CXX_COMPILER_ID MATCHES "^(GNU|Clang|AppleClang|IntelLLVM)$")
+    target_compile_options(tiledarray_internal_warnings INTERFACE
+      $<$<OR:$<COMPILE_LANGUAGE:CXX>,$<COMPILE_LANGUAGE:C>,$<COMPILE_LANGUAGE:HIP>>:-Werror>
+      # NVCC: forward -Werror to the host compiler, not to nvcc itself
+      # (nvcc's own -Werror is a different switch with a different surface).
+      $<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=-Werror>)
+    # gcc's interprocedural-after-inlining warnings have a long history of
+    # false positives — particularly across template-heavy inlining and
+    # idiomatic throw-on-assert patterns — that are repeatedly traded
+    # across releases (gcc-12/13/14/15 all have outstanding upstream
+    # bugzilla PRs in this family). Demote the noisiest of them to plain
+    # warnings on gcc so they still surface in build logs but do not
+    # gate CI. Clang does not exhibit these and stays under -Werror.
+    if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+      set(_ta_gcc_ipa_warnings
+          -Wno-error=nonnull
+          -Wno-error=stringop-overflow
+          -Wno-error=stringop-overread
+          -Wno-error=array-bounds
+          -Wno-error=dangling-pointer
+          -Wno-error=use-after-free
+          -Wno-error=restrict
+          -Wno-error=maybe-uninitialized)
+      foreach(_flag IN LISTS _ta_gcc_ipa_warnings)
+        target_compile_options(tiledarray_internal_warnings INTERFACE
+          $<$<OR:$<COMPILE_LANGUAGE:CXX>,$<COMPILE_LANGUAGE:C>>:${_flag}>
+          $<$<COMPILE_LANGUAGE:CUDA>:-Xcompiler=${_flag}>)
+      endforeach()
+      unset(_ta_gcc_ipa_warnings)
+    endif()
+  else()
+    message(WARNING "TA_WERROR=ON but compiler '${CMAKE_CXX_COMPILER_ID}' is not in the supported set; ignoring.")
+  endif()
+endif()

examples/scalapack/CMakeLists.txt

@@ -31,6 +31,7 @@ foreach(_exec conversion evp)
   # Add executable
   add_executable(scalapack-${_exec} EXCLUDE_FROM_ALL ${_exec}.cpp)
   target_link_libraries(scalapack-${_exec} PRIVATE tiledarray)
+  target_link_libraries(scalapack-${_exec} PRIVATE tiledarray_internal_warnings)
   add_dependencies(examples-tiledarray scalapack-${_exec})
 
 endforeach()

external/versions.cmake

@@ -12,8 +12,8 @@ set(TA_INSTALL_EIGEN_URL_HASH SHA256=b4c198460eba6f28d34894e3a5710998818515104d6
 set(TA_INSTALL_EIGEN_PREVIOUS_URL_HASH MD5=b9e98a200d2455f06db9c661c5610496)
 
 set(TA_TRACKED_MADNESS_URL https://github.com/m-a-d-n-e-s-s/madness.git CACHE STRING "GIT_REPOSITORY for cloning MADNESS source")
-set(TA_TRACKED_MADNESS_TAG dd8f5daa4fb40693ca1ef728b8a4e93f31d39955 CACHE STRING "GIT_TAG (branch or hash) for cloning MADNESS")
-set(TA_TRACKED_MADNESS_PREVIOUS_TAG 8abd78b8a304a88b951449d8cb127f5a91f27721)
+set(TA_TRACKED_MADNESS_TAG f7aa1401e CACHE STRING "GIT_TAG (branch or hash) for cloning MADNESS")
+set(TA_TRACKED_MADNESS_PREVIOUS_TAG 7d8aaf9d51981e4accf4d84742270d1473f8ca2e)
 set(TA_TRACKED_MADNESS_VERSION 0.10.1)
 set(TA_TRACKED_MADNESS_PREVIOUS_VERSION 0.10.1)
 

src/CMakeLists.txt

@@ -343,6 +343,14 @@ add_library(tiledarray ${TILEDARRAY_SOURCE_FILES} ${TILEDARRAY_HEADER_FILES})
   target_link_libraries(${targetname} PUBLIC ${TILEDARRAY_PRIVATE_LINK_LIBRARIES})
   target_link_libraries(${targetname} PUBLIC MADworld)
   target_link_libraries(${targetname} PUBLIC Boost::headers)
+  # Pull compile options off the internal warnings INTERFACE target without
+  # creating a real link dependency on it — `tiledarray` is installed/exported,
+  # and a PRIVATE link would force the (intentionally non-installed)
+  # tiledarray_internal_warnings target into the same export set. The
+  # generator expression keeps these flags PRIVATE to TA's own TUs, so they
+  # do not propagate to downstream consumers via find_package(tiledarray).
+  target_compile_options(${targetname} PRIVATE
+      $<TARGET_PROPERTY:tiledarray_internal_warnings,INTERFACE_COMPILE_OPTIONS>)
 
   # build all external deps before building tiledarray
   add_dependencies(${targetname} External-tiledarray)

src/TiledArray/array_impl.h

@@ -813,10 +813,9 @@ class ArrayImpl : public TensorImpl<Policy>,
           if (fut.probe()) continue;
         }
         if constexpr (Exec == HostExecutor::MADWorld) {
-          Future<value_type> tile =
-              this->world().taskq.add([this_sptr = this->shared_from_this(),
-                                       index = ordinal_type(index),
-                                       op_shared_handle, this]() -> value_type {
+          Future<value_type> tile = this->world().taskq.add(
+              [this_sptr = this->shared_from_this(),
+               index = ordinal_type(index), op_shared_handle]() -> value_type {
                 return op_shared_handle(
                     this_sptr->trange().make_tile_range(index));
               });
@@ -954,7 +953,6 @@ std::shared_ptr<ArrayImpl<Tile, Policy>> make_with_new_trange(
   if constexpr (!is_dense_v<Policy>) {
     // each rank computes contributions to the shape norms from its local tiles
     Tensor<float> target_shape_norms(target_tiles_range, 0);
-    auto& source_trange = source_array.trange();
     const auto e = source_array.cend();
     for (auto it = source_array.cbegin(); it != e; ++it) {
       auto source_tile_idx = it.index();
@@ -995,7 +993,6 @@ std::shared_ptr<ArrayImpl<Tile, Policy>> make_with_new_trange(
 
   // loop over local tile and sends its contributions to the targets
   {
-    auto& source_trange = source_array.trange();
     const auto e = source_array.cend();
     auto& target_tiles_range = target_trange.tiles_range();
     for (auto it = source_array.cbegin(); it != e; ++it) {

src/TiledArray/einsum/string.h

@@ -20,13 +20,15 @@ std::pair<T, U> split2(const std::string& s, const std::string& d) {
 }
 
 // Split delimiter must match completely
-std::vector<std::string> split(const std::string& s, char d) {
+[[maybe_unused]] std::vector<std::string> split(const std::string& s, char d) {
   std::vector<std::string> res;
   return boost::split(res, s,
                       [&d](char c) { return c == d; } /*boost::is_any_of(d)*/);
 }
 
-std::string trim(const std::string& s) { return boost::trim_copy(s); }
+[[maybe_unused]] std::string trim(const std::string& s) {
+  return boost::trim_copy(s);
+}
 
 template <typename T>
 std::string str(const T& obj) {

src/TiledArray/expressions/binary_engine.h

@@ -147,10 +147,6 @@ class BinaryEngine : public ExprEngine<Derived> {
         TiledArray::detail::is_tensor_of_tensor_v<left_tile_type>;
     constexpr bool right_tile_is_tot =
         TiledArray::detail::is_tensor_of_tensor_v<right_tile_type>;
-    constexpr bool args_are_plain_tensors =
-        !left_tile_is_tot && !right_tile_is_tot;
-    constexpr bool args_are_mixed_tensors =
-        left_tile_is_tot ^ right_tile_is_tot;
     // implicit_permute_{outer,inner}() denotes whether permutations will be
     // fused into consuming operation
     if (left_outer_permtype_ == PermutationType::matrix_transpose ||

src/TiledArray/expressions/blk_tsr_expr.h

@@ -32,8 +32,8 @@
 #include <TiledArray/expressions/unary_expr.h>
 #include "blk_tsr_engine.h"
 
-#include <range/v3/algorithm.hpp>
-#include <range/v3/view.hpp>
+#include <range/v3/algorithm/any_of.hpp>
+#include <range/v3/view/zip.hpp>
 
 #include <optional>
 

src/TiledArray/expressions/cont_engine.h

@@ -640,13 +640,6 @@ class ContEngine : public BinaryEngine<Derived> {
                                                       right_tile_type> &&
             TiledArray::detail::is_tensor_v<left_tile_type>;
         if constexpr (tot_x_t || t_x_tot) {
-          using arg_tile_element_type =
-              std::conditional_t<tot_x_t, left_tile_element_type,
-                                 right_tile_element_type>;
-          using scalar_type =
-              std::conditional_t<tot_x_t, right_tile_element_type,
-                                 left_tile_element_type>;
-
           auto scal_op = [perm = !this->implicit_permute_inner_
                                      ? inner(this->perm_)
                                      : Permutation{}](

src/TiledArray/tensor/kernels.h

@@ -1276,8 +1276,9 @@ template <typename ResultTensorAllocator = void, typename TensorA,
                                       is_annotation_v<Annot>>>
 auto tensor_contract(TensorA const& A, Annot const& aA, TensorB const& B,
                      Annot const& aB, Annot const& aC) {
-  using Result = result_tensor_t<std::multiplies<>, TensorA, TensorB,
-                                 ResultTensorAllocator>;
+  using Result [[maybe_unused]] =
+      result_tensor_t<std::multiplies<>, TensorA, TensorB,
+                      ResultTensorAllocator>;
 
   TensorContractionPlan plan(aA, aB, aC);
 

src/TiledArray/util/initializer_list.h

@@ -212,9 +212,15 @@ auto flatten_il(T&& il, OutputItr out_itr) {
     ++out_itr;
   }
   // We were given a vector or we have recursed to the most nested
-  // initializer_list, either way copy the contents to the buffer
+  // initializer_list, either way copy the contents to the buffer.
+  // Guard against empty ranges: std::copy on a 0-length range with a
+  // possibly-null output iterator (e.g. std::array<T,0>::begin()) inlines
+  // to __builtin_memmove(null, null, 0), tripping gcc's -Wnonnull even
+  // though the copy itself is a no-op.
   else if constexpr (ranks_left == 1) {
-    out_itr = std::copy(il.begin(), il.end(), out_itr);
+    if (il.size() != 0) {
+      out_itr = std::copy(il.begin(), il.end(), out_itr);
+    }
   }
   // The initializer list is at least a matrix, so recurse over sub-lists
   else {

tests/btas.cpp

@@ -260,7 +260,7 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(tensor_ctor, Tensor, tensor_types) {
   // can copy TA::Tensor to btas::Tensor
   TA::Tensor<typename Tensor::value_type> ta_tensor;
   ta_tensor = make_rand_tile<decltype(ta_tensor)>(r);
-  BOOST_REQUIRE_NO_THROW(Tensor(ta_tensor));
+  BOOST_REQUIRE_NO_THROW((Tensor(ta_tensor)));
   Tensor t2(ta_tensor);
   for (auto i : r) {
     BOOST_CHECK_EQUAL(ta_tensor(i), t2(i));

tests/dist_array.cpp

@@ -63,6 +63,19 @@ std::string to_parallel_archive_file_name(const char* prefix_name, int rank) {
   snprintf(buf, sizeof(buf), "%s.%5.5d", prefix_name, rank);
   return buf;
 }
+
+// Replace the trailing XXXXXX in `name_template` with a unique suffix.
+// Uses mkstemp + close + remove so the resulting name can be reused by
+// callers that want to open it themselves (single-file archive) or use it
+// as a prefix for per-rank files (parallel archive). Unlike mktemp(3),
+// which clang/macOS flags as deprecated, this is race-free against other
+// in-process callers.
+void make_unique_filename_template(char* name_template) {
+  const int fd = mkstemp(name_template);
+  MADNESS_ASSERT(fd != -1);
+  ::close(fd);
+  std::remove(name_template);
+}
 }  // namespace
 
 BOOST_FIXTURE_TEST_SUITE(array_suite, ArrayFixture)
@@ -649,7 +662,7 @@ BOOST_AUTO_TEST_CASE(serialization_by_tile) {
     buf.reset();
   } else {  // ... else use TextFstreamOutputArchive
     char archive_file_name[] = "tmp.XXXXXX";
-    mktemp(archive_file_name);
+    make_unique_filename_template(archive_file_name);
     madness::archive::TextFstreamOutputArchive oar(archive_file_name);
 
     for (auto tile : a) {
@@ -676,7 +689,7 @@ BOOST_AUTO_TEST_CASE(serialization_by_tile) {
 
 BOOST_AUTO_TEST_CASE(dense_serialization) {
   char archive_file_name[] = "tmp.XXXXXX";
-  mktemp(archive_file_name);
+  make_unique_filename_template(archive_file_name);
   madness::archive::BinaryFstreamOutputArchive oar(archive_file_name);
   a.serialize(oar);
 
@@ -694,7 +707,7 @@ BOOST_AUTO_TEST_CASE(dense_serialization) {
 
 BOOST_AUTO_TEST_CASE(sparse_serialization) {
   char archive_file_name[] = "tmp.XXXXXX";
-  mktemp(archive_file_name);
+  make_unique_filename_template(archive_file_name);
   madness::archive::BinaryFstreamOutputArchive oar(archive_file_name);
   b.serialize(oar);
 
@@ -713,7 +726,7 @@ BOOST_AUTO_TEST_CASE(sparse_serialization) {
 BOOST_AUTO_TEST_CASE(parallel_serialization) {
   const int nio = 1;  // use 1 rank for I/O
   char archive_file_prefix_name[] = "tmp.XXXXXX";
-  mktemp(archive_file_prefix_name);
+  make_unique_filename_template(archive_file_prefix_name);
   madness::archive::ParallelOutputArchive<> oar(world, archive_file_prefix_name,
                                                 nio);
   oar & a;
@@ -737,7 +750,7 @@ BOOST_AUTO_TEST_CASE(parallel_serialization) {
 BOOST_AUTO_TEST_CASE(parallel_sparse_serialization) {
   const int nio = 1;  // use 1 rank for 1
   char archive_file_prefix_name[] = "tmp.XXXXXX";
-  mktemp(archive_file_prefix_name);
+  make_unique_filename_template(archive_file_prefix_name);
   madness::archive::ParallelOutputArchive<> oar(world, archive_file_prefix_name,
                                                 nio);
   oar & b;
@@ -773,7 +786,7 @@ BOOST_AUTO_TEST_CASE(issue_225) {
   S.fill(1.0);
 
   char archive_file_name[] = "tmp.XXXXXX";
-  mktemp(archive_file_name);
+  make_unique_filename_template(archive_file_name);
   madness::archive::BinaryFstreamOutputArchive oar(archive_file_name);
   St("i,j") = S("j,i");
   BOOST_REQUIRE_NO_THROW(oar & S);