On Sun, Mar 8, 2026 at 12:26 AM Collin Funk <[email protected]> wrote:
> Jeffrey Walton <[email protected]> writes: > > > On Sat, Mar 7, 2026 at 11:33 PM Paul Eggert <[email protected]> wrote: > > > >> On 2026-03-07 18:57, Jeffrey Walton wrote: > >> > You may want to put an assert in the rotates to ensure the rotate > amount > >> is > >> > less than the bit size. > >> > >> C2y says that these functions don't have that restriction: if the number > >> of bits is N, then rotating by nonnegative R is equivalent to rotating > >> by R % N. So there's no need for an assertion to check that the shift > >> argument is in range. > >> > > > > If that is the case, then the masking is not needed: > > > > +# if ! defined __gl_stdc_rotate_left > > +# define __gl_stdc_rotate_left(value, count) \ > > + ((value << (count & (sizeof value * 8 - 1))) \ > > + | (value >> ((-count) & (sizeof value * 8 - 1)))) > > +# endif > > ISO C 2y § 6.5.8 "Bitwise shift operators" says: > > If the value of the right operand is negative or is greater than or > equal to the width of the promoted left operand, the behavior is > undefined. > > If you remove the masks and then run ./configure with > CFLAGS='-fsanitize=undefined', you will see test-stdc_rotate_left and > test-stdc_rotate_right will fail. I added test cases for shifts 0 to 2 > times the width of values. > Right. Paul claims it is not needed. > >> Unless you're thinking we should have an assert that the computation > >> internally doesn't shift by more than that? If so, I disagree: it's > >> obviously in range and we don't want to be burdened down by the > >> machinery of 'assert'. > >> > > > > Yes. I like to use asserts to alert the caller they are using something > > incorrectly. It is a lot easier to let the debugger snap than look at a > > backtrace looking for a problem. > > I agree it isn't common, but there is probably some reason to want to > shift more than the width. I would rather not bug someone with that need > with failed assertions. If the shift or rotate is a constant value, then you risk having the code removed _if_ the shift is too large. That is, you are assuming GCC's behavior. Jeff
