From: Linus Torvalds > Sent: 28 July 2024 17:57 > > On Sun, 28 Jul 2024 at 07:21, David Laight <David.Laight@xxxxxxxxxx> wrote: > > > > +/* Allow if both x and y are valid for either signed or unsigned compares. */ > > +#define __types_ok(x, y) \ > > + ((__is_ok_signed(x) && __is_ok_signed(y)) || \ > > + (__is_ok_unsigned(x) && __is_ok_unsigned(y))) > > This seems horrendous, exactly because it expands both x and y twice. > And the "expand multiple times" was really the fundamental problem. This version is better than the previous one ;-) > Why not just change the model to say it's a bitmask of "signedness > bits", the bits are "signed ok" and "unsigned ok", and turn it into > > /* Signedness matches? */ > #define __types_ok(x, y) \ > (__signedness_bits(x) & __signedness_bits(y)) Something like that might work, but it would take some effort to get right. It would be better to remove the 'low hanging fruit' of min(pointer_type) and the places where a constant is needed first. Both those require extra expansions and tend to make it all that much harder. > and __signedness_ok() simply does something like "1 if unsigned type, > 2 if signed type, 3 if signed positive integer". > > Something like (very very handwavy, very very untested): > > __builtin_choose_expr(is_signed_type(typeof(x)), > 2+__if_constexpr(x,(x)>0,0), > 1) You'd want to test '(x) >= 0' and the compiler is going to bleat (with -Wall) if (x) is an unsigned type - even though the code isn't used. Neither __builtin_choose_expr() or _Generic() help with that. Unless you need the types to differ ?: is just as good. > Actually, I think that "__if_constexpr()" could very well be "if known > positive value", ie 'x' itself doesn't have to be constant, but "x>0" > has to be a constant (the difference being that the compiler may be > able to tell that some variable is always positive, even if it's a > variable): > > #define statically_true(x) __builtin_constant_p((x),(x),0) > #define is_positive_value(x) statically_true((x)>=0) I think that test could be done on __x (ie the local copy). But then you can't use static_assert() and get a sane error message. (But don't look at what clang outputs...) > and then use > > __builtin_choose_expr(is_signed_type(typeof(x)), > 2+is_positive_value(x), 1) > > and yes, I realize I count zero as a positive value, but writing out > "nonnegative()" is annoying and we never care. I got annoyed earlier :-) > > I guess we could say "is_unsigned_value()"? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)