Updated clang -Weverything checks to clang-7
authorMark Abraham <mark.j.abraham@gmail.com>
Wed, 17 Apr 2019 12:31:44 +0000 (14:31 +0200)
committerPaul Bauer <paul.bauer.q@gmail.com>
Tue, 23 Apr 2019 14:31:09 +0000 (16:31 +0200)
This is supposed to be used with the latest tested clang
compiler. That is now clang 7, but the logic is hard-coded to clang 6,
which is why some things only failed in post-submit testing where we
have such a compiler for the clang-cuda build.

Fixed issues only found with clang -Weverything.

Also fixed those found with clang-8, but as we do not yet use that in
Jenkins, they're commented out for now.

Change-Id: I83be1cb770d486cc81a453d4911361f06a056ce3

src/CMakeLists.txt
src/gromacs/CMakeLists.txt
src/gromacs/gmxana/gmx_dielectric.cpp
src/gromacs/gmxana/gmx_dipoles.cpp
src/gromacs/mdrun/integrator.h
src/gromacs/mdrun/minimize.cpp
src/gromacs/nbnxm/kernel_file_generator/kernel_simd_template.h.pre
src/gromacs/nbnxm/kernels_simd_2xmm/kernels.h
src/gromacs/nbnxm/kernels_simd_4xm/kernels.h

index 64fae1cc31a9c8a9ab2a497d38ee9cf40ac49c3d..d65461a98991966c7e5cca326d2c615377ad4a36 100644 (file)
@@ -60,6 +60,19 @@ set(IGNORED_CLANG_ALL_WARNINGS
     "-Wno-covered-switch-default" #GCC gives maybe-uninitialized without default label and checks for illegal enum values.
     "-Wno-switch-enum" # default statement for enum is OK
 
+    # TODO uncomment the next few ignore lines when we upgrade to test with clang 8 in Jenkins
+
+    # The barriers we use for tMPI and Nbnxm are sufficient, but it's
+    # not known whether they're excessive. We assume they not
+    # excessive.
+    # "-Wno-atomic-implicit-seq-cst"
+
+    # We need to use macros like
+    # GMX_CATCH_ALL_AND_EXIT_WITH_FATAL_ERROR. Those will look strange
+    # if they don't have a semicolon after them, and might confuse
+    # tools like IDEs also.
+    # "-Wno-extra-semi-stmt"
+
     #Following ones are undecided/TODO
     "-Wno-disabled-macro-expansion"
     "-Wno-cast-align"
index 135f6608c7709ad3f4034c7e78d09af7a36dca9f..15ae002aa74bcff3214dc00fa53af5fa061bdeb4 100644 (file)
@@ -332,7 +332,10 @@ set_target_properties(libgromacs PROPERTIES
 gmx_manage_lmfit()
 target_link_libraries(libgromacs PRIVATE lmfit)
 
-if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND CMAKE_CXX_COMPILER_VERSION MATCHES "^6\.0")
+# Fix everything found by the latest version of clang that we use in
+# Jenkins testing. This should be updated when we update the latest
+# tested version of clang.
+if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" AND CMAKE_CXX_COMPILER_VERSION MATCHES "^7\.0")
    target_compile_options(libgromacs PRIVATE $<$<COMPILE_LANGUAGE:CXX>:-Weverything ${IGNORED_CLANG_ALL_WARNINGS}>)
 endif()
 if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
index df896db69e6d1cc6760b740d1afa4e591556592a..e7d350e7bc8d27129c01aedb013b4098c77c461a 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2013,2014,2015,2016,2017,2018, by the GROMACS development team, led by
+ * Copyright (c) 2013,2014,2015,2016,2017,2018,2019, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -66,7 +66,7 @@ static int calc_nbegin(int nx, real x[], real tbegin)
     /* Assume input x is sorted */
     for (nbegin = 0; (nbegin < nx) && (x[nbegin] <= tbegin); nbegin++)
     {
-        ;
+        // Deliberately left empty
     }
     if ((nbegin == nx) || (nbegin == 0))
     {
index de453d4a292bd7d781d61413b18f8646cf2bd6a9..984d9287ce7a9ee1af1ee49996ea3255a29d305d 100644 (file)
@@ -502,7 +502,7 @@ static void mol_quad(int k0, int k1, rvec x[], const t_atom atom[], rvec quad)
     rvec     com;        /* center of mass */
     rvec     r;          /* distance of atoms to center of mass */
     double **inten;
-    double   dd[DIM], **ev, tmp;
+    double   dd[DIM], **ev;
 
     snew(inten, DIM);
     snew(ev, DIM);
@@ -586,15 +586,18 @@ static void mol_quad(int k0, int k1, rvec x[], const t_atom atom[], rvec quad)
      * At the moment I have no idea how this will work out for other molecules...
      */
 
-#define SWAP(i)                                 \
-    if (dd[(i)+1] > dd[i]) {                      \
-        tmp       = dd[i];                              \
-        dd[i]     = dd[(i)+1];                          \
-        dd[(i)+1] = tmp;                            \
+    if (dd[1] > dd[0])
+    {
+        std::swap(dd[0], dd[1]);
+    }
+    if (dd[2] > dd[1])
+    {
+        std::swap(dd[1], dd[2]);
+    }
+    if (dd[1] > dd[0])
+    {
+        std::swap(dd[0], dd[1]);
     }
-    SWAP(0);
-    SWAP(1);
-    SWAP(0);
 
     for (m = 0; (m < DIM); m++)
     {
index 5a85e9efa4fbff6d2d902447e9d3f41305fcef1d..77d472385e718646a5e6bbfad85d55bdba01a2a2 100644 (file)
@@ -106,12 +106,7 @@ using IntegratorFunctionType = void();
  * code. Once many of them have become modules, we should change this
  * approach.
  *
- * Note that the presence of const reference members means that the
- * default constructor would be implicitly deleted. But we only want
- * to make one of these when we know how to initialize these members,
- * so that is perfect. To ensure this remains true even if we would
- * remove those members, we explicitly delete this constructor.
- * Other constructors, copies and moves are OK. */
+ * Use a braced initializer list to construct one of these. */
 struct Integrator
 {
     //! Handles logging.
@@ -195,8 +190,6 @@ struct Integrator
     /*! \brief Function to run the correct IntegratorFunctionType,
      * based on the .mdp integrator field. */
     void run(unsigned int ei, bool doRerun);
-    //! We only intend to construct such objects with an initializer list.
-    Integrator() = delete;
 };
 
 }      // namespace gmx
index d80e5cba93cc972e39ea297e40e1f51f17a57543..925a8b5d43890042d7cb62d4655c2a8bf6faf96f 100644 (file)
@@ -739,13 +739,10 @@ namespace
  * updated, then the member will be value initialized, which will
  * typically mean initialization to zero.
  *
- * We only want to construct one of these with an initializer list, so
- * we explicitly delete the default constructor. */
+ * Use a braced initializer list to construct one of these. */
 class EnergyEvaluator
 {
     public:
-        //! We only intend to construct such objects with an initializer list.
-        EnergyEvaluator() = delete;
         /*! \brief Evaluates an energy on the state in \c ems.
          *
          * \todo In practice, the same objects mu_tot, vir, and pres
index b758ccd0d396f5d61f64f085f64f30d6926c811c..23820d9b94ed62f3f739572247512b3cf65ccfce 100644 (file)
  * The minor index of the array goes over both the LJ combination rules,
  * which is only supported by plain cut-off, and the LJ switch/PME functions.
  */
-p_nbk_func_noener nbnxm_kernel_noener_simd_{1}[coulktNR][vdwktNR] =
+static p_nbk_func_noener nbnxm_kernel_noener_simd_{1}[coulktNR][vdwktNR] =
 {2}
-p_nbk_func_ener nbnxm_kernel_ener_simd_{1}[coulktNR][vdwktNR] =
+static p_nbk_func_ener nbnxm_kernel_ener_simd_{1}[coulktNR][vdwktNR] =
 {3}
-p_nbk_func_ener nbnxm_kernel_energrp_simd_{1}[coulktNR][vdwktNR] =
+static p_nbk_func_ener nbnxm_kernel_energrp_simd_{1}[coulktNR][vdwktNR] =
 {4}
 
 #endif /* INCLUDE_KERNELFUNCTION_TABLES */
index 81aa381bf7696fe54c0c34cc705e159daea4a412..e287e8d14353feee20ea8c26c59c30d8cf37105f 100644 (file)
@@ -143,7 +143,7 @@ nbk_func_noener       nbnxm_kernel_ElecEwTwinCut_VdwLJEwCombGeom_F_2xmm;
  * The minor index of the array goes over both the LJ combination rules,
  * which is only supported by plain cut-off, and the LJ switch/PME functions.
  */
-p_nbk_func_noener nbnxm_kernel_noener_simd_2xmm[coulktNR][vdwktNR] =
+static p_nbk_func_noener nbnxm_kernel_noener_simd_2xmm[coulktNR][vdwktNR] =
 {
     {
         nbnxm_kernel_ElecRF_VdwLJCombGeom_F_2xmm,
@@ -187,7 +187,7 @@ p_nbk_func_noener nbnxm_kernel_noener_simd_2xmm[coulktNR][vdwktNR] =
     },
 };
 
-p_nbk_func_ener nbnxm_kernel_ener_simd_2xmm[coulktNR][vdwktNR] =
+static p_nbk_func_ener nbnxm_kernel_ener_simd_2xmm[coulktNR][vdwktNR] =
 {
     {
         nbnxm_kernel_ElecRF_VdwLJCombGeom_VF_2xmm,
@@ -231,7 +231,7 @@ p_nbk_func_ener nbnxm_kernel_ener_simd_2xmm[coulktNR][vdwktNR] =
     },
 };
 
-p_nbk_func_ener nbnxm_kernel_energrp_simd_2xmm[coulktNR][vdwktNR] =
+static p_nbk_func_ener nbnxm_kernel_energrp_simd_2xmm[coulktNR][vdwktNR] =
 {
     {
         nbnxm_kernel_ElecRF_VdwLJCombGeom_VgrpF_2xmm,
index 6a22a40cb75b1aff323a9d8d4655e7c61e122cbc..3ad2c58784d506b5394d8b044d337e393dd7cf85 100644 (file)
@@ -143,7 +143,7 @@ nbk_func_noener       nbnxm_kernel_ElecEwTwinCut_VdwLJEwCombGeom_F_4xm;
  * The minor index of the array goes over both the LJ combination rules,
  * which is only supported by plain cut-off, and the LJ switch/PME functions.
  */
-p_nbk_func_noener nbnxm_kernel_noener_simd_4xm[coulktNR][vdwktNR] =
+static p_nbk_func_noener nbnxm_kernel_noener_simd_4xm[coulktNR][vdwktNR] =
 {
     {
         nbnxm_kernel_ElecRF_VdwLJCombGeom_F_4xm,
@@ -187,7 +187,7 @@ p_nbk_func_noener nbnxm_kernel_noener_simd_4xm[coulktNR][vdwktNR] =
     },
 };
 
-p_nbk_func_ener nbnxm_kernel_ener_simd_4xm[coulktNR][vdwktNR] =
+static p_nbk_func_ener nbnxm_kernel_ener_simd_4xm[coulktNR][vdwktNR] =
 {
     {
         nbnxm_kernel_ElecRF_VdwLJCombGeom_VF_4xm,
@@ -231,7 +231,7 @@ p_nbk_func_ener nbnxm_kernel_ener_simd_4xm[coulktNR][vdwktNR] =
     },
 };
 
-p_nbk_func_ener nbnxm_kernel_energrp_simd_4xm[coulktNR][vdwktNR] =
+static p_nbk_func_ener nbnxm_kernel_energrp_simd_4xm[coulktNR][vdwktNR] =
 {
     {
         nbnxm_kernel_ElecRF_VdwLJCombGeom_VgrpF_4xm,