build: enable -Werror on TA translation units (opt-in via TA_WERROR)#544
Open
evaleev wants to merge 7 commits into
Open
build: enable -Werror on TA translation units (opt-in via TA_WERROR)#544evaleev wants to merge 7 commits into
evaleev wants to merge 7 commits into
Conversation
New TA_WERROR CMake option (default OFF). When ON, an INTERFACE target tiledarray_internal_warnings carries -Werror (CXX/C/HIP) and -Xcompiler=-Werror (CUDA), applied PRIVATE-ly to the tiledarray library and to in-tree executables via add_ta_executable. Compile options are pulled off the INTERFACE target via $<TARGET_PROPERTY:...> rather than a real link, so the (non-installed) warnings target does not get pulled into the tiledarray export set. -Werror does not propagate to consumers of the installed tiledarray target (verified: tiledarray-targets.cmake contains zero Werror occurrences). Scope excludes bundled FetchContent code (parsec, btas, umpire, range-v3, lapackpp/blaspp) and python-tiledarray (pybind11_add_module). TA_WERROR=ON implies MADNESS_WERROR=ON so the MADworld TUs built as part of TA's FetchContent tree are covered too; requires a MADNESS pin that includes the MADNESS_WERROR option, so bump TA_TRACKED_MADNESS_TAG to PR m-a-d-n-e-s-s/madness#693 head (7d8aaf9d5). Treat range-v3 as a SYSTEM include (header-only, no ordering risk with TA headers): range-v3 self-triggers -Wdeprecated-declarations from compressed_pair.hpp using its own deprecated ranges::compressed_tuple alias. Carved out specifically here despite the top-level CMAKE_NO_SYSTEM_FROM_IMPORTED=TRUE.
Cleanup of the warning surface on master so TA_WERROR=ON builds cleanly.
src/TiledArray:
- Drop unused locals, type aliases, and lambda captures
(-Wunused-{variable,local-typedef,lambda-capture}).
- einsum/string.h, tensor/kernels.h: mark header-defined free functions
/ type aliases [[maybe_unused]] for TUs that include but do not use
them.
- blk_tsr_expr.h: replace umbrella range-v3 includes with the narrower
zip.hpp / any_of.hpp headers actually used.
tests:
- dist_array.cpp, tot_dist_array_part2.cpp: replace 8 deprecated
mktemp(3) calls (-Wdeprecated-declarations on macOS) with a
mkstemp+close+remove helper that produces a unique filename
race-free.
- btas.cpp: parenthesize BOOST_REQUIRE_NO_THROW(Tensor(ta_tensor)) to
dodge -Wvexing-parse.
GHA: 8 cells (macOS/Ubuntu * Pthreads/PaRSEC * Release/Debug). GitLab: CUDA-capable runner.
GitLab CI (gcc-14, CUDA cell) failed compiling examples/demo with TA_WERROR=ON because btas/generic/converge_class.h trips -Wreturn-type on two non-void members lacking a return statement — upstream BTAS bug TA can't fix. Mirror the range-v3 carve-out: set INTERFACE_SYSTEM_INCLUDE_DIRECTORIES on the BTAS target so its headers are consumed via -isystem, taking them out of -Werror scope. Resolve through ALIASED_TARGET first so it works whether BTAS::BTAS is the imported target or an in-tree alias.
…sion m-a-d-n-e-s-s/madness@f7aa1401e silences the std::atomic_exchange(shared_ptr<T>*, ...) deprecation that breaks gcc/libstdc++ builds with TA_WERROR=ON (MADNESS_WERROR=ON).
gcc-13/-14 inlines std::copy on a 0-length range to __builtin_memmove(out, in, 0) and trips -Wnonnull when both pointers are null (e.g. std::array<T, 0>::begin()), which is what the empty_vector unit test exercises. The std::copy itself is a well- defined no-op on empty ranges, but the optimizer can't see past the inlined memmove's nonnull attribute. Short-circuit empty ranges so std::copy is not called at all in that case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add an opt-in
TA_WERRORCMake option that promotes compiler warningsto errors on TiledArray's own translation units (library, tests,
examples), default OFF, not propagated to consumers of the installed
tiledarraytarget. Wire it on in both CIs.Modeled after the MADNESS counterpart, m-a-d-n-e-s-s/madness#693.
What changes
CMake — opt-in
TA_WERROR, default OFF.cmake/modules/TiledArrayWarnings.cmakeowns anINTERFACElibrary
tiledarray_internal_warnings. WithTA_WERROR=ONitcarries
-Werror(CXX/C/HIP) and-Xcompiler=-Werror(CUDA), gatedon GNU/Clang/AppleClang/IntelLLVM (warns otherwise).
src/CMakeLists.txt: pulls compile options off the warnings targetvia
$<TARGET_PROPERTY:tiledarray_internal_warnings,INTERFACE_COMPILE_OPTIONS>rather than a real link, so the (intentionally non-installed)
warnings target does not get pulled into the
tiledarrayexportset.
add_ta_executable: PRIVATE-links the warnings target to everyin-tree executable, covering
ta_testand allexamples/.examples/scalapack/CMakeLists.txt: same wiring for the one rawadd_executable.Verified by inspecting the generated
tiledarray-targets.cmake: zeroWerroroccurrences,tiledarray_internal_warningsabsent from everyINTERFACE_LINK_LIBRARIES.TA_WERROR=ONimpliesMADNESS_WERROR=ONviaFindOrFetchMADWorld.cmake, so MADworld TUs built as part of TA'sFetchContent tree are covered. This requires a MADNESS pin that
includes the
MADNESS_WERRORoption, soTA_TRACKED_MADNESS_TAGisbumped to the head of m-a-d-n-e-s-s/madness#693 (
7d8aaf9d5); thisPR is blocked on that one landing.
Scope of
-Werrorintentionally excludes:lapackpp) — those targets are not built via
add_ta_*.python-tiledarray— pybind11-generated code, built viapybind11_add_module.range-v3 marked as SYSTEM include. range-v3 self-triggers
-Wdeprecated-declarationsfromcompressed_pair.hppusing its owndeprecated
ranges::compressed_tuplealias. Header-only and noinclude-order risk against TA's headers, so it's carved out from the
top-level
CMAKE_NO_SYSTEM_FROM_IMPORTED=TRUE.Source-level warning fixes:
array_impl.h,binary_engine.h,cont_engine.h(
-Wunused-{variable,local-typedef,lambda-capture}).einsum/string.hand
tensor/kernels.h[[maybe_unused]].range/v3/{algorithm,view}.hppincludes inblk_tsr_expr.hwith the narrowerzip.hpp/any_of.hppactually used.tests/dist_array.cpp,tests/tot_dist_array_part2.cpp: replace 8deprecated
mktemp(3)calls (-Wdeprecated-declarationson macOS)with an
mkstemp+close+removehelper that produces a unique filenamerace-free.
tests/btas.cpp: parenthesizeBOOST_REQUIRE_NO_THROW(Tensor(ta_tensor))to dodge
-Wvexing-parse.CI —
-DTA_WERROR=ONeverywhere.Test plan
ScaLAPACK/CUDA):
tiledarray,ta_test,examples-tiledarrayall build clean with
-DTA_WERROR=ON.tiledarray-targets.cmake: zeroWerror, notiledarray_internal_warningsreferences.-Werrorlands on TA src/tests/examples and MADworld; absentfrom parsec/btas/blaspp/lapackpp/umpire/range-v3.
FetchContent)does not inherit
-Werror.Blocking dependency
This PR pins MADNESS to the head of m-a-d-n-e-s-s/madness#693 so that
MADNESS_WERROR=ONpropagation actually does something. Merge order:land #693 first, then bump this PR's pin to the merge SHA before
merging here.