Welcome to Software Development on Codidact!

Will you help us build our independent community of developers helping developers? We're small and trying to grow. We welcome questions about all aspects of software development, from design to code to QA and more. Got questions? Got answers? Got code you'd like someone to review? Please join us.

Review Suggested Edit

You can't approve or reject suggested edits because you haven't yet earned the Edit Posts ability.

The core problem that this rule is supposed to address is that 8 bit arithmetic actually doesn't exist in C. All expressions are promoted to larger integer types and that's a language flaw one has to be aware of. The rule also seeks to prevent accidental overflows like `my_uint8 = 1u << 15;`. But it's not really meant to block out idiomatic C code like assigning an integer constant expression to a`uint8_t` variable using `=` (simple assignment).
Therefore 10.3 comes with exceptions (MISRA-C:2012+Amendments/MISRA-C:2023):
> 1. An _essentially signed integer constant expression_, with a rank no greater than signed int, may be assigned to an object of _essentially unsigned type_ if its value can be represented in that type.
This is meant to cover cases like `my_uint8 = 1;` but should apply in your case too.
- `1u << BIT0` is essentially unsigned. The type of `BIT0` is actually not relevant in determining the type of the expression, since the type of a shift expression is always that of the (promoted) left operand.
Had you written `1 << BIT0` then it would have been essentially signed, but obviously we shouldn't do that since that opens up for signed bitwise arithmetic and would therefore violate a bunch of other MISRA rules.
So the exception really ought to cover cases of _essentially unsigned integer constant expressions_ too - I'd regard this as a minor defect in the MISRA guidelines.
- `1u << BIT0` is an integer constant expression, given that `BIT0` is just some `#define` (and not a variable etc).
- `uint8_t` is essentially unsigned.
- Given that `BIT0` is one of the numbers 0 to 7, it can fit just fine in a `uint8_t`.
If all of the above is true and your code had actually been `some_register = (1u << BIT0);`, then you should be able to claim MISRA compliance without deviations. The explicit cast is not required, but doesn't cause any harm either.
__However__, that would be true if you used "simple assignment", plain old `=`. You are using _compound assignments_ such as `|=`. Compound assignments are intrinsically problematic for type safety as they always come with implicit promotions and that's the actual root of the problem here.
`some_register |= (1u << BIT0)` is equivalent to
`some_register = some_register | (1u << BIT0);`
Except `some_register`, which is probably `volatile` qualified in a real application, is guaranteed to only be evaluated once during compound assignment. Still, the above means we are no longer dealing with an integer constant expression - `some_register | (1u << BIT0)` is run-time evaluated.
And what's worse, the resulting type of it is _not_ `uint8_t` but `unsigned int`, thanks to the implicit promotion caused by the `|`. Subtle promotions like that are dangerous and something MISRA tries to stomp out. In this specific case the promotion is harmless, but in other cases, perhaps not.
So actually, if you use compound assignment the cast achieves nothing. Because you'll end up writing code equivalent to
some_register = some_register | (uint8_t)(1u << BIT0);
And that doesn't prevent the implicit promotion of `|` - there is no way to prevent it. There's not really a fix to it, this is a problem in the C language itself. While both operands of this temporary expression are "essentially unsigned" there are actual no problems in the code - if they aren't, then there are problems and there exists no way to fix them.
**So the best practice is actually IMO to avoid compound assignments altogether, due to their built-in type promotion problems.**
Furthermore on the hardware level, you can't be quite sure what machine code you end up with when doing compound assignment - do you get a read-modify-write instruction on the assembler level or do you get actual read, then modify, then write separate instructions on a higher level (non-atomic)? If it does the wrong thing, it might cause hiccups if dealing with various oddball flag/status registers in microcontroller peripherals. In case writing 1 clears flags etc, then compound assignment wouldn't work anyway and you'd have to use simple assignment, or otherwise you'd nuke every set flag in the register rather than preserving it.
In your specific case, there was never a need to use compound assignment in the first place - just use simple assignment and overwrite the old register value. In case you do need to preserve various flags, maybe consider doing it in a more explicit way:
```c
uint8_t some_register = SOME_REGISTER; // read actual hw register to RAM
~~some_register = (uint8_t)(some_register | (1u << BIT0); // prep the contents in RAM~~
SOME_REGISTER = some_register; // write to hw register
```
This sates other MISRA rules such as not accessing a `volatile` variable twice within an expression. Is it less readable? Not really. Is it more verbose? Indeed. But it shows that "hey I actually know what I'm doing, I'm not just aimlessly typing C code with no clue of what types I'm even using".
---
Other remarks:
- `static_cast<uint8_t>` is absolutely not compliant since that's C++. If you got a tool telling you otherwise, you got some serious tooling problems. MISRA C:2012/2023 requires ISO C90, C99 or C11. You cannot use C++, you can not use non-standard extensions like GNU C etc etc.

The core problem that this rule is supposed to address is that 8 bit arithmetic actually doesn't exist in C. All expressions are promoted to larger integer types and that's a language flaw one has to be aware of. The rule also seeks to prevent accidental overflows like `my_uint8 = 1u << 15;`. But it's not really meant to block out idiomatic C code like assigning an integer constant expression to a`uint8_t` variable using `=` (simple assignment).
Therefore 10.3 comes with exceptions (MISRA-C:2012+Amendments/MISRA-C:2023):
> 1. An _essentially signed integer constant expression_, with a rank no greater than signed int, may be assigned to an object of _essentially unsigned type_ if its value can be represented in that type.
This is meant to cover cases like `my_uint8 = 1;` but should apply in your case too.
- `1u << BIT0` is essentially unsigned. The type of `BIT0` is actually not relevant in determining the type of the expression, since the type of a shift expression is always that of the (promoted) left operand.
Had you written `1 << BIT0` then it would have been essentially signed, but obviously we shouldn't do that since that opens up for signed bitwise arithmetic and would therefore violate a bunch of other MISRA rules.
So the exception really ought to cover cases of _essentially unsigned integer constant expressions_ too - I'd regard this as a minor defect in the MISRA guidelines.
- `1u << BIT0` is an integer constant expression, given that `BIT0` is just some `#define` (and not a variable etc).
- `uint8_t` is essentially unsigned.
- Given that `BIT0` is one of the numbers 0 to 7, it can fit just fine in a `uint8_t`.
If all of the above is true and your code had actually been `some_register = (1u << BIT0);`, then you should be able to claim MISRA compliance without deviations. The explicit cast is not required, but doesn't cause any harm either.
__However__, that would be true if you used "simple assignment", plain old `=`. You are using _compound assignments_ such as `|=`. Compound assignments are intrinsically problematic for type safety as they always come with implicit promotions and that's the actual root of the problem here.
`some_register |= (1u << BIT0)` is equivalent to
`some_register = some_register | (1u << BIT0);`
Except `some_register`, which is probably `volatile` qualified in a real application, is guaranteed to only be evaluated once during compound assignment. Still, the above means we are no longer dealing with an integer constant expression - `some_register | (1u << BIT0)` is run-time evaluated.
And what's worse, the resulting type of it is _not_ `uint8_t` but `unsigned int`, thanks to the implicit promotion caused by the `|`. Subtle promotions like that are dangerous and something MISRA tries to stomp out. In this specific case the promotion is harmless, but in other cases, perhaps not.
So actually, if you use compound assignment the cast achieves nothing. Because you'll end up writing code equivalent to
some_register = some_register | (uint8_t)(1u << BIT0);
And that doesn't prevent the implicit promotion of `|` - there is no way to prevent it. There's not really a fix to it, this is a problem in the C language itself. While both operands of this temporary expression are "essentially unsigned" there are actual no problems in the code - if they aren't, then there are problems and there exists no way to fix them.
**So the best practice is actually IMO to avoid compound assignments altogether, due to their built-in type promotion problems.**
Furthermore on the hardware level, you can't be quite sure what machine code you end up with when doing compound assignment - do you get a read-modify-write instruction on the assembler level or do you get actual read, then modify, then write separate instructions on a higher level (non-atomic)? If it does the wrong thing, it might cause hiccups if dealing with various oddball flag/status registers in microcontroller peripherals. In case writing 1 clears flags etc, then compound assignment wouldn't work anyway and you'd have to use simple assignment, or otherwise you'd nuke every set flag in the register rather than preserving it.
In your specific case, there was never a need to use compound assignment in the first place - just use simple assignment and overwrite the old register value. In case you do need to preserve various flags, maybe consider doing it in a more explicit way:
```c
uint8_t some_register = SOME_REGISTER; // read actual hw register to RAM
some_register = (uint8_t)(some_register | (1u << BIT0)); // prep the contents in RAM
SOME_REGISTER = some_register; // write to hw register
```
This sates other MISRA rules such as not accessing a `volatile` variable twice within an expression. Is it less readable? Not really. Is it more verbose? Indeed. But it shows that "hey I actually know what I'm doing, I'm not just aimlessly typing C code with no clue of what types I'm even using".
---
Other remarks:
- `static_cast<uint8_t>` is absolutely not compliant since that's C++. If you got a tool telling you otherwise, you got some serious tooling problems. MISRA C:2012/2023 requires ISO C90, C99 or C11. You cannot use C++, you can not use non-standard extensions like GNU C etc etc.

Communities

Review Suggested Edit