Re: o3tl::make_unsigned

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

_______________________________________________
LibreOffice mailing list
LibreOffice@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/libreoffice




[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]     [Photo]

  Powered by Linux