Search code examples
cmisracompound-assignment

MISRA: Compound Assignment operators

I have taken over code that has a lot of compound assignment operators in it. I think that compound operators are not 'really' MISRA compliant. I can't seem to find any reference to them.

I believe I understand what is happening and that it should actually be separated.

UINT16 A |= B; /* Incorrect */
UINT16 A = (UINT16)((UINT32)A | (UINT32)B); /* Correct */
UINT16 A <<= 1u; /* Incorrect */
UINT16 A = (UINT16)((UINT32)A << (UINT32)1u); /* Correct */

So, my questions are:

  1. Does MISRA frown upon compound assignments?
  2. Is there any kind of quick fix, instead of ignoring the warning?

Thank you,

Solution

  • Does MISRA frown upon compound assignments?

    Not as such. The rules for compound assignment are similar to the rules for simple assignment. MISRA speaks of assignment operators in general times, including all of them.

    MISRA does however frown upon implicit type promotions, see Implicit type promotion rules. You can't understand these MISRA warnings unless you understand implict promotions.

    Is there any kind of quick fix, instead of ignoring the warning?

    You can't really fix this without understanding the warning. The only quick fix is to only use uint32_t everywhere and never signed or small integer types, but that isn't always feasible. Your original code would have been compliant if all variables were uint32_t.

    In general, the latest MISRA only allows various conversions between types of the same essential type category. Unsigned integers is one such category, signed integers in another, and so on.

    It isn't easy to tell how exactly your code violates MISRA without knowing the type of B and sizeof(int). This has nothing to do with compound assignment as such, except that compound assignment operators are a bit cumbersome to work with when it comes to implicit promotions.

    MISRA frowns upon assigning the value of an expression (after implicit promotion) to a narrower essential type of the same category or to a different category. In plain English you shouldn't for example assign the result of a uint32_t operation to a uint16_t variable or a signed result to an unsigned variable. This is most often fixed with casting at appropriate places.

    Regarding your specific examples, assuming B is uint16_t and the CPU is 32 bit, you get problems with implicit type promotion.

    Since A |= B is equivalent to A | B, the usual arithmetic conversions will promote the operands to int in case of 32 bit CPU. So it is a signed 32 bit type.

    Assume you had A << 31u - then this would actually have invoked an undefined behavior bug, which the rule seeks to protect against.

    Sufficient fixes for MISRA-C compliance:

    A = (uint16_t) (A | B); // compliant
A = (uint16_t) ((uint32_t)A << 1u) // compliant