From 5b90563c7802b4679ab4caea7e7cba21c5fb66f8 Mon Sep 17 00:00:00 2001 From: Jonah Miller Date: Fri, 29 Nov 2024 13:51:02 -0700 Subject: [PATCH] add check for warnings on gcc as well as clang --- .github/workflows/warnings.yml | 37 ++++++++++++++ CMakeLists.txt | 2 +- .../base/root-finding-1d/root_finding.hpp | 50 +++++++++---------- singularity-eos/eos/eos_base.hpp | 7 +-- singularity-eos/eos/get_sg_eos_p_t.cpp | 2 +- singularity-eos/eos/get_sg_eos_rho_e.cpp | 2 +- singularity-eos/eos/get_sg_eos_rho_p.cpp | 2 +- singularity-eos/eos/get_sg_eos_rho_t.cpp | 2 +- .../eos/modifiers/eos_unitsystem.hpp | 3 ++ 9 files changed, 74 insertions(+), 33 deletions(-) create mode 100644 .github/workflows/warnings.yml diff --git a/.github/workflows/warnings.yml b/.github/workflows/warnings.yml new file mode 100644 index 0000000000..98316b7ccf --- /dev/null +++ b/.github/workflows/warnings.yml @@ -0,0 +1,37 @@ +name: Warnings + +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + +jobs: + warnings-gcc: + name: Ensure no warnings from gcc + runs-on: ubuntu-latest + + steps: + - name: Checkout code + uses: actions/checkout@v3 + with: + submodules: recursive + - name: Set system to non-interactive mode + run: export DEBIAN_FRONTEND=noninteractive + - name: install dependencies + run: | + sudo apt-get update -y -qq + sudo apt-get install -y --allow-downgrades --allow-remove-essential --allow-change-held-packages -qq build-essential + - name: build and run tests + run: | + mkdir -p bin + cd bin + cmake -DCMAKE_BUILD_TYPE=Debug \ + -DSINGULARITY_STRICT_WARNINGS=ON \ + -DSINGULARITY_USE_FORTRAN=OFF \ + -DSINGULARITY_BUILD_FORTRAN_BACKEND=ON \ + -DSINGULARITY_BUILD_TESTS=ON \ + -DSINGULARITY_FORCE_SUBMODULE_MODE=ON \ + -DSINGULARITY_USE_KOKKOS=ON \ + .. + make -j4 diff --git a/CMakeLists.txt b/CMakeLists.txt index fbfa81e865..ab5d62bd02 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -563,7 +563,7 @@ target_compile_options( ) if (SINGULARITY_STRICT_WARNINGS) target_compile_options(singularity-eos_Interface INTERFACE - -Wall -Werror) + -Wall -Werror -Wno-unknown-pragmas) endif() if(TARGET singularity-eos_Library) diff --git a/singularity-eos/base/root-finding-1d/root_finding.hpp b/singularity-eos/base/root-finding-1d/root_finding.hpp index e62615a7d1..7305141475 100644 --- a/singularity-eos/base/root-finding-1d/root_finding.hpp +++ b/singularity-eos/base/root-finding-1d/root_finding.hpp @@ -70,20 +70,20 @@ enum class Status { SUCCESS = 0, FAIL = 1 }; */ class RootCounts { private: - static constexpr int nbins_{15}; + static constexpr std::size_t nbins_{15}; mutable Real counts_[nbins_]; public: PORTABLE_INLINE_FUNCTION RootCounts() { - for (int i{0}; i < nbins_; ++i) + for (std::size_t i{0}; i < nbins_; ++i) counts_[i] = 0; } PORTABLE_INLINE_FUNCTION void reset() { - for (int i{0}; i < nbins_; ++i) + for (std::size_t i{0}; i < nbins_; ++i) counts_[i] = 0; } - PORTABLE_INLINE_FUNCTION void increment(int i) const { + PORTABLE_INLINE_FUNCTION void increment(std::size_t i) const { assert(i < nbins_ && i >= 0); #ifdef PORTABILITY_STRATEGY_NONE counts_[i] += 1; @@ -91,24 +91,24 @@ class RootCounts { } PORTABLE_INLINE_FUNCTION Real total() const { Real tot{1.e-20}; - for (int i{0}; i < nbins_; ++i) + for (std::size_t i{0}; i < nbins_; ++i) tot += counts_[i]; return tot; } - PORTABLE_INLINE_FUNCTION const Real &operator[](const int i) const { + PORTABLE_INLINE_FUNCTION const Real &operator[](const std::size_t i) const { assert(i < nbins_ && i >= 0); return counts_[i]; } - PORTABLE_INLINE_FUNCTION Real &operator[](const int i) { + PORTABLE_INLINE_FUNCTION Real &operator[](const std::size_t i) { assert(i < nbins_ && i >= 0); return counts_[i]; } PORTABLE_INLINE_FUNCTION void print_counts() const { - for (int i{0}; i < nbins_; ++i) + for (std::size_t i{0}; i < nbins_; ++i) printf("%e\n", counts_[i]); } - PORTABLE_INLINE_FUNCTION int nBins() const { return nbins_; } - PORTABLE_INLINE_FUNCTION int more() const { return nbins_ - 1; } + PORTABLE_INLINE_FUNCTION std::size_t nBins() const { return nbins_; } + PORTABLE_INLINE_FUNCTION std::size_t more() const { return nbins_ - 1; } }; PORTABLE_INLINE_FUNCTION bool check_bracket(const Real ya, const Real yb) { @@ -119,11 +119,11 @@ template PORTABLE_INLINE_FUNCTION bool set_bracket(const T &f, Real &a, const Real guess, Real &b, Real &ya, const Real yg, Real &yb, const bool &verbose = false) { - constexpr int max_search_depth = 6; + constexpr std::size_t max_search_depth = 6; Real dx = b - a; - for (int level = 0; level < max_search_depth; level++) { - const int nlev = (1 << level); - for (int i = 0; i < nlev; i++) { + for (std::size_t level = 0; level < max_search_depth; level++) { + const std::size_t nlev = (1 << level); + for (std::size_t i = 0; i < nlev; i++) { const Real x = a + (i + 0.5) * dx; const Real yx = f(x); if (check_bracket(yx, yg)) { @@ -158,7 +158,7 @@ PORTABLE_INLINE_FUNCTION Status regula_falsi(const T &f, const Real ytarget, Real &xroot, const RootCounts *counts = nullptr, const bool &verbose = false) { - constexpr int max_iter = SECANT_NITER_MAX; + constexpr std::size_t max_iter = SECANT_NITER_MAX; auto func = [&](const Real x) { return f(x) - ytarget; }; Real ya = func(a); Real yg = func(guess); @@ -187,9 +187,9 @@ PORTABLE_INLINE_FUNCTION Status regula_falsi(const T &f, const Real ytarget, ya *= sign; yb *= sign; - int b1 = 0; - int b2 = 0; - int iteration_count = 0; + std::size_t b1 = 0; + std::size_t b2 = 0; + std::size_t iteration_count = 0; while (b - a > 2.0 * xtol && (std::abs(ya) > ytol || std::abs(yb) > ytol) && iteration_count < max_iter) { Real c = (a * yb - b * ya) / (yb - ya); @@ -251,7 +251,7 @@ PORTABLE_INLINE_FUNCTION Status newton_raphson(const T &f, const Real ytarget, const bool &verbose = false, const bool &fail_on_bound_root = true) { - constexpr int max_iter = NEWTON_RAPHSON_NITER_MAX; + constexpr std::size_t max_iter = NEWTON_RAPHSON_NITER_MAX; Real _x = guess; Real _xold = 0.0; auto status = Status::SUCCESS; @@ -259,7 +259,7 @@ PORTABLE_INLINE_FUNCTION Status newton_raphson(const T &f, const Real ytarget, Real yg; Real dfunc; - int iter; + std::size_t iter; for (iter = 0; iter < max_iter; iter++) { std::tie(yg, dfunc) = f(_x); // C++11 tuple unpacking @@ -383,7 +383,7 @@ PORTABLE_INLINE_FUNCTION Status secant(const T &f, const Real ytarget, const Rea Real x_last, y, yp, ym, dyNum, dyDen, dy; Real x = xguess; - unsigned int iter{0}; + std::size_t iter{0}; for (iter = 0; iter < SECANT_NITER_MAX; ++iter) { x_last = x; dx = fabs(1.e-7 * x) + xtol; @@ -490,7 +490,7 @@ PORTABLE_INLINE_FUNCTION Status bisect(const T &f, const Real ytarget, const Rea x += 2. * xtol; } // do { // Try to find reasonable region for bisection - for (int i{0}; i < BISECT_REG_MAX; ++i) { + for (std::size_t i{0}; i < BISECT_REG_MAX; ++i) { dx = fabs(grow * x); xl = x - dx; xr = x + dx; @@ -547,10 +547,10 @@ PORTABLE_INLINE_FUNCTION Status bisect(const T &f, const Real ytarget, const Rea "\til = %.10e\n" "\tir = %.10e\n", xguess, ytarget, xl, xr, fl, fr, il, ir); - int nx = 300; + std::size_t nx = 300; Real dx = (xmax - xmin) / (nx - 1); fprintf(stderr, "Area map:\nx\ty\n"); - for (int i = 0; i < nx; i++) { + for (std::size_t i = 0; i < nx; i++) { fprintf(stderr, "%.4f\t%.4e\n", x + i * dx, f(x + i * dx)); } #endif @@ -559,7 +559,7 @@ PORTABLE_INLINE_FUNCTION Status bisect(const T &f, const Real ytarget, const Rea } } - for (int i{0}; i < BISECT_NITER_MAX; ++i) { + for (std::size_t i{0}; i < BISECT_NITER_MAX; ++i) { Real xm = 0.5 * (xl + xr); Real fm = f(xm) - ytarget; if (fl * fm <= 0) { diff --git a/singularity-eos/eos/eos_base.hpp b/singularity-eos/eos/eos_base.hpp index 6e838d108d..83d0bf03a9 100644 --- a/singularity-eos/eos/eos_base.hpp +++ b/singularity-eos/eos/eos_base.hpp @@ -41,7 +41,7 @@ constexpr std::size_t MAX_NUM_CHARS = 121; // Cuda doesn't have strcat, so we implement it ourselves PORTABLE_FORCEINLINE_FUNCTION char *StrCat(char *destination, const char *source) { - int i, j; // not in loops because they're re-used. + std::size_t i, j; // not in loops because they're re-used. // specifically avoid strlen, which isn't on GPU for (i = 0; destination[i] != '\0'; i++) { @@ -49,9 +49,10 @@ char *StrCat(char *destination, const char *source) { // assumes destination has enough memory allocated for (j = 0; source[j] != '\0'; j++) { // MAX_NUM_CHARS-1 to leave room for null terminator - PORTABLE_REQUIRE((i + j) < MAX_NUM_CHARS - 1, + std::size_t ipj = i + j; + PORTABLE_REQUIRE(ipj < MAX_NUM_CHARS - 1, "Concat string must be within allowed size"); - destination[i + j] = source[j]; + destination[ipj] = source[j]; } // null terminate destination string destination[i + j] = '\0'; diff --git a/singularity-eos/eos/get_sg_eos_p_t.cpp b/singularity-eos/eos/get_sg_eos_p_t.cpp index befe6ff0a7..8b0857edf5 100644 --- a/singularity-eos/eos/get_sg_eos_p_t.cpp +++ b/singularity-eos/eos/get_sg_eos_p_t.cpp @@ -40,7 +40,7 @@ void get_sg_eos_p_t(const char *name, int ncell, int nmat, indirection_v &offset const int32_t token{tokens.acquire()}; const int32_t tid{small_loop ? iloop : token}; // need to initialize the scratch before it's used to avoid undefined behavior - for (int idx = 0; idx < solver_scratch.extent(1); ++idx) { + for (std::size_t idx = 0; idx < solver_scratch.extent(1); ++idx) { solver_scratch(tid, idx) = 0.0; } // caching mechanism diff --git a/singularity-eos/eos/get_sg_eos_rho_e.cpp b/singularity-eos/eos/get_sg_eos_rho_e.cpp index 8501c4cde8..29a6516caf 100644 --- a/singularity-eos/eos/get_sg_eos_rho_e.cpp +++ b/singularity-eos/eos/get_sg_eos_rho_e.cpp @@ -41,7 +41,7 @@ void get_sg_eos_rho_e(const char *name, int ncell, indirection_v &offsets_v, // initialize values for solver / lookup i_func(i, tid, mass_sum, npte, vfrac_sum, 0.0, 1.0, 0.0); // need to initialize the scratch before it's used to avoid undefined behavior - for (int idx = 0; idx < solver_scratch.extent(1); ++idx) { + for (std::size_t idx = 0; idx < solver_scratch.extent(1); ++idx) { solver_scratch(tid, idx) = 0.0; } // get cache from offsets into scratch diff --git a/singularity-eos/eos/get_sg_eos_rho_p.cpp b/singularity-eos/eos/get_sg_eos_rho_p.cpp index a6c5a8ea4a..65400ae7b9 100644 --- a/singularity-eos/eos/get_sg_eos_rho_p.cpp +++ b/singularity-eos/eos/get_sg_eos_rho_p.cpp @@ -43,7 +43,7 @@ void get_sg_eos_rho_p(const char *name, int ncell, indirection_v &offsets_v, i_func(i, tid, mass_sum, npte, vfrac_sum, 0.0, 0.0, 1.0); Real sie_tot_true{0.0}; // need to initialize the scratch before it's used to avoid undefined behavior - for (int idx = 0; idx < solver_scratch.extent(1); ++idx) { + for (std::size_t idx = 0; idx < solver_scratch.extent(1); ++idx) { solver_scratch(tid, idx) = 0.0; } const int neq = npte + 1; diff --git a/singularity-eos/eos/get_sg_eos_rho_t.cpp b/singularity-eos/eos/get_sg_eos_rho_t.cpp index 9735a4c1a1..692e76b409 100644 --- a/singularity-eos/eos/get_sg_eos_rho_t.cpp +++ b/singularity-eos/eos/get_sg_eos_rho_t.cpp @@ -44,7 +44,7 @@ void get_sg_eos_rho_t(const char *name, int ncell, indirection_v &offsets_v, // calculate pte condition (lookup for 1 mat cell) Real sie_tot_true{0.0}; // need to initialize the scratch before it's used to avoid undefined behavior - for (int idx = 0; idx < solver_scratch.extent(1); ++idx) { + for (std::size_t idx = 0; idx < solver_scratch.extent(1); ++idx) { solver_scratch(tid, idx) = 0.0; } const int neq = npte; diff --git a/singularity-eos/eos/modifiers/eos_unitsystem.hpp b/singularity-eos/eos/modifiers/eos_unitsystem.hpp index cb98a373ae..e69092bf8c 100644 --- a/singularity-eos/eos/modifiers/eos_unitsystem.hpp +++ b/singularity-eos/eos/modifiers/eos_unitsystem.hpp @@ -34,12 +34,15 @@ namespace singularity { using namespace eos_base; // tag dispatch for constructors for UnitSystem +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunused-variable" namespace eos_units_init { static struct ThermalUnitsInit { } thermal_units_init_tag; static struct LengthTimeUnitsInit { } length_time_units_init_tag; } // namespace eos_units_init +#pragma GCC diagnostic pop template class UnitSystem : public EosBase> {