C++ integral promotions and sonar rules

I’m testing SonarCloud rules related to signed/unsigned conversion on a simple C++ project on GitHub. It seems that it doesn’t in some cases when integral promotion takes effect.

E.g. rule s845 - Signed and unsigned types should not be mixed in expressions:

official example which works:

int32_t a = 10;
const uint32_t b = a; // noncompliant
const int32_t c = (a > 0) ? a : b; // noncompliant

example with smaller integral types, like unsigned short, which doesn’t work:

const int32_t a = 10;
const uint16_t b = 11;
const int32_t c  = (a > b) ? a : b; // noncompliant - doesn't work probably due to integral promotions?

It seems that Sonar works with the types after implicit conversions / integral promotion is applied → the uint16_t b automatically becomes int in the expression (a > b) ? a : b, the whole expression results in int which matches the resulting variable int32_t c.

But still we are mixing signed / unsigned types which breaks some MISRA rules which we would like to catch using Sonar.

Please see more more examples in my simple Sonar project. Is this expected behavior?

Also note that e.g. rule s874 doesn’t fire for a code from original example.

Hello @Mi-La,

If, in your second example, you initialize a with a non-constant value, you’ll get an additional issue for the comparison:

Use “std::cmp_greater” to compare this potentially negative value with an unsigned value
(Note: This will only be triggered if you compile at least in C++20 mode).

As of now, we have not implemented an exact equivalent to MISRA rule 7.0.5 that would make this code non-compliant. It is on our radar, but not planned yet.

This rule is not enabled by default. Did you add it to your quality profile?

Hello!

Thanks for the response.

Regarding to the rule s845 - Signed and unsigned types should not be mixed in expressions, there is a note:

“This rule will detect implicit conversions that change the signedness.”

I believe that “promotions of integral types” are also “implicit conversions”, and when uint16_t is implicitly promoted to int, it changes signedness. So maybe at least the generic description of the rule should be changed? Or a warning that integral promotions are not considered could be added?

This rule is not enabled by default. Did you add it to your quality profile?

Yes, I believe it’s enabled in my quality profile.

You are right that S845 is not behaving as documented. It tries to do more advanced heuristics to avoid having too many false positives, but falls short in some cases. I created a ticket to improve this situation.

For S874, I looked more deeply at your example, and if at first I could reproduce the issue, after 2 changes, I could make it work as expected:

  • I made sure to compile in C++17, since the example is compliant in C++20,
  • I replaced the a and b constants with variables of unknown values. The rule has an exception when the values are constants.
static void s874(uint16_t a, int16_t b)
{
  if (( a & b ) == 0x1234U) // noncompliant until C++20, correctly reported
  {
      // empty
  }
  if (~b == 0x1234U) // noncompliant until C++20, correctly reported
  {
      // empty
  }
  (void)s874_f(b);
}

Thanks for the ticket and for the explanation of S874. I will try to use variables of unknown values in my further tests, it makes sense.