Skip to content

Add Clash numeric types plus basic impls#1176

Open
rslawson wants to merge 6 commits intomainfrom
rs/numeric-types
Open

Add Clash numeric types plus basic impls#1176
rslawson wants to merge 6 commits intomainfrom
rs/numeric-types

Conversation

@rslawson
Copy link
Copy Markdown
Collaborator

@rslawson rslawson commented Feb 2, 2026

This implements basic functionality for Index<N, T>, Unsigned<N, T>, Signed<N, T>, and BitVec<N, M>. This PR is currently marked as draft since I still need to:

  • Add documentation to the types themselves and to the size check traits, not just their methods
  • Create macros as aliases for the types
  • Create instantiation macros
  • Add tests?

Anywho, at least an initial review pass and/or suggestions on things I should add in this PR that I didn't (and that I don't have WIP stuff for already from before I did a lot of downscoping, like operator impls).

@hydrolarus
Copy link
Copy Markdown
Contributor

Have you done any tests to see if generated code does not need any trait bounds? To me it seems like it it shouldn't be required, but there's no usage examples, just making sure since I'm afraid of trait bounds 😱

@rslawson
Copy link
Copy Markdown
Collaborator Author

rslawson commented Feb 3, 2026

Have you done any tests to see if generated code does not need any trait bounds? To me it seems like it it shouldn't be required, but there's no usage examples, just making sure since I'm afraid of trait bounds 😱

I haven't done that yet, no. That's what I'll be working on today, so hopefully it's something I can complete today (^:

Comment on lines +396 to +413
let input_str = input.to_string();
if !(input_str.starts_with("0x") || input_str.starts_with("0X"))
|| input_str
.chars()
.skip(2)
.any(|c| !(c.is_ascii_hexdigit() || c == ' ' || c == '_'))
{
return syn::Error::new(
input.span(),
concat!(
"Expected a hexadecimal literal! Must start with `0x` or `0X` and contain ",
"only hexdigits, spaces, and underscores",
)
.to_string(),
)
.into_compile_error()
.into();
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it make sense to also support binary literals and maybe even base 10 literals?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could add support for binary literals pretty easily, just the issue is that for decimal literals there's the issue of not having a way to turn that into binary very easily past a certain size.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added binary literals, at least. Still not sure what to do for decimal literals.

Comment thread firmware-support/bittide-hal/src/manual_additions/bitvector.rs
Comment thread firmware-support/bittide-hal/src/manual_additions/bitvector.rs Outdated
Comment thread firmware-support/bittide-hal/src/manual_additions/bitvector.rs Outdated
Comment thread firmware-support/bittide-hal/src/manual_additions/bitvector.rs Outdated
Comment thread firmware-support/bittide-hal/src/manual_additions/index.rs Outdated
Comment thread firmware-support/bittide-hal/src/manual_additions/index.rs
Comment thread firmware-support/bittide-hal/src/manual_additions/signed.rs Outdated
Comment thread firmware-support/bittide-hal/src/manual_additions/calendar.rs Outdated
Comment thread firmware-support/bittide-hal/src/manual_additions/calendar.rs Outdated
@rslawson rslawson force-pushed the rs/numeric-types branch 5 times, most recently from 3c4d3da to 5da7507 Compare March 2, 2026 15:46
@rslawson rslawson requested a review from hydrolarus March 2, 2026 18:09
let mut char_iter = input_str.chars().filter(char::is_ascii_hexdigit).rev();
let mut lower = true;
let mut idx = 0;
loop {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been thinking if there is a good way to use from_str_radix for this instead of hand rolling this, but I guess you want arbitrarily large literals too, so this wouldn't work 😔 Maybe this could be a comment here though? Otherwise I'll wonder the same thing in two months when I look at this again haha

Comment thread firmware-support/bittide-macros/src/lib.rs
@rslawson rslawson force-pushed the rs/numeric-types branch 2 times, most recently from dbbb82e to f2ca52c Compare March 3, 2026 12:27
@rslawson rslawson force-pushed the rs/numeric-types branch 3 times, most recently from 2bf627f to cdd3069 Compare March 18, 2026 12:31
@rslawson rslawson marked this pull request as ready for review March 18, 2026 13:50
@rslawson
Copy link
Copy Markdown
Collaborator Author

Marked as ready for review. I know there's one more thing I need to do (sign extension for Signed<N, T> when converting from BitVector<M, N>) before it's actually ready to go, but what with all the other things I want to do done plus all the tests passing I'm happy to call it ready for review.

@rslawson rslawson force-pushed the rs/numeric-types branch 2 times, most recently from d00584e to 95fa3d0 Compare March 19, 2026 15:10
@rslawson rslawson requested a review from hydrolarus March 19, 2026 15:16
Copy link
Copy Markdown
Contributor

@hydrolarus hydrolarus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looking good, but some questions regarding macro hygiene, style of type macros and usage of expr-macros

Comment on lines +692 to +694
let n = n as usize;
let len = n.div_ceil(8);
quote! { BitVector<#n, #len> }
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This leaves suffixes everywhere, right? So BitVector<24usize, 3usize>. Wouldn't it make sense to use unsuffixed literals since usually the types of the literals aren't so important on the type level?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same goes for the other types below

Comment thread firmware-support/bittide-macros/src/lib.rs Outdated
Comment thread firmware-support/bittide-macros/src/lib.rs Outdated
Comment thread firmware-support/bittide-macros/src/lib.rs
Comment thread firmware-binaries/sim-tests/elastic_buffer_wb_test/src/main.rs Outdated
Comment thread firmware-binaries/sim-tests/elastic_buffer_wb_test/src/main.rs Outdated
Comment thread firmware-binaries/sim-tests/nested_interconnect_test/src/main.rs Outdated
Comment thread firmware-binaries/sim-tests/switch_calendar_test/src/main.rs Outdated
Comment thread firmware-binaries/sim-tests/switch_calendar_test/src/main.rs Outdated
@rslawson rslawson requested a review from hydrolarus April 21, 2026 13:36
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.

2 participants