boostorg / multiprecision

Boost.Multiprecision
Boost Software License 1.0
194 stars 111 forks source link

Syntax of casting in 1.83 #563

Closed ckormanyos closed 1 year ago

ckormanyos commented 1 year ago

In 1.83 and maybe for a while I've found a syntax of casting issue that I'd like to address prior to 1.84.

Issue: I'm not exactly sure how to repair the case (although I might have refactored the line myself to begin with). Anyway, could we review this line together John (@jzmaddock)?

Adding limb_type to limb_type to get a double_limb_type was flagged by one of my many syntax checkers. And I agree with the flag.

I came up with the line below. Thoughts?

   const double_limb_type two_n_mod = static_cast<double_limb_type>(static_cast<double_limb_type>(1u) + static_cast<double_limb_type>(static_cast<limb_type>(~static_cast<limb_type>(0u) - mod) % mod));
ckormanyos commented 1 year ago

Or do we need to cast the bitwise not of zero like below?

   const double_limb_type two_n_mod = static_cast<double_limb_type>(static_cast<double_limb_type>(1u) + static_cast<double_limb_type>(static_cast<double_limb_type>(~static_cast<double_limb_type>(0u) - mod) % mod));

Or does it just not matter since the initialization is only in the lower-limb of the double-limb type anyway?

jzmaddock commented 1 year ago

In terms of correctness, mod must be >0 so (~static_cast<limb_type>(0u) - mod) is less than the largest representable value, so adding one (after the mod which can only reduce the value further or leave it the same) is fine and cannot overflow.

But to fix the lint checkers, this should be fine: const double_limb_type two_n_mod = static_cast<double_limb_type>(1u) + static_cast<double_limb_type>((~static_cast<limb_type>(0u) - mod) % mod);

ckormanyos commented 1 year ago

adding one (after the mod which can only reduce the value further or leave it the same) is fine and cannot overflow

Ah, yes. Thanks John

If it's ok then, I'll hammer in one different variation to add limb + limb = limb, then cast to double_limb. Then I'll PR it after clearing up another PR in the pipeline.

   const double_limb_type two_n_mod =
      static_cast<double_limb_type>
      (
         static_cast<limb_type>(1u + static_cast<limb_type>(static_cast<limb_type>(~static_cast<limb_type>(0u) - mod) % mod))
      );
jzmaddock commented 1 year ago

I suspect that will still trip up the lint tools, as I assume they're warning on the +1 addition?

ckormanyos commented 1 year ago

suspect that will still trip up the lint tools, as I assume they're warning on the +1 addition?

Ugghhh, yes, it was. Great insight.

I finally opted (am wanting to opt) for this:

   const double_limb_type two_n_mod =
      static_cast<double_limb_type>
      (
         static_cast<double_limb_type>(1u) + static_cast<limb_type>(static_cast<limb_type>(~static_cast<limb_type>(0u) - mod) % mod)
      );

Much ado (from my side) about a line, I know... But, hey, at least I'm not goinng on about auto type deduction (yet), ...