o3tl::make_unsigned
Luboš Luňák
l.lunak at collabora.com
Wed Jan 29 10:41:15 UTC 2020
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 at collabora.com
More information about the LibreOffice
mailing list