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

Virtual modifier explicit mapping is limited to platform-dependent int size #594

Closed
wismill opened this issue Jan 15, 2025 · 0 comments · Fixed by #605
Closed

Virtual modifier explicit mapping is limited to platform-dependent int size #594

wismill opened this issue Jan 15, 2025 · 0 comments · Fixed by #605
Labels
compile-keymap Indicates a need for improvements or additions to keymap compilation
Milestone

Comments

@wismill
Copy link
Member

wismill commented Jan 15, 2025

Originally posted by @wismill in #450 (comment)

          @mahkoh Good point, setting `virtual_modifiers` using numeric values is also possible, although this is quite an obscure setting IMHO.

There is a limitation though: the numeric values ExprInteger::ival are int and depending on the target compiler/platform it may not be big enough to hold the mask. Agreed, nowadays on most cases we should have sizeof(int) * CHAR_BIT >= 32, but the C standard guarantee only 16 bits. Such overflow would be completely silent and difficult to debug. Note that overflow is still possible and silent with the current implementation with 32-bits ints: e.g. virtual_modifiers PureVirtual = 0xffffffff + 1;

So we may want to switch ExprInteger::ival from int to the explicit int32_t, which should be plenty enough.

@wismill wismill added the compile-keymap Indicates a need for improvements or additions to keymap compilation label Jan 15, 2025
@wismill wismill added this to the 1.9.0 milestone Jan 17, 2025
@wismill wismill modified the milestones: 1.9.0, 1.11.0, 1.10.0 Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compile-keymap Indicates a need for improvements or additions to keymap compilation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant