2

I am writing a code under the MISRA rule. I am getting a MISRA error for the below expression

check_Val = ( ~( 0x000Fu << Src_Data ) ); //where Src_Data is uint8 and check_Val is uint32.

I analyzed the error

Violates MISRA 2004 Required Rule 10.1, Implicit conversion of complex integer expression

Since check_Val is uint32, the Lf should fit on it. Then why it is giving an error.

LPs
  • 16,045
  • 8
  • 30
  • 61
Cool_Binami
  • 93
  • 4
  • 12
  • what tool gives you this output ? – amdixon Oct 05 '15 at 13:49
  • Possible duplicate of [Bitshift and integer promotion?](http://stackoverflow.com/questions/3482262/bitshift-and-integer-promotion) – Tom Tanner Oct 05 '15 at 13:52
  • 1
    @TomTanner How can it be a duplicate if it doesn't mention MISRA at all? – Lundin Oct 05 '15 at 13:53
  • 2
    You can't know that it "should" fit, since the value of `Src_Data` could be anything up to 255, which would make the shift very large indeed (which doesn't work, and is probably related to the flagging). – unwind Oct 05 '15 at 13:54
  • Probably operation is performed promoting elements to `signed int`. That's why `MISRA` is warning you. – LPs Oct 05 '15 at 13:54
  • @Lundin the misra rule is getting upset because of implicit promotions, that's why – Tom Tanner Oct 05 '15 at 13:55
  • 1
    Tool gives this out put is PC-lint – Cool_Binami Oct 05 '15 at 13:55
  • @TomTanner What implicit promotion? There is no implicit promotion in this example, given that int is 32 bits. – Lundin Oct 05 '15 at 13:56
  • @Lundin: Where is stated that `int` is 32 bits? – too honest for this site Oct 05 '15 at 14:00
  • To validate unwind comment you could simply change your Src_Data var with a constant, eg. 7. – LPs Oct 05 '15 at 14:01
  • also int != unsigned int – Tom Tanner Oct 05 '15 at 14:04
  • Firstly uint8 is usually a typedef of unsigned char. Since left shift requires an int value as its right operand you have a type promotion which MISRA would be unhappy about. Secondly what is the word size of the architecture? If you are on an 8-bit platform uint32 would be a type unsigned long but the result of the expression is only 16 bits in size. – Kenneth Oct 05 '15 at 14:05
  • So I looked this up. Disregard the whole implicit promotion issue, this is an artificial problem created by the old MISRA:2004 standard. The tool is correct, regardless of what implicit promotions that actually occur in the code. See my answer for details. – Lundin Oct 05 '15 at 14:39

1 Answers1

3

Rule 10.1 is concerned with implicit promotion of integers, by using the MISRA:2004 concept of "underlying type", that is, the intended type of the expression. It was a weird concept which lead to numerous superfluous casts, this has been fixed in MISRA:2012.

The underlying type of your expression is that of the integer literal 0x000Fu. For integer literals it stated that underlying type is the smallest possible type that is capable of representing the object (see p40-41). So the underlying type of 0x000Fu will be uint8_t, because it can fit in a byte and it is of unsigned type.

Even though, as far as the C language and the compiler are concerned, that integer literal is actually of type unsigned int and no promotions will occur. MISRA:2004 doesn't care.

Meaning that you have to cast the operand to uint32_t before the operation (to bypass rule 10.1) or cast it to uint8_t after the operation (to sate 10.1 and also 10.5 regarding shifts). I suggest you cast it to uint32_t to get rid of the problem.

Fixed code should therefore be

check_Val = ~( (uint32_t)0x000Fu << Src_Data );

Also, since both operands of a shift operator are integer promoted, the right-hand operator gets implicitly promoted to int as well. That promotion can never cause harm, and I'm not sure it is even a MISRA violation, but I suppose it could be causing warnings too. In that case, you would also have to cast the right operand:

check_Val = ~( (uint32_t)0x000Fu << (uint32_t)Src_Data );

I would recommend to use MISRA:2012 if possible, where these rules have been clarified and the "underlying type" concept has been replaced by one which doesn't lead to so many pointless casts.

Lundin
  • 195,001
  • 40
  • 254
  • 396
  • It's a way to specify a constant of the appropriate size and value. Though it does apparently create a value of the type `uint_least32_t` rather than `uint32_t`, so I'm not sure MISRA will accept it. – EOF Oct 05 '15 at 14:43
  • @EOF In that case it would work, although I would avoid icky macros when possible. They will also cause numerous other MISRA issues, since MISRA doesn't like function-like macros at all, for very sound reasons. – Lundin Oct 05 '15 at 14:48