From ee5586bcd89a6d0512563ae5b149b53147598383 Mon Sep 17 00:00:00 2001 From: Weiqun Zhang Date: Mon, 19 Sep 2022 19:37:46 -0700 Subject: [PATCH] Add more warnings * Add -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches to gcc. * Add -Wnon-virtual-dtor to clang. * Add more warnings to CI. * Fix some non-virtual dtors and some other warnings. --- .github/workflows/clang.yml | 4 ++-- .github/workflows/cuda.yml | 4 ++-- .github/workflows/gcc.yml | 16 ++++++++-------- .github/workflows/hip.yml | 4 ++-- .github/workflows/intel.yml | 2 +- .github/workflows/macos.yml | 4 ++-- Src/AmrCore/AMReX_ErrorList.H | 4 ++++ Src/EB/AMReX_distFcnElement.H | 6 +++--- Src/LinearSolvers/MLMG/AMReX_MLEBTensorOp.cpp | 12 ++++++------ .../MLMG/AMReX_MLNodeLaplacian_misc.cpp | 8 ++++++++ Tools/GNUMake/comps/gnu.mak | 10 +++++++--- Tools/GNUMake/comps/llvm.mak | 2 +- 12 files changed, 46 insertions(+), 30 deletions(-) diff --git a/.github/workflows/clang.yml b/.github/workflows/clang.yml index bdd629ce11f..79bbf1947b7 100644 --- a/.github/workflows/clang.yml +++ b/.github/workflows/clang.yml @@ -14,7 +14,7 @@ jobs: library_clang: name: Clang@6.0 C++14 SP NOMPI Debug [lib] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wno-c++17-extensions"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wno-c++17-extensions -Wnon-virtual-dtor"} steps: - uses: actions/checkout@v2 - name: Dependencies @@ -50,7 +50,7 @@ jobs: tests_clang: name: Clang@6.0 C++14 SP Particles DP Mesh Debug [tests] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wno-c++17-extensions -O1"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wno-c++17-extensions -O1 -Wnon-virtual-dtor"} # It's too slow with -O0 steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/cuda.yml b/.github/workflows/cuda.yml index c5fbceb5d7e..6e080d8a848 100644 --- a/.github/workflows/cuda.yml +++ b/.github/workflows/cuda.yml @@ -11,7 +11,7 @@ jobs: tests-cuda10: name: CUDA@10.2 GNU@6.5.0 C++14 Release [tests] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wunreachable-code"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wunreachable-code -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond"} steps: - uses: actions/checkout@v2 - name: Dependencies @@ -42,7 +42,7 @@ jobs: tests-cuda11: name: CUDA@11.2 GNU@9.3.0 C++17 Release [tests] runs-on: ubuntu-20.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches"} steps: - uses: actions/checkout@v2 - name: Dependencies diff --git a/.github/workflows/gcc.yml b/.github/workflows/gcc.yml index 188d7d32f95..5ee581b4fef 100644 --- a/.github/workflows/gcc.yml +++ b/.github/workflows/gcc.yml @@ -15,7 +15,7 @@ jobs: library: name: GNU@7.5 C++17 Release [lib] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches"} steps: - uses: actions/checkout@v2 - name: Dependencies @@ -43,7 +43,7 @@ jobs: tests_build_3D: name: GNU@7.5 C++14 3D Debug Fortran [tests] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -O1"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -O1 -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches"} # It's too slow with -O0 steps: - uses: actions/checkout@v2 @@ -66,7 +66,7 @@ jobs: tests_build_2D: name: GNU@7.5 C++14 2D Debug Fortran [tests] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -O1"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -O1 -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches"} # It's too slow with -O0 steps: - uses: actions/checkout@v2 @@ -89,7 +89,7 @@ jobs: tests_build_1D: name: GNU@7.5 C++14 1D Debug Fortran [tests] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -O1"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -O1 -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches"} # -Werror temporarily skipped until we have functional testing established # It's too slow with -O0 steps: @@ -114,7 +114,7 @@ jobs: tests_cxx20: name: GNU@10.1 C++20 OMP [tests] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches"} steps: - uses: actions/checkout@v2 - name: Dependencies @@ -147,7 +147,7 @@ jobs: tests-nonmpi: name: GNU@7.5 C++14 NOMPI [tests] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches"} steps: - uses: actions/checkout@v2 - name: Dependencies @@ -176,7 +176,7 @@ jobs: tests-nofortran: name: GNU@7.5 C++14 w/o Fortran [tests] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wunreachable-code"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wunreachable-code -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches"} steps: - uses: actions/checkout@v2 - name: Dependencies @@ -274,7 +274,7 @@ jobs: tests_run: name: GNU@7.5 C++14 [tests] runs-on: ubuntu-18.04 - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wunreachable-code"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wunreachable-code -Wnon-virtual-dtor -Wlogical-op -Wmisleading-indentation -Wduplicated-cond -Wduplicated-branches"} steps: - uses: actions/checkout@v2 - name: Dependencies diff --git a/.github/workflows/hip.yml b/.github/workflows/hip.yml index 47d9a89828e..a487d27bf9c 100644 --- a/.github/workflows/hip.yml +++ b/.github/workflows/hip.yml @@ -20,7 +20,7 @@ jobs: # ^ # /opt/rocm-4.1.1/hip/include/hip/hcc_detail/hip_runtime.h:176:9: note: macro 'select_impl_' defined here # #define select_impl_(_1, _2, impl_, ...) impl_ - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wno-deprecated-declarations -Wno-gnu-zero-variadic-macro-arguments"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wnon-virtual-dtor -Wno-deprecated-declarations -Wno-gnu-zero-variadic-macro-arguments"} steps: - uses: actions/checkout@v2 - name: Dependencies @@ -67,7 +67,7 @@ jobs: # ^ # /opt/rocm-4.1.1/hip/include/hip/hcc_detail/hip_runtime.h:176:9: note: macro 'select_impl_' defined here # #define select_impl_(_1, _2, impl_, ...) impl_ - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wno-deprecated-declarations -Wno-gnu-zero-variadic-macro-arguments"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wnon-virtual-dtor -Wno-deprecated-declarations -Wno-gnu-zero-variadic-macro-arguments"} steps: - uses: actions/checkout@v2 - name: Dependencies diff --git a/.github/workflows/intel.yml b/.github/workflows/intel.yml index 6fef4fc0459..80ae98cd2f1 100644 --- a/.github/workflows/intel.yml +++ b/.github/workflows/intel.yml @@ -11,7 +11,7 @@ jobs: name: DPCPP GFortran@7.5 C++17 [tests] runs-on: ubuntu-20.04 # mkl/rng/device/detail/mrg32k3a_impl.hpp has a number of sign-compare error - env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wno-sign-compare"} + env: {CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wnon-virtual-dtor -Wno-sign-compare"} steps: - uses: actions/checkout@v2 - name: Dependencies diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 61eb9b9ccdb..67db29cdcd8 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -14,7 +14,7 @@ jobs: env: # build universal binaries for M1 "Apple Silicon" and Intel CPUs CMAKE_OSX_ARCHITECTURES: "arm64;x86_64" - CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wno-c++17-extensions -Wno-range-loop-analysis" + CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wnon-virtual-dtor -Wno-c++17-extensions -Wno-range-loop-analysis" # -Wno-range-loop-analysis: Apple clang has a bug in range-loop-analysis steps: - uses: actions/checkout@v2 @@ -39,7 +39,7 @@ jobs: name: AppleClang@11.0 GFortran@9.3 [tests] runs-on: macos-latest env: - CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wno-c++17-extensions -Wno-range-loop-analysis" + CXXFLAGS: "-fno-operator-names -Werror -Wall -Wextra -Wpedantic -Wnull-dereference -Wfloat-conversion -Wshadow -Woverloaded-virtual -Wextra-semi -Wunreachable-code -Wnon-virtual-dtor -Wno-c++17-extensions -Wno-range-loop-analysis" # -Wno-range-loop-analysis: Apple clang has a bug in range-loop-analysis steps: - uses: actions/checkout@v2 diff --git a/Src/AmrCore/AMReX_ErrorList.H b/Src/AmrCore/AMReX_ErrorList.H index 8cf67ea5567..1cc8d61fd07 100644 --- a/Src/AmrCore/AMReX_ErrorList.H +++ b/Src/AmrCore/AMReX_ErrorList.H @@ -420,6 +420,8 @@ std::ostream& operator << (std::ostream& os, const ErrorList& elst); struct UserFunc { + virtual ~UserFunc () {} + virtual void operator() (const amrex::Box& bx, amrex::Array4 const& dat, amrex::Array4 const& tag, @@ -470,6 +472,8 @@ std::ostream& operator << (std::ostream& os, const ErrorList& elst); const AMRErrorTagInfo& info = AMRErrorTagInfo()) noexcept : m_userfunc(userfunc), m_field(field), m_info(info), m_ngrow(ngrow) {} + virtual ~AMRErrorTag () {} + virtual void operator() (amrex::TagBoxArray& tb, const amrex::MultiFab* mf, char clearval, diff --git a/Src/EB/AMReX_distFcnElement.H b/Src/EB/AMReX_distFcnElement.H index f839bdb5747..2a9c7a0c2f4 100644 --- a/Src/EB/AMReX_distFcnElement.H +++ b/Src/EB/AMReX_distFcnElement.H @@ -12,7 +12,7 @@ class distFcnElement2d { public: //! Constructor distFcnElement2d() {} - ~distFcnElement2d() {} + virtual ~distFcnElement2d() {} virtual distFcnElement2d* newDistFcnElement2d() const = 0; @@ -29,7 +29,7 @@ class distFcnElement2d { class LineDistFcnElement2d: public distFcnElement2d { public: LineDistFcnElement2d() {} - ~LineDistFcnElement2d() {} + virtual ~LineDistFcnElement2d() {} virtual distFcnElement2d* newDistFcnElement2d() const override; @@ -58,7 +58,7 @@ class LineDistFcnElement2d: public distFcnElement2d { class SplineDistFcnElement2d: public distFcnElement2d { public: SplineDistFcnElement2d() {} - ~SplineDistFcnElement2d() {} + virtual ~SplineDistFcnElement2d() {} virtual distFcnElement2d* newDistFcnElement2d() const override; diff --git a/Src/LinearSolvers/MLMG/AMReX_MLEBTensorOp.cpp b/Src/LinearSolvers/MLMG/AMReX_MLEBTensorOp.cpp index 247e0fb292e..590e062a3a1 100644 --- a/Src/LinearSolvers/MLMG/AMReX_MLEBTensorOp.cpp +++ b/Src/LinearSolvers/MLMG/AMReX_MLEBTensorOp.cpp @@ -598,12 +598,12 @@ MLEBTensorOp::compVelGrad (int amrlev, const Array& fl } - else if ( loc==Location::FaceCenter ) - { - - amrex::Abort("compVelGrad not yet implemented for cut-cells "); - - } +// else if ( loc==Location::FaceCenter ) +// { +// +// amrex::Abort("compVelGrad not yet implemented for cut-cells "); +// +// } else // loc==Location::FaceCentroid { diff --git a/Src/LinearSolvers/MLMG/AMReX_MLNodeLaplacian_misc.cpp b/Src/LinearSolvers/MLMG/AMReX_MLNodeLaplacian_misc.cpp index df5ab489d2f..339ca98e072 100644 --- a/Src/LinearSolvers/MLMG/AMReX_MLNodeLaplacian_misc.cpp +++ b/Src/LinearSolvers/MLMG/AMReX_MLNodeLaplacian_misc.cpp @@ -26,7 +26,11 @@ MLNodeLaplacian::averageDownCoeffs () { for (int mglev = 0; mglev < m_num_mg_levels[amrlev]; ++mglev) { +#if (AMREX_SPACEDIM == 1) + int ndims = 1; +#else int ndims = (m_use_harmonic_average || m_use_mapped) ? AMREX_SPACEDIM : 1; +#endif for (int idim = 0; idim < ndims; ++idim) { if (m_sigma[amrlev][mglev][idim] == nullptr) { @@ -101,7 +105,11 @@ MLNodeLaplacian::averageDownCoeffsSameAmrLevel (int amrlev) if (m_coarsening_strategy != CoarseningStrategy::Sigma) return; +#if (AMREX_SPACEDIM == 1) + const int nsigma = 1; +#else const int nsigma = (m_use_harmonic_average || m_use_mapped) ? AMREX_SPACEDIM : 1; +#endif for (int mglev = 1; mglev < m_num_mg_levels[amrlev]; ++mglev) { diff --git a/Tools/GNUMake/comps/gnu.mak b/Tools/GNUMake/comps/gnu.mak index 10510f30a8d..5e621eb140e 100644 --- a/Tools/GNUMake/comps/gnu.mak +++ b/Tools/GNUMake/comps/gnu.mak @@ -97,7 +97,7 @@ else endif ifeq ($(WARN_ALL),TRUE) - warning_flags = -Wall -Wextra + warning_flags = -Wall -Wextra -Wlogical-op ifeq ($(WARN_SIGN_COMPARE),FALSE) warning_flags += -Wno-sign-compare @@ -109,7 +109,7 @@ ifeq ($(WARN_ALL),TRUE) endif ifeq ($(gcc_major_ge_6),1) - warning_flags += -Wnull-dereference + warning_flags += -Wnull-dereference -Wmisleading-indentation -Wduplicated-cond endif ifeq ($(gcc_major_ge_5),1) @@ -124,11 +124,15 @@ ifeq ($(WARN_ALL),TRUE) warning_flags += -Wno-array-bounds endif + ifeq ($(gcc_major_ge7),1) + warning_flags += -Wduplicated-branches + endif + ifeq ($(gcc_major_ge10),1) warning_flags += -Wextra-semi endif - CXXFLAGS += $(warning_flags) -Woverloaded-virtual + CXXFLAGS += $(warning_flags) -Woverloaded-virtual -Wnon-virtual-dtor CFLAGS += $(warning_flags) endif diff --git a/Tools/GNUMake/comps/llvm.mak b/Tools/GNUMake/comps/llvm.mak index ad516e0799d..86da5884b7f 100644 --- a/Tools/GNUMake/comps/llvm.mak +++ b/Tools/GNUMake/comps/llvm.mak @@ -50,7 +50,7 @@ ifeq ($(WARN_ALL),TRUE) warning_flags += -Wshadow endif - CXXFLAGS += $(warning_flags) -Woverloaded-virtual + CXXFLAGS += $(warning_flags) -Woverloaded-virtual -Wnon-virtual-dtor CFLAGS += $(warning_flags) endif