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@xxxxxxxxxxxxx _______________________________________________ LibreOffice mailing list LibreOffice@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/libreoffice