◐ Shell
clean mode source ↗

bpo-29882: Fix portability bug introduced in GH-30774 by mdickinson · Pull Request #30794 · python/cpython

@vstinner This is already explained in the long comment that you didn't review. :-)

The problem here isn't the constant; it's the type declaration.

For the multiplication in the last line of the function to be portable, we need at least one of the unsigned operands in that multiplication to not be promoted to int. An inline constant 0x01010101U satisfies that criterion: by C99 §6.4.4.1, together with C's guarantees about the minimum precision of long, it has type either unsigned int or unsigned long. A uint32_t constant with the same value does not satisfy that criterion, for reasons already explained.

And there isn't a type declaration that works here. I just explained why uint32_t won't work. If we declare SUM as unsigned int instead of uint32_t, we have portability issues on machines where int isn't large enough to represent the value 0x01010101. If we declare it as unsigned long, we're back to doing a 64-bit-by-64-bit multiply on almost all current Linux and macOS boxes.

If you really want to keep the constant, another option is to leave this line exactly as-is and change the multiplication in the last line to x * (SUM + 0U); that + 0U effectively forces the second multiplicand to have type with rank greater than or equal to that of int, making it immune to further integer promotion.

But as Tim observed in the #30774 discussion, none of this prevents us from potentially doing a 512-bit-by-512-bit multiply on a box that has 512-bit integers. But short of relying on compiler-specific intrinsics, that's inescapable anyway: standard C simply isn't capable of doing arithmetic on anything smaller than an int.