🐛 Avoid lambdas in comparison and scaling for LLVM 23 LTO

src/core/include/mp-units/framework/quantity.h

@@ -299,22 +299,33 @@ template<typename Q1, typename Q2, typename Cmp>
   }
 }
 
+// Note from libhal: this used to be a lambda inside `compare_in_common_type()`. LLVM 23 (seen
+// with clang 23.1.2, 85ac560262434c9ccfc0c183ec22d4138ed647fb) emits a broken module under full
+// LTO with `-g` when a function template holding a lambda is instantiated both in a module unit
+// and in a TU that imports that module unit: "invalid retained nodes, retained node does not
+// belong to subprogram". `ld.lld` aborts the link ("Broken module found"); Apple's ld64 only warns
+// and strips the debug info. libhal hit it with `-std=c++23 -stdlib=libc++ -flto -g -Os
+// -fsanitize=address` on Linux x86_64 and arm64, where a module unit and its unit test both
+// compared `quantity<si::hertz, unsigned>` values. LLVM 22.1.8 is not affected. A named function
+// has no local type for the linker to merge, so it avoids the bug. Like the lambda, it is called
+// from either branch below, so there is still no fallthrough `return` for MSVC Debug C4702.
+template<typename Q1, typename Q2, typename Cmp>
+[[nodiscard]] constexpr auto compare_in_common_type_fallback(const Q1& lhs, const Q2& rhs, Cmp cmp)
+{
+  using ct = std::common_type_t<Q1, Q2>;
+  const ct ct_lhs(lhs);
+  const ct ct_rhs(rhs);
+  MP_UNITS_DIAGNOSTIC_PUSH
+  MP_UNITS_DIAGNOSTIC_IGNORE_FLOAT_EQUAL
+  return cmp(ct_lhs.numerical_value_ref_in(ct::unit), ct_rhs.numerical_value_ref_in(ct::unit));
+  MP_UNITS_DIAGNOSTIC_POP
+}
+
 template<typename Q1, typename Q2, typename Cmp>
 [[nodiscard]] constexpr auto compare_in_common_type(const Q1& lhs, const Q2& rhs, Cmp cmp)
 {
   using ct = std::common_type_t<Q1, Q2>;
   using ct_rep = value_type_t<typename ct::rep>;
-  // `fallback_cmp` is a lambda so it can be invoked from either branch below — a bare fallthrough
-  // `return` after the `if constexpr` chain would trip MSVC Debug C4702 ("unreachable code") on
-  // instantiations where the optimization branch returns.
-  auto fallback_cmp = [&] {
-    const ct ct_lhs(lhs);
-    const ct ct_rhs(rhs);
-    MP_UNITS_DIAGNOSTIC_PUSH
-    MP_UNITS_DIAGNOSTIC_IGNORE_FLOAT_EQUAL
-    return cmp(ct_lhs.numerical_value_ref_in(ct::unit), ct_rhs.numerical_value_ref_in(ct::unit));
-    MP_UNITS_DIAGNOSTIC_POP
-  };
   if constexpr (!treat_as_floating_point<ct_rep> && sizeof(ct_rep) < sizeof(int128_t)) {
     // widen to a double-width integer so the unit-ratio scaling can't overflow
     using wide_t = double_width_int_for_t<ct_rep>;
@@ -324,10 +335,10 @@ template<typename Q1, typename Q2, typename Cmp>
       return cmp(scale<wide_t>(lhs_m, lhs.numerical_value_is_an_implementation_detail_),
                  scale<wide_t>(rhs_m, rhs.numerical_value_is_an_implementation_detail_));
     } else {
-      return fallback_cmp();
+      return compare_in_common_type_fallback(lhs, rhs, cmp);
     }
   } else {
-    return fallback_cmp();
+    return compare_in_common_type_fallback(lhs, rhs, cmp);
   }
 }
 

src/core/include/mp-units/framework/scaling.h

@@ -86,6 +86,18 @@ template<typename T>
   return value / value;
 }
 
+// Note from libhal: this and `scale_fp_for()` below were immediately-invoked lambdas. They are
+// named functions to avoid the LLVM 23 LTO bug described above `compare_in_common_type_fallback()`
+// in `quantity.h`.
+template<typename T, typename D, typename Q>
+[[nodiscard]] constexpr auto div_remainder(const T& dividend, const D& divisor, const Q& quot)
+{
+  if constexpr (requires { dividend % divisor; })
+    return dividend % divisor;
+  else
+    return dividend - quot * divisor;
+}
+
 /**
  * @brief Integer division with the quotient rounded according to @c Mode
  */
@@ -106,12 +118,7 @@ template<rounding_mode Mode, typename T, typename D>
     // `%` is optional for a `RealScalar` though, so the subtraction stays as the general form: it
     // needs only the arithmetic the concept mandates, which is what the emulated 128-bit integer
     // and any other representation type without `%` rely on.
-    const auto rem = [&] {
-      if constexpr (requires { dividend % divisor; })
-        return dividend % divisor;
-      else
-        return dividend - quot * divisor;
-    }();
+    const auto rem = div_remainder(dividend, divisor, quot);
     using rem_type = std::remove_const_t<decltype(rem)>;
     const rem_type no_rem = get_zero(rem);
     if constexpr (Mode == rounding_mode::rounded_down)
@@ -232,6 +239,15 @@ template<auto M, rounding_mode Mode, typename T>
   }
 }
 
+template<typename To, UnitMagnitude M, typename From>
+[[nodiscard]] constexpr auto scale_fp_for(const From& value)
+{
+  if constexpr (treat_as_integral<value_type_t<From>> || !std::floating_point<value_type_t<From>>)
+    return scale_fp<M{}>(static_cast<To>(value));
+  else
+    return scale_fp<M{}>(value);
+}
+
 template<typename To, rounding_mode Mode, UnitMagnitude M, typename From>
   requires UnitMagnitudeScalable<From>
 [[nodiscard]] constexpr auto scale_impl(M m, const From& value)
@@ -255,12 +271,7 @@ template<typename To, rounding_mode Mode, UnitMagnitude M, typename From>
     //    To could overflow the wrapper's narrow rep (e.g. scaled_integer<int,P<-25>>
     //    times itself yields P<-50>, which cannot be downshifted to P<-15> within
     //    a 32-bit int).  Converting to To first keeps all arithmetic in To's type.
-    const auto scaled = [&] {
-      if constexpr (treat_as_integral<value_type_t<From>> || !std::floating_point<value_type_t<From>>)
-        return scale_fp<M{}>(static_cast<To>(value));
-      else
-        return scale_fp<M{}>(value);
-    }();
+    const auto scaled = scale_fp_for<To, M>(value);
     // A floating-point destination represents the scaled value with at most half ULP error
     // (`rounded` semantics, the only policy valid for it), so rounding applies only when the
     // destination is integral and a mode other than the `silent_cast` truncation is requested.