Date: prev next · Thread: first prev next last
2020 Archives by date, by thread · List index


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.


Context


Privacy Policy | Impressum (Legal Info) | Copyright information: Unless otherwise specified, all text and images on this website are licensed under the Creative Commons Attribution-Share Alike 3.0 License. This does not include the source code of LibreOffice, which is licensed under the Mozilla Public License (MPLv2). "LibreOffice" and "The Document Foundation" are registered trademarks of their corresponding registered owners or are in actual use as trademarks in one or more countries. Their respective logos and icons are also subject to international copyright laws. Use thereof is explained in our trademark policy.