o3tl::make_unsigned

Stephan Bergmann sbergman at redhat.com
Wed Jan 29 13:04:26 UTC 2020


On 29/01/2020 11:41, Luboš Luňák wrote:
> 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

There is no assumptions here?  o3tl::make_unsigned can be used iff its 
argument is known to be non-negative.  (Which is quite often the case, 
e.g., because there is a preceding

   if (e1 >= 0) ...

or because it is the result of OUString::getLength etc.)

> casting to unsigned, which silently can make valid comparisons change result,
> your wrapper will just detect and abort on that. Technically it's the

As you note in the following sentence, there are cases where there is no 
real alternative to promoting the comparison to an unsigned type.

> 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.

Often, the unsigned types come from APIs we do not control.



More information about the LibreOffice mailing list