Fix clang 10 -Wimplicit-int-float-conversion warnings
authorSzilárd Páll <pall.szilard@gmail.com>
Sun, 12 Apr 2020 16:16:43 +0000 (16:16 +0000)
committerPaul Bauer <paul.bauer.q@gmail.com>
Sun, 12 Apr 2020 16:16:43 +0000 (16:16 +0000)
src/gromacs/fileio/libxdrf.cpp
src/gromacs/random/uniformrealdistribution.h
src/gromacs/simd/tests/simd_integer.cpp

index 3c336fda934e0e46480b762d95ccd77569a6dc1a..2099e5e798f39a2fee503ac60aef7e0222025a59 100644 (file)
@@ -62,7 +62,14 @@ const char* xdr_datatype_names[] = { "int", "float", "double", "large int", "cha
  | with some routines to assist in this task (those are marked
  | static and cannot be called from user programs)
  */
-#define MAXABS (INT_MAX - 2)
+
+// Integers above 2^24 do not have unique representations in
+// 32-bit floats ie with 24 bits of precision.  We use maxAbsoluteInt
+// to check that float values can be transformed into an in-range
+// 32-bit integer. There is no need to ensure we are within the range
+// of ints with exact floating-point representations. However, we should
+// reject all floats above that which converts to an in-range 32-bit integer.
+const float maxAbsoluteInt = nextafterf(float(INT_MAX), 0.f); // NOLINT(cert-err58-cpp)
 
 #ifndef SQR
 #    define SQR(x) ((x) * (x))
@@ -490,7 +497,7 @@ int xdr3dfcoord(XDR* xdrs, float* fp, int* size, float* precision)
             {
                 lf = *lfp * *precision - 0.5;
             }
-            if (std::fabs(lf) > MAXABS)
+            if (std::fabs(lf) > maxAbsoluteInt)
             {
                 /* scaling would cause overflow */
                 errval = 0;
@@ -514,7 +521,7 @@ int xdr3dfcoord(XDR* xdrs, float* fp, int* size, float* precision)
             {
                 lf = *lfp * *precision - 0.5;
             }
-            if (std::fabs(lf) > MAXABS)
+            if (std::fabs(lf) > maxAbsoluteInt)
             {
                 /* scaling would cause overflow */
                 errval = 0;
@@ -538,7 +545,7 @@ int xdr3dfcoord(XDR* xdrs, float* fp, int* size, float* precision)
             {
                 lf = *lfp * *precision - 0.5;
             }
-            if (std::abs(lf) > MAXABS)
+            if (std::abs(lf) > maxAbsoluteInt)
             {
                 /* scaling would cause overflow */
                 errval = 0;
@@ -575,9 +582,9 @@ int xdr3dfcoord(XDR* xdrs, float* fp, int* size, float* precision)
             return 0;
         }
 
-        if (static_cast<float>(maxint[0]) - static_cast<float>(minint[0]) >= MAXABS
-            || static_cast<float>(maxint[1]) - static_cast<float>(minint[1]) >= MAXABS
-            || static_cast<float>(maxint[2]) - static_cast<float>(minint[2]) >= MAXABS)
+        if (static_cast<float>(maxint[0]) - static_cast<float>(minint[0]) >= maxAbsoluteInt
+            || static_cast<float>(maxint[1]) - static_cast<float>(minint[1]) >= maxAbsoluteInt
+            || static_cast<float>(maxint[2]) - static_cast<float>(minint[2]) >= maxAbsoluteInt)
         {
             /* turning value in unsigned by subtracting minint
              * would cause overflow
index a90e320f2fc807052d9af1590e0fc563ae1c908b..bc7cb61405354569caf0207b387153f16db3722f 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2015,2016,2018,2019, by the GROMACS development team, led by
+ * Copyright (c) 2015,2016,2018,2019,2020, 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.
@@ -52,6 +52,7 @@
 
 #include <algorithm>
 #include <limits>
+#include <type_traits>
 
 #include "gromacs/math/functions.h"
 #include "gromacs/utility/basedefinitions.h"
@@ -109,10 +110,17 @@ RealType generateCanonical(Rng& g)
     const uint64_t range    = Rng::max() - Rng::min() + uint64_t(1);
     uint64_t       log2R    = (range == 0) ? std::numeric_limits<uint64_t>::digits : log2I(range);
     uint64_t       k        = realBits / log2R + (realBits % log2R != 0) + (realBits == 0);
-    RealType       r        = Rng::max() - Rng::min() + RealType(1);
-    RealType       s        = g() - Rng::min();
-    RealType       base     = r;
-    RealType       result;
+    // Note that Rng::max and Rng::min are typically an integer type.
+    // Only unsigned integer types can express the range using the
+    // same type. Converting to RealType before computing the range
+    // would work but we have no need for that.
+    static_assert(std::is_unsigned<decltype(Rng::max())>::value
+                          && std::is_unsigned<decltype(Rng::min())>::value,
+                  "Rng::max and Rng::min must be unsigned");
+    RealType r    = RealType(Rng::max() - Rng::min()) + RealType(1);
+    RealType s    = g() - Rng::min();
+    RealType base = r;
+    RealType result;
 
     for (uint64_t i = 1; i < k; ++i)
     {
index 8eff177f7629a6feb83de285d29e86e0ed90bc10..ffcec29abc9a5c819a0fb9d00b08e77463ccc10f 100644 (file)
@@ -228,8 +228,8 @@ TEST_F(SimdIntegerTest, cvtI2R)
 {
     GMX_EXPECT_SIMD_REAL_EQ(setSimdRealFrom1R(2.0), cvtI2R(SimdInt32(2)));
     GMX_EXPECT_SIMD_REAL_EQ(setSimdRealFrom1R(-2.0), cvtI2R(SimdInt32(-2)));
-    GMX_EXPECT_SIMD_REAL_EQ(setSimdRealFrom1R(102448689), cvtI2R(SimdInt32(102448689)));
-    GMX_EXPECT_SIMD_REAL_EQ(setSimdRealFrom1R(-102448689), cvtI2R(SimdInt32(-102448689)));
+    GMX_EXPECT_SIMD_REAL_EQ(setSimdRealFrom1R(102448689._real), cvtI2R(SimdInt32(102448689)));
+    GMX_EXPECT_SIMD_REAL_EQ(setSimdRealFrom1R(-102448689._real), cvtI2R(SimdInt32(-102448689)));
 }
 #    endif // GMX_SIMD_HAVE_REAL