-
Notifications
You must be signed in to change notification settings - Fork 11
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
[cyacc] Add convenience macros to simplify calling delay functions #33
base: cycacc
Are you sure you want to change the base?
Conversation
Calling delay functions with non-trivial inputs requires using the turbofish operator and braces. For example, a 5 minute delay would have to be called as delay_sec::<{ 5 * 60 }>(). Since this syntax is a bit clunky and not very user friendly, macros are added for each delay function such that delay_sec!(5 * 60) expands to delay_sec::<{ 5 * 60 }>()
I am against this. Its merely cosmetic. You start by hiding the turbofish and inline const expression, and you endup with everything as a macro before your know it. Then you get a split in the community where sometimes code is behind a macro, sometimes it is not, just for cosmetic reasons.
error: comparison operators cannot be chained
--> src/main.rs:5:5
|
5 | foo<42 + 2>();
| ^ ^
|
help: use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
|
5 | foo::<42 + 2>();
| ++ error: expressions must be enclosed in braces to be used as const generic arguments
--> src/main.rs:5:8
|
5 | foo::<42 + 2>();
| ^^^^^^
|
help: enclose the `const` expression in braces
|
5 | foo::<{ 42 + 2 }>();
| + + With that said, at the end of the day, I don't care enough to spend time fighting it. |
You're right that it's basically just cosmetic. In general I don't mind the turbofish, but when we're just trying to implement "a function whose parameters have to be const" I find it annoying how much more cluttered it is compared to a standard function. Moving the library over from using regular functions to using const generics is already a big change, and I feel more comfortable making it if it still looks a little bit more like a regular function to the user. |
Except they are not regular functions. Macros are also visibly not regular functions. The reason behind the You could argue that the functions are actually generating machine code without ever producing a function call. That's the whole point. You want an exact number of cycles right here and now. And so maybe you could furthermore argue that a macro reflects this better in abstract. At this point I wouldn't know what to choose. Having both style might be confusing. I like the purity of keeping it what it is: a function with compile time arguments. Because a function is an abstract concept anyways. If you start doing stuff like At the end of the day, I care more about the cycle accurate codegen than the syntax. I will let others chime in and bring new arguments. If the macro style means the code merges into master faster and is available to all, then so be it. |
On Sun, Jun 12, 2022 at 10:22:48PM -0700, François-Xavier Bourlet wrote:
.... I will let others chime in
Do not wait for me
|
Calling delay functions with non-trivial inputs requires using the turbofish operator and braces. For example, a 5 minute delay would have to be called as
delay_sec::<{ 5 * 60 }>()
.This syntax is a bit clunky and not very user-friendly. Turbofish is an obscure syntax; even the person who literally wrote these functions forgot that you need to use the turbofish to call them (#32), so I think expecting users to remember it just to use the basic functionality of this library is an unnecessary burden.
This PR adds macros for every delay function (delay_cycles!, delay_us!, delay_ms!, delay_sec!) such that for example
delay_sec!(5 * 60)
expands todelay_sec::<{ 5 * 60 }>()