localspook wrote: Okay, I had to spend a good bit of time thinking about whether hardcoding the cast to `int` (as opposed to some other type) is correct. Here's what I found:
- The operands to a shift undergo _integral promotions_ [expr.shift], [conv.prom] - The type of a shift expression is the type of its LHS after promotions (the RHS is irrelevant) - We limit the check to only analyze cases where the LHS has unsigned integer type, so the only integral promotion rule that applies is [conv.prom]/1: > A prvalue of an integer type other than bool, char8_t, char16_t, char32_t, or wchar_t whose integer conversion rank (6.8.6) is less than the rank of int can be converted to a prvalue of type int if int can represent all the values of the source type; otherwise, the source prvalue can be converted to a prvalue of type unsigned int. So, *technically*, on some wack 16-bit architecture where `sizeof(unsigned short) == sizeof(unsigned int)`, the check would fail to preserve the type of the source expression even with `HonorIntCast` enabled: ```cpp unsigned short n; n << 10 | n >> 6; // <-- Type is promoted to 'unsigned int' (with no widening) // Becomes std::rotl(n, 10); // <-- Type is changed to 'unsigned short' ``` Now, *I* don't personally care about this fringe case and the change LGTM as-is, but in case someone does, I wanted to post this so we can make the decision with open eyes. https://github.com/llvm/llvm-project/pull/186324 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
