-
Notifications
You must be signed in to change notification settings - Fork 46
[wip] chore: use quote-based code generation #135
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
src/utils.rs
Outdated
| fn field_name(&self) -> Ident { | ||
| sanitize_name(self.get_name(), ToSnakeCase::to_snake_case).ident() | ||
| } | ||
| fn field_name2(&self, prefix: &str, suffix: &str) -> Ident { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe more descriptive name? field_name_with_affixes or wrap_field_name?
| fn field_name2(&self, prefix: &str, suffix: &str) -> Ident { | ||
| format!( | ||
| "{prefix}{}{suffix}", | ||
| sanitize_name(self.get_name(), ToSnakeCase::to_snake_case) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to use field_name to reduce duplication?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly - i would need to check
| let tmp: String; | ||
| sanitize_name( | ||
| if suffix.is_empty() { | ||
| self.get_name() | ||
| } else { | ||
| tmp = format!("{}{suffix}", self.get_name()); | ||
| &tmp | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what about this to get rid of tmp String?
| let tmp: String; | |
| sanitize_name( | |
| if suffix.is_empty() { | |
| self.get_name() | |
| } else { | |
| tmp = format!("{}{suffix}", self.get_name()); | |
| &tmp | |
| }, | |
| sanitize_name( | |
| &if suffix.is_empty() { | |
| self.get_name().to_string() | |
| } else { | |
| format!("{}{suffix}", self.get_name()) | |
| }, | |
| ToShoutySnakeCase::to_shouty_snake_case, | |
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh, i am not a big fan of optimizing away a single line of code at a cost of memory allocation. It does make sense for complex logic savings, but here... meh
nyurik
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @trnila for looking at it. I was mostly experimenting at this stage -- this whole PR is a draft just to keep everyone posted of the direction. I keep looking at the https://github.com/yasuo-ozu/template_quote to see if something like that would offer significantly cleaner codegen... It won't be perfect, but may offer some clarity...
| let tmp: String; | ||
| sanitize_name( | ||
| if suffix.is_empty() { | ||
| self.get_name() | ||
| } else { | ||
| tmp = format!("{}{suffix}", self.get_name()); | ||
| &tmp | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tbh, i am not a big fan of optimizing away a single line of code at a cost of memory allocation. It does make sense for complex logic savings, but here... meh
| fn field_name2(&self, prefix: &str, suffix: &str) -> Ident { | ||
| format!( | ||
| "{prefix}{}{suffix}", | ||
| sanitize_name(self.get_name(), ToSnakeCase::to_snake_case) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possibly - i would need to check
This pull request refactors the codebase to modernize code generation and streamline identifier handling, while removing legacy code that is no longer needed. The most significant changes are the migration to token-based code generation using
proc-macro2andquote, the introduction of new utilities for identifier management, and the cleanup of obsolete formatting and error-handling code.Migration to token-based code generation:
Write-based code generation inFeatureConfigwith methods that generateTokenStreams usingproc-macro2andquote, enabling more robust and idiomatic macro code generation. (src/feature_config.rs) [1] [2]Cargo.tomlto use explicit versions ofproc-macro2and a newer version ofquote.Identifier and naming utilities:
src/utils.rsfor generating and sanitizing Rust identifiers (Ident) from DBC names, including support for enum names, multiplexed variant names, and conversion traits (ToIdent,Tokens).ValTypeinsrc/signal_type.rsto implementIdentFragment, supporting its use in macro-generated identifiers. [1] [2]Removal of legacy and unused code:
PadAdapterstruct and associated formatting logic, as well as allWrite-based formatting utilities, in favor of token-based code generation. (src/pad.rs)CanErrorenum and itsDisplayimplementation from the includes module, as error handling is now generated directly in the output. (src/includes/errors.rs,src/includes/mod.rs) [1] [2]Testing and snapshot updates:
tests/snapshots.rsto use in-memory code generation and snapshot testing withinsta, reflecting the new code generation approach and ensuring test coverage for generated error types.These changes modernize the code generation pipeline, improve maintainability, and set the stage for further enhancements to the macro and codegen infrastructure.