[CDRIVER-6043] Modify definitions in intutil to avoid compound-literals (#2056)

* Modify definitions in intutil to avoid compound-lits

The followin changes are made:

- We avoid compound literals expanded from macros. This is to avoid
  buggy compound-literal support in VS 2017 (See CDRIVER-6043).

  Instead, we pass the relevant argument to a function that initializes
  the new aggregate object.

  This has the downside that we cannot use the macros in constant
  expressions, but that was a minor feature that wasn't really used
  anywhere.
- Add a new macro to define bit masks. This simplifies the expressions
  that build bit patterns used for two's complement arithmetic.
- Rename the members of `mlib_upsized_integer` to give them meaningful
  names.

Author: vector-of-bool

src/common/src/mlib/ckdint.h

@@ -190,19 +190,17 @@ mlib_extern_c_begin ();
 // clang-format off
 // Generates an 0b11111 bit pattern for appropriate size:
 #define _mlibMaxofUnsigned(V) \
-   (sizeof(V) == sizeof(uintmax_t) \
-      ? UINTMAX_MAX /* No funny bit math, just return the max of the max int */ \
-      /* Generate an 0b11111... bit pattern */ \
-      : (UINTMAX_C(1) << ( \
-         /* Guard against an over-shift if V is uintmax_t: */ \
-           (sizeof(V) < sizeof(uintmax_t)) \
-         * (sizeof(V) * CHAR_BIT)) \
-         ) - 1)
+   /* NOLINTNEXTLINE(bugprone-sizeof-expression) */ \
+   mlib_bits(mlib_bitsizeof((V)), 0)
 
 // Generates an 0b01111 bit pattern for the two's complement max value:
-#define _mlibMaxofSigned(V) (_mlibMaxofUnsigned (V) >> 1ull)
+#define _mlibMaxofSigned(V) \
+   /* NOLINTNEXTLINE(bugprone-sizeof-expression) */ \
+   mlib_bits(mlib_bitsizeof(V) - 1u, 0)
 // Generates an 0b10000... bit pattern for the two's complement min value:
-#define _mlibMinofSigned(V) ((0 & (V)) - (UINTMAX_C (1) << ((sizeof (V) * CHAR_BIT) - 1)))
+#define _mlibMinofSigned(V) \
+   /* NOLINTNEXTLINE(bugprone-sizeof-expression) */ \
+   (0 - mlib_bits(1, mlib_bitsizeof(V) - 1u))
 // For completeness:
 #define _mlibMinofUnsigned(V) 0
 // Yields true iff the operand expression has a signed type, but requires that
@@ -265,7 +263,7 @@ static inline bool (mlib_add) (uintmax_t *dst, bool dst_signed, bool a_signed, u
    // Perform regular wrapping arithmetic on the unsigned value. The bit pattern
    // is equivalent if there is two's complement signed arithmetic.
    const uintmax_t sum = *dst = a + b;
-   const uintmax_t signbit = (UINTMAX_C (1) << ((sizeof (intmax_t) * CHAR_BIT) - 1));
+   const uintmax_t signbit = mlib_bits (1, mlib_bitsizeof (uintmax_t) - 1u);
    // Now we check whether that overflowed according to the sign configuration.
    // We use some bit fiddling magic that treat the signbit as a boolean for
    // "is this number negative?" or "is this number “large” (i.e. bigger than signed-max)?"
@@ -359,7 +357,7 @@ static inline bool (mlib_sub) (uintmax_t *dst, bool dst_signed, bool a_signed, u
 {
    // Perform the subtraction using regular wrapping arithmetic
    const uintmax_t diff = *dst = a - b;
-   const uintmax_t signbit = (UINTMAX_C (1) << ((sizeof (intmax_t) * CHAR_BIT) - 1));
+   const uintmax_t signbit = mlib_bits (1, mlib_bitsizeof (uintmax_t) - 1u);
    // Test whether the operation overflowed for the given sign configuration
    // (See mlib_add for more details on why we do this bit fiddling)
    if (dst_signed) {
@@ -460,7 +458,7 @@ static inline bool (mlib_mul) (uintmax_t *dst, bool dst_signed, bool a_signed, u
    mlib_noexcept
 {
    // Multiplication is a lot more subtle
-   const uintmax_t signbit = (UINTMAX_C (1) << ((sizeof (intmax_t) * CHAR_BIT) - 1));
+   const uintmax_t signbit = mlib_bits (1, mlib_bitsizeof (uintmax_t) - 1u);
    if (dst_signed) {
       if (a_signed) {
          if (b_signed) {
@@ -576,7 +574,7 @@ _mlib_ckdint (void *dst,
 {
    // Perform the arithmetic on uintmax_t, for wrapping behavior
    uintmax_t tmp;
-   bool ovr = fn (&tmp, minval < 0, a.is_signed, a.i.u, b.is_signed, b.i.u);
+   bool ovr = fn (&tmp, minval < 0, a.is_signed, a.bits.as_unsigned, b.is_signed, b.bits.as_unsigned);
    // Endian-adjusting for writing the result
    const char *copy_from = (const char *) &tmp;
    if (!mlib_is_little_endian ()) {
@@ -651,7 +649,7 @@ _mlib_checked_cast (intmax_t min_,
                   here.lineno,
                   here.func,
                   expr,
-                  (long long) val.i.s,
+                  (long long) val.bits.as_signed,
                   typename_);
       } else {
          fprintf (stderr,
@@ -660,16 +658,16 @@ _mlib_checked_cast (intmax_t min_,
                   here.lineno,
                   here.func,
                   expr,
-                  (unsigned long long) val.i.u,
+                  (unsigned long long) val.bits.as_unsigned,
                   typename_);
       }
       fflush (stderr);
       abort ();
    }
    if (val.is_signed) {
-      return (uintmax_t) val.i.s;
+      return (uintmax_t) val.bits.as_signed;
    }
-   return val.i.u;
+   return val.bits.as_unsigned;
 }
 
 mlib_extern_c_end ();

src/common/src/mlib/ckdint.test.cpp

@@ -15,9 +15,6 @@
 #endif // __has_builtin
 #endif
 
-static_assert (mlib_upsize_integer (42ll).is_signed, "mlib_upsize_integer yielded the wrong answer");
-static_assert (!mlib_upsize_integer (42ull).is_signed, "mlib_upsize_integer yielded the wrong answer");
-
 template <typename... Ts> struct typelist {
 };
 using integer_types = typelist<char,

src/common/src/mlib/cmp.h

@@ -77,32 +77,32 @@ mlib_always_inline static enum mlib_cmp_result (mlib_cmp) (struct mlib_upsized_i
    if (x.is_signed) {
       if (y.is_signed) {
          // Both signed
-         if (x.i.s < y.i.s) {
+         if (x.bits.as_signed < y.bits.as_signed) {
             return mlib_less;
-         } else if (x.i.s > y.i.s) {
+         } else if (x.bits.as_signed > y.bits.as_signed) {
             return mlib_greater;
          }
       } else {
          // X signed, Y unsigned
-         if (x.i.s < 0 || (uintmax_t) x.i.s < y.i.u) {
+         if (x.bits.as_signed < 0 || (uintmax_t) x.bits.as_signed < y.bits.as_unsigned) {
             return mlib_less;
-         } else if ((uintmax_t) x.i.s > y.i.u) {
+         } else if ((uintmax_t) x.bits.as_signed > y.bits.as_unsigned) {
             return mlib_greater;
          }
       }
    } else {
       if (!y.is_signed) {
          // Both unsigned
-         if (x.i.u < y.i.u) {
+         if (x.bits.as_unsigned < y.bits.as_unsigned) {
             return mlib_less;
-         } else if (x.i.u > y.i.u) {
+         } else if (x.bits.as_unsigned > y.bits.as_unsigned) {
             return mlib_greater;
          }
       } else {
          // X unsigned, Y signed
-         if (y.i.s < 0 || x.i.u > (uintmax_t) y.i.s) {
+         if (y.bits.as_signed < 0 || x.bits.as_unsigned > (uintmax_t) y.bits.as_signed) {
             return mlib_greater;
-         } else if (x.i.u < (uintmax_t) y.i.s) {
+         } else if (x.bits.as_unsigned < (uintmax_t) y.bits.as_signed) {
             return mlib_less;
          }
       }
@@ -123,9 +123,9 @@ mlib_always_inline static enum mlib_cmp_result (mlib_cmp) (struct mlib_upsized_i
 static inline bool (mlib_in_range) (intmax_t min_, uintmax_t max_, struct mlib_upsized_integer val) mlib_noexcept
 {
    if (val.is_signed) {
-      return mlib_cmp (val.i.s, >=, min_) && mlib_cmp (val.i.s, <=, max_);
+      return mlib_cmp (val.bits.as_signed, >=, min_) && mlib_cmp (val.bits.as_signed, <=, max_);
    } else {
-      return mlib_cmp (val.i.u, >=, min_) && mlib_cmp (val.i.u, <=, max_);
+      return mlib_cmp (val.bits.as_unsigned, >=, min_) && mlib_cmp (val.bits.as_unsigned, <=, max_);
    }
 }
 

src/common/src/mlib/intutil.h

@@ -32,27 +32,59 @@
  */
 #define mlib_is_signed(T) (!((T) (-1) > 0))
 
+/**
+ * @brief Like `sizeof`, but returns the number of bits in the object representation
+ */
+#define mlib_bitsizeof(T) ((sizeof (T)) * ((size_t) CHAR_BIT))
+
 // clang-format off
+/**
+ * @brief Generate a mask of contiguous bits.
+ *
+ * @param NumOnes The non-negative number of contiguous 1 bits
+ * @param NumZeros The non-negative number of contiguous 0 bits to set in the low position
+ *
+ * The generated mask is of the form:
+ *
+ *             NumZeros
+ *                 │
+ *                ┌┴─┐
+ *                │  │
+ *     `0..0 1..1 0..0`
+ *           │  │
+ *           └┬─┘
+ *            │
+ *        NumOnes
+ *
+ * Explain the arithmetic below:
+ *
+ * 1. `ones = 0b1111...` : All high bits
+ * 2. `tmp  = ones >> (NumOnes - num_bits_of(ones))` : Truncate to the number of 1s we want
+ * 3. `res  = tmp  << NumZeros` : Add the 0s in the low position
+ */
+#define mlib_bits(NumOnes, NumZeros) ( \
+   ((NumOnes) \
+      ? (~UINTMAX_C(0) >> ((mlib_bitsizeof(uintmax_t) - (uintmax_t)(NumOnes)))) \
+      : 0) \
+   << ((uintmax_t)(NumZeros)))
+
 /**
  * @brief Given an integral type, yield an integral constant value representing
  * the maximal value of that type.
  */
 #define mlib_maxof(T) \
    ((T) (mlib_is_signed (T) \
-        ? ((T) ((((T) 1 << (sizeof (T) * CHAR_BIT - 2)) - 1) * 2 + 1)) \
-        : ((T) ~(T) 0)))
+        ? ((T) mlib_bits(mlib_bitsizeof(T) - 1u, 0)) \
+        : ((T) mlib_bits(mlib_bitsizeof(T),      0))))
 
 /**
  * @brief Given an integral type, yield an integral constant value for the
  * minimal value of that type.
  */
 #define mlib_minof(T) \
-   MLIB_PRAGMA_IF_MSVC (warning (push)) \
-   MLIB_PRAGMA_IF_MSVC (warning (disable : 4146)) \
    ((T) (!mlib_is_signed (T) \
         ? (T) 0 \
-        : (T) (-((((T) 1 << (sizeof (T) * CHAR_BIT - 2)) - 1) * 2 + 1) - 1))) \
-   MLIB_PRAGMA_IF_MSVC (warning (pop))
+        : (T) mlib_bits(1, mlib_bitsizeof(T) - 1u)))
 // clang-format on
 
 /**
@@ -63,11 +95,11 @@
 typedef struct mlib_upsized_integer {
    union {
       // The signed value of the integer
-      intmax_t s;
+      intmax_t as_signed;
       // The unsigned value of the integer
-      uintmax_t u;
-   } i;
-   // Whether the upscaled integer is stored in the signed field or the unsigned field
+      uintmax_t as_unsigned;
+   } bits;
+   // Whether the upscaled integer bits should be treated as a two's complement signed integer
    bool is_signed;
 } mlib_upsized_integer;
 
@@ -85,23 +117,29 @@ typedef struct mlib_upsized_integer {
  * If the integer to upcast is the same size as `intmax_t`, we need to decide whether to store
  * it as unsigned. The expression `(_mlibGetOne(Value)) - 2 < 1` will be `true` iff the operand is signed,
  * otherwise false. If the operand is signed, we can safely cast to `intmax_t` (it probably already
- * is of that type), otherwise, we can to `uintmax_t` and the returned `mlib_upsized_integer` will
+ * is of that type), otherwise, we cast to `uintmax_t` and the returned `mlib_upsized_integer` will
  * indicate that the stored value is unsigned. The expression `1 - 2 < 1` is chosen
  * to avoid `-Wtype-limits` warnings from some compilers about unsigned comparison.
  */
 #define mlib_upsize_integer(Value) \
-   MLIB_PRAGMA_IF_MSVC (warning(push)) \
-   MLIB_PRAGMA_IF_MSVC (warning(disable : 4189)) \
+   mlib_upsize_integer((uintmax_t)(intmax_t)((Value)), _mlibShouldTreatBitsAsSigned(Value))
+#define _mlibShouldTreatBitsAsSigned(Value) \
    /* NOLINTNEXTLINE(bugprone-sizeof-expression) */ \
-   ((sizeof ((Value)) < sizeof (intmax_t) || (_mlibGetOne(Value) - 2) < _mlibGetOne(Value)) \
-      ? mlib_init(mlib_upsized_integer) {{(intmax_t) (Value)}, true} \
-      : mlib_init(mlib_upsized_integer) {{(intmax_t) (uintmax_t) (Value)}}) \
-   MLIB_PRAGMA_IF_MSVC (warning(pop))
+   (sizeof ((Value)) < sizeof (intmax_t) || (_mlibGetOne(Value) - 2) < _mlibGetOne(Value))
 // Yield a 1 value of similar-ish type to the given expression. The ternary
 // forces an integer promotion of literal 1 match the type of `V`, while leaving
 // `V` unevaluated. Note that this will also promote `V` to be at least `(unsigned) int`,
 // so the 1 value is only "similar" to `V`, and may be of a larger type
 #define _mlibGetOne(V) (1 ? 1 : (V))
+// Function impl for upsize_integer
+static inline mlib_upsized_integer
+(mlib_upsize_integer) (uintmax_t bits, bool treat_as_signed)
+{
+   mlib_upsized_integer ret;
+   ret.bits.as_unsigned = bits;
+   ret.is_signed = treat_as_signed;
+   return ret;
+}
 // clang-format on
 
 #endif // MLIB_INTUTIL_H_INCLUDED

src/common/src/mlib/str.h

@@ -276,15 +276,16 @@ _mstr_adjust_index (mstr_view s, mlib_upsized_integer pos, bool clamp_to_length)
       // We want to clamp to the length, and the given value is greater than the string length.
       return s.len;
    }
-   if (pos.is_signed && pos.i.s < 0) {
+   if (pos.is_signed && pos.bits.as_signed < 0) {
       // This will add the negative value to the length of the string. If such
       // an operation would result a negative value, this will terminate the
       // program.
-      return mlib_assert_add (size_t, s.len, pos.i.s);
+      return mlib_assert_add (size_t, s.len, pos.bits.as_signed);
    }
    // No special behavior, just assert that the given position is in-bounds for the string
-   mlib_check (pos.i.u <= s.len, because, "the string position index must not be larger than the string length");
-   return pos.i.u;
+   mlib_check (
+      pos.bits.as_unsigned <= s.len, because, "the string position index must not be larger than the string length");
+   return pos.bits.as_unsigned;
 }
 
 /**

src/common/src/mlib/test.h

@@ -193,16 +193,16 @@ _mlibCheckIntCmp (enum mlib_cmp_result cres, // The cmp result to check
                right_expr);
       fprintf (stderr, "    ");
       if (left.is_signed) {
-         fprintf (stderr, "%lld", (long long) left.i.s);
+         fprintf (stderr, "%lld", (long long) left.bits.as_signed);
       } else {
-         fprintf (stderr, "%llu", (unsigned long long) left.i.u);
+         fprintf (stderr, "%llu", (unsigned long long) left.bits.as_unsigned);
       }
       fprintf (stderr, " ⟨%s⟩\n", left_expr);
       fprintf (stderr, "    ");
       if (right.is_signed) {
-         fprintf (stderr, "%lld", (long long) right.i.s);
+         fprintf (stderr, "%lld", (long long) right.bits.as_signed);
       } else {
-         fprintf (stderr, "%llu", (unsigned long long) right.i.u);
+         fprintf (stderr, "%llu", (unsigned long long) right.bits.as_unsigned);
       }
       fprintf (stderr, " ⟨%s⟩\n", right_expr);
       fflush (stderr);

src/common/tests/test-mlib.c

@@ -52,6 +52,17 @@ _test_checks (void)
    }
 }
 
+static void
+_test_bits (void)
+{
+   mlib_check (mlib_bits (0, 0), eq, 0);           // 0b000
+   mlib_check (mlib_bits (1, 0), eq, 1);           // 0b001
+   mlib_check (mlib_bits (2, 0), eq, 3);           // 0b011
+   mlib_check (mlib_bits (1, 1), eq, 2);           // 0b010
+   mlib_check (mlib_bits (5, 3), eq, 248);         // 0b11111000
+   mlib_check (mlib_bits (64, 0), eq, UINT64_MAX); // 0b111...
+}
+
 static void
 _test_minmax (void)
 {
@@ -95,23 +106,23 @@ _test_upsize (void)
 {
    struct mlib_upsized_integer up;
    up = mlib_upsize_integer (31);
-   ASSERT (up.is_signed);
-   ASSERT (up.i.s == 31);
+   mlib_check (up.is_signed);
+   mlib_check (up.bits.as_signed == 31);
 
    // Casting from the max unsigned integer generates an unsigned upsized integer:
    up = mlib_upsize_integer ((uintmax_t) 1729);
-   ASSERT (!up.is_signed);
-   ASSERT (up.i.u == 1729);
+   mlib_check (!up.is_signed);
+   mlib_check (up.bits.as_unsigned == 1729);
 
    // Max signed integer makes a signed upsized integer:
    up = mlib_upsize_integer ((intmax_t) 1729);
-   ASSERT (up.is_signed);
-   ASSERT (up.i.s == 1729);
+   mlib_check (up.is_signed);
+   mlib_check (up.bits.as_signed == 1729);
 
    // From a literal:
    up = mlib_upsize_integer (UINTMAX_MAX);
-   ASSERT (!up.is_signed);
-   ASSERT (up.i.u == UINTMAX_MAX);
+   mlib_check (!up.is_signed);
+   mlib_check (up.bits.as_unsigned == UINTMAX_MAX);
 }
 
 static void
@@ -880,6 +891,7 @@ void
 test_mlib_install (TestSuite *suite)
 {
    TestSuite_Add (suite, "/mlib/checks", _test_checks);
+   TestSuite_Add (suite, "/mlib/intutil/bits", _test_bits);
    TestSuite_Add (suite, "/mlib/intutil/minmax", _test_minmax);
    TestSuite_Add (suite, "/mlib/intutil/upsize", _test_upsize);
    TestSuite_Add (suite, "/mlib/cmp", _test_cmp);