Add more warnings (AMReX-Codes#2956)

* 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.
Thierry992 · Sep 20, 2022 · a6e0c11 · a6e0c11
1 parent 826cd37
commit a6e0c11
Show file tree

Hide file tree

Showing 12 changed files with 46 additions and 30 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 amrex::Real> const& dat,
                                amrex::Array4<char> 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
@@ -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
@@ -598,12 +598,12 @@ MLEBTensorOp::compVelGrad (int amrlev, const Array<MultiFab*,AMREX_SPACEDIM>& 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
@@ -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
@@ -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
@@ -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