o3tl::make_unsigned

Stephan Bergmann sbergman at redhat.com
Wed Jan 29 14:40:55 UTC 2020


On 29/01/2020 15:20, Luboš Luňák wrote:
> On Wednesday 29 of January 2020, Stephan Bergmann wrote:
>> 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 the assumption. Using o3tl::make_signed would not require such an
> assumption, or it would be even less likely false.

But a precondition-free o3tl::make_signed would need to map an unsigned 
type to a wider signed types, which need not exist.

>> (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.)
> 
>   That still requires reading the code, and people may skip that and simply use
> the function because "that's how it's done" and then miss corner cases.
> 
>>
>>> 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.
> 
>   Where? A signed type can always hold all values of an unsigned type, if large
> enough, but it's not true the other way around, so it's the signed types that
> should be preferred.

The "if large enough" is the hard part.

>>> 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).
> 
>   This IOW says that if you cast the unsigned to signed, you're less likely to
> change the value than if you cast signed to unsigned. A negative value in
> these comparisons may be unlikely, but unsigned values with the highest bit
> used are extremely unlikely.

Why resort to code that likely works, when we can write code that is 
guaranteed to work?

>   For reference, both Java's and C#'s List classes use int for size and index
> types, and use long for file size and position types. Apparently it does the
> job.

Sure.  But what we are faced with here are C/C++ APIs that use unsigned 
types, and we have to interoperate with those.

>   Reading my first mail I probably wasn't clear on what it is that I want, so
> let me clarify: What I'm saying is that there should be o3tl::make_signed()
> instead of make_unsigned(). That'll both move us towards the better types,
> and it'll be safer - it's less likely to assert on unsigned type having the
> highest bit set than on a signed type being negative, and it doesn't need
> analyzing the code the way >=0 does.

You mean, an o3tl::make_signed that maps from an unsigned type to the 
signed type of the same rank?  What would its precondition be, require 
that the given value is sufficiently small?  Typically not being able to 
guarantee that statically, code would then need to first check for '<= 
std::numeric_limits<T>::max()' before being able to call o3tl::make_signed?
>>>    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.
> 
>   That's unfortunate, but just because we can't be perfect that doesn't mean we
> can't be at least moving in that direction, instead of moving in the wrong
> direction that cannot even get close to being perfect. You can't write code
> without signed types, but you can without unsigned[*], and there will always
> be more signed variables, and converting unsigned->signed is safer => we
> should prefer signed whenever possible.
> 
> [*] Barring the bitfield etc. stuff that's not relevant here.

I still fail to see how converting from unsigned to signed is generally 
possible, leave alone safe.



More information about the LibreOffice mailing list