Enum types aren't validated
- Dominant language
- Python
- Stars
- 92
- Forks
- 30
- PR merge metrics
- No merged PRs in 30d
Description
## Background
Given the following definition:
```python
enum McuCommand:
ACK = 0x000000
NACK = 0x000001
BUSY = 0x000005
struct McuMessageHeader:
0 [+1] McuCommand reason
```
I would expect the generated C++ code to validate the `reason` field to make sure it is a valid enum value. Or at least I'd want to have a helper that could validate it for me.
```c++
std::array msg;
msg[0] = std::byte{static_cast(McuCommand::BUSY) + 1};
auto view = MakeMcuMessageHeaderView(&msg);
assert(!view.Ok()); // doesn't assert
```
## Issue
We appear to use `AllValuesAreOk` as our validator for `EnumView` types, for example:
```c++
template
inline typename ::emboss::support::EnumView<
/**/ ::McuCommand,
::emboss::support::FixedSizeViewParameters<24, ::emboss::support::AllValuesAreOk>,
typename ::emboss::support::BitBlock>, 24>>
GenericMcuMessageHeaderView::reason()
const;
```
This leads to, well, all values being ok.
## Possible fixes
I see this as a few incremental improvements depending on what we want to do.
### Generate a validator
Minmal of what we should do so that users can manually validate. Something like:
```c++
struct EnumTrait<::McuCommand> {
static constexpr bool is_valid(std::underlying_type_t<::McuCommand> value) {
switch(value) {
case ::McuCommand::ACK:
case ::McuCommand::NACK:
case ::McuCommand::BUSY:
return true;
default:
return false;
}
}
}
```
### Add some min and max helpers
It might be useful for quick checks to include a min and max value. Something like:
```c++
struct EnumTrait<::McuCommand> {
static constexpr std::underlying_type_t<::McuCommand> min = ::McuCommand::ACK;
static constexpr std::underlying_type_t<::McuCommand> max = ::McuCommand::BUSY;
static constexpr bool is_valid(std::underlying_type_t<::McuCommand> value) {
if (value < min || value > max) { return false; }
//
}
}
```
### Add a proper validator
We could also generate a proper validator that will work with emboss views:
```c++
struct EnumValidator {
template
static constexpr bool ValueIsOk(ValueType value) {
// add some checks that the value would fit in the underlying type
return EnumTrait::is_valid(static_cast>(value));
}
};
```
### Hook the validator in
We could go further and use the validator by default.
*Pros:* This is the right thing to do
*Cons:* This might cause runtime assertions for existing code.
Something like:
```c++
template
inline typename ::emboss::support::EnumView<
/**/ ::McuCommand,
::emboss::support::FixedSizeViewParameters<24, EnumValidator>,
typename ::emboss::support::BitBlock>, 24>>
GenericMcuMessageHeaderView::reason()
const;
```
Contributor guide
Assessment
This issue has not been assessed yet.