Re: o3tl::make_unsigned

On Wednesday 29 of January 2020, Stephan Bergmann wrote:
>    if (o3tl::make_unsigned(e1) < e2) ...    // (C)
>
> instead of (B), avoiding an explicit cast and making the intent clear.
> (o3tl::make_unsigned is defined "header-only", so can be used everywhere
> LIBO_INTERNAL_ONLY is defined.)
>
> The caveat is that e1 must be known to be non-negative (and
> o3tl::make_unsigned asserts that).

 That is quite a caveat, to assume that such comparisons of such two values 
never include one of them being negative. I think the problem is already in 
casting to unsigned, which silently can make valid comparisons change result, 
your wrapper will just detect and abort on that. Technically it's the 
unsigned value that should be cast to signed, of higher resolution if needed 
(which it often is not, given that these often come from things like 
containers operating on size_t that never grow big enough to hold values that 
do not fit plain int).

 But IM(NSH)O this is just patching over a fundamental problem, namely the 
problem of using unsigned types in the first place. There are only few cases 
where nowadays unsigned types make sense (bitfields, hex values, binary 
formats), everything else comes from the ancient times where an extra bit 
actually made a difference. Trying to squeeze in one extra bit in 31bits or 
even 63bits just doesn't make sense these days. And trying to use unsigned 
types because it "semantically makes sense" quickly leads to them getting 
compared to something signed somewhere and thus all these stupid C promotion 
rules.

 The real fix to these problems is simply to use int, unless there's a good 
reason to use something else. If the idea is to fix the codebase, the idea 
may be just as well to fix it in a way that doesn't just paper over. So while 
I like the idea of fixing this, I disagree with the direction taken, both 
short-term and long-term.

-- 
 Luboš Luňák
 l.lunak@collabora.com