o3tl::make_unsigned

Stephan Bergmann sbergman at redhat.com
Wed Jan 29 09:45:54 UTC 2020


In C/C++, if you compare an expression e1 of signed integral type 
against an expression e2 of unsigned integral type like

   sal_Int32 e1 = ...;
   sal_uInt32 e2 = ...;
   if (e1 < e2) ...    // (A)

compilers often emit warnings that you are mixing signed and unsigned 
operators in a comparison (which likely does not do what is naively 
expected, due to integral promotions performed on the operands).

That is why the above code (A) is often written something like

   if (sal_uInt32(e1) < e2) ...    // (B)

Which has the drawback that if the type of e1 ever changes (to 
sal_Int64, say), the cast-to-silence-a-warning may accidentally stay 
around, silently truncating large values.  Another drawback is that it 
is not obvious whether the cast is there to silence a warning, or serves 
another purpose (to reinterpret signed values as unsigned, say; see below).

Enter 
<https://gerrit.libreoffice.org/plugins/gitiles/core/+/6417668b3e12d9659ac5dc4a2f60aa8ad3bca675%5E!/> 
"Introduce o3tl::make_unsigned to cast from signed to unsigned type". 
If e1 is known to be non-negative, it lets you write

   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). 
<https://gerrit.libreoffice.org/plugins/gitiles/core/+/aef7feb3e695ecf6d411f0777196dcc4281e201a%5E!/> 
"New loplugin:unsignedcompare" added a Clang plugin that uses some 
heuristics to find patterns like (B) and asks to rewrite them as (C). 
There is a slight chance that the heuristics fail and the cast-from e1 
may legitimately be negative, and the cast to an unsigned type was 
actually meant to reinterpret the signed value as an unsigned one, 
rather than to silence a signed-vs.-unsigned warning.

<https://gerrit.libreoffice.org/plugins/gitiles/core/+/0288c8ffecff4956a52b9147d441979941e8b87f%5E%21/> 
"Rephrase cast from sal_Int32 to sal_uInt32" is the only such case I 
identified where the loplugin:unsignedcompare heuristics would have 
failed.  But I may of course have missed others, and introduced uses of 
o3tl::make_unsigned in inappropriate places.  So if you encounter a 
failed assert from o3tl::make_unsigned, this is the place to look.



More information about the LibreOffice mailing list