Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adaptive generated enumeration storage width #7394

Closed
gjc13 opened this issue Jun 4, 2021 · 5 comments · Fixed by #7746
Closed

Adaptive generated enumeration storage width #7394

gjc13 opened this issue Jun 4, 2021 · 5 comments · Fixed by #7746

Comments

@gjc13
Copy link
Contributor

gjc13 commented Jun 4, 2021

Problem

Currently the zap generated enumerations doesn't specify the storage width or must have unified storage width.

Proposed Solution

Modify the ZAP generation engine to generate the storage width based on the enumeration values.

@wqx6
Copy link
Contributor

wqx6 commented Jun 15, 2021

Maybe we can use enum class ENUM_TYPE :uintx_t when defining the enumeration and use Put(...,static_cast<std::underlying_type_t<ENUM_TYPE>>) when calling the enumeration. like https://en.cppreference.com/w/cpp/types/underlying_type. It seems a safe solution, but it needs a lot of changes at where using the enumeration.

@bzbarsky-apple
Copy link
Contributor

Using enum class is orthogonal to the problem of figuring out what the right underlying type is for an enum....

Ideally that would just come from the spec or some other data source and feed into ZAP.

@tecimovic @vivien-apple

@jmeg-sfy
Copy link
Contributor

jmeg-sfy commented Jun 18, 2021

@bzbarsky-apple this is something i would like to explicity use and define in my xml file. casting is not a solution and is prone to runtime problem . what is the policy regarding enum and packed structure
Also can we enforce preprocessing compiler checks on ZAP using static_assert (sizeof(MyStruct_t) == sizeof(uint8_t)

@bzbarsky-apple
Copy link
Contributor

@jmeg-sfy Sorry for the lag; I hope #7746 does what you wanted...

@jmeg-sfy
Copy link
Contributor

@jmeg-sfy Sorry for the lag; I hope #7746 does what you wanted...

yes all is solved now thanks !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants