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

Initial migration from const-generics to typenum #189

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Ben-PH
Copy link

@Ben-PH Ben-PH commented Dec 15, 2024

This is an initial PoC of what a migration to use of typenum and GenericArray might look like.

@thomasaarholt
Copy link

(Not the developer, just curious)

Could you expand on what the motivation for moving away from const generics is here?

@Ben-PH
Copy link
Author

Ben-PH commented Dec 15, 2024

typenum allows you to do things like specify legal values at compile time, as well as removes the need to choose an integer type until runtime.

For example, if you wanted to implement a "sub-matrix" extension trait, you could constrain it so its width and height is less than the matrix it's extending. when you are placing it in the "super-matrix", you can constrain it so that it's left most column, plus its width is less than the width of the super-matrix, and similarly for the highest row.

this is all at compile time.

as for integer types, you do not need to choose usize, u32, etc., until an actual machine-representation of the integer is required.

@thomasaarholt
Copy link

Thank you for the explanation!

@Ben-PH
Copy link
Author

Ben-PH commented Dec 15, 2024

Another way to put it:

const generics allow you to specify "use u32/usize/u64/etc here", typenum says "use any integer value here", and you can specify what "legal" is (non-zero, even/odd, the result of arithmetic, etc)

oh yeah, that's the other thing: you can do arithmetic with typenum. no need for SIZE because you can just derive that within the type system my multiplying the width-type by the height-type.

@HaoboGu
Copy link
Owner

HaoboGu commented Dec 16, 2024

I've looked through the PR, afaict, typenum brings us the compile-time checking and calculation features, which could be very useful. But now I tend to not use it because:

  1. The last release of typenum was over 1 year ago, I'm not sure whether it's been actively maintained.
  2. The compile-time checking part can be done in proc-macro, by checking values set in keyboard.toml. We've already had some, like:
    if layout.keymap.len() <= layout.layers as usize {
    // The required number of layers is less than what's set in keymap
    // Fill the rest with empty keys
    for _ in layout.keymap.len()..layout.layers as usize {
    // Add 2D vector of empty keys
    layout.keymap.push(vec![
    vec!["_".to_string(); layout.cols as usize];
    layout.rows as usize
    ]);
    }
    } else {
    return rmk_compile_error!(
    "keyboard.toml: Layer number in keymap is larger than [layout.layers]".to_string()
    );
    }
  3. The compile-time calculation part could be covered by Tracking Issue for complex generic constants: feature(generic_const_exprs) rust-lang/rust#76560. typenum has a long history, it appeared even before const_generics. But for the future, I prefer to use features provided by official Rust, not a third-party crate.

@Ben-PH
Copy link
Author

Ben-PH commented Dec 16, 2024

After our discussion in DMs, I'd be happy to re-write the PR so that it only affects the internal code-base, and not the public facing API.

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 this pull request may close these issues.

3 participants