Skip to content

Avoid register type aliases #466

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 7, 2020
Merged

Avoid register type aliases #466

merged 2 commits into from
Aug 7, 2020

Conversation

couchand
Copy link
Contributor

@couchand couchand commented Aug 7, 2020

Part 3 of #463, builds on #465.

  • Removes generated use of the register type aliases in favor of directly referencing Reg<REGISTER_SPEC>. I think this helps clarify how the generated and generic types work together.

No breaking change concerns on this change set.

r? @burrbull

@couchand couchand requested a review from a team as a code owner August 7, 2020 05:44
@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Aug 7, 2020
@couchand couchand mentioned this pull request Aug 7, 2020
Closed
@couchand couchand force-pushed the 202008/eschew-type-alias branch from e272e10 to 55197b0 Compare August 7, 2020 05:46
@therealprof
Copy link
Contributor

I'm a bit worried about generated source code size here. Do you have any numbers how much the source code blows up with that change? The compiler is already not too swift (yeah, sorry for the euphemism) thanks to the massive amount of generated code; we should try our best not to make it worse...

@burrbull
Copy link
Member

burrbull commented Aug 7, 2020

Yes, code size and compilation time questions are opened. We can reduce code size using derive_more::Deref/DerefMut/From, but this will require proc_macro2 in dependencies.

@couchand couchand force-pushed the 202008/eschew-type-alias branch from 55197b0 to d01cbe6 Compare August 7, 2020 16:18
@couchand
Copy link
Contributor Author

couchand commented Aug 7, 2020 •
edited
Loading

Rebased on master, leaving out the commits from #465 since the two PRs are completely independent. Please take another look.

@therealprof I suspect your question is more geared towards #465 rather than here, but I'll answer it in both places.

For Rahix/avr-device:

$ du --summarize src-*
7928	src-466
7920	src-master

For lpc-rs/lpc-pac:

$ du --summarize lpc*-*
4176	lpc11uxx-466
4168	lpc11uxx-master
6304	lpc176x5x-466
6296	lpc176x5x-master
277572	lpc54606-466
277548	lpc54606-master
4228	lpc82x-466
4212	lpc82x-master
117640	lpc845-466
117632	lpc845-master

@couchand couchand mentioned this pull request Aug 7, 2020
Merged
@therealprof
Copy link
Contributor

@therealprof I suspect your question is more geared towards #465 rather than here, but I'll answer it in both places.

Oops, yes you're right.

therealprof
therealprof approved these changes Aug 7, 2020
View reviewed changes
Copy link
Contributor

@therealprof therealprof left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

bors r+

@bors
Copy link
Contributor

bors bot commented Aug 7, 2020

Build succeeded:

@bors bors bot merged commit 48d4e20 into rust-embedded:master Aug 7, 2020
@therealprof therealprof mentioned this pull request Aug 8, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@therealprof therealprof therealprof approved these changes

Assignees

@burrbull burrbull

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@couchand @therealprof @burrbull @rust-highfive