Skip to content

Emit more Cluster arrays as arrays instead of lists of elements. #534

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
Jul 21, 2021
Merged

Emit more Cluster arrays as arrays instead of lists of elements. #534

merged 2 commits into from
Jul 21, 2021

Conversation

rcls
Copy link
Contributor

@rcls rcls commented Jul 19, 2021 •
edited
Loading

(This obsoletes #531)
Some Cluster arrays cannot be emitted as Rust (built-in) arrays due to padding between elements.
Add a helper zero-sized type ArrayProxy that overloads [] to do pointer-arithmetic internally.

I believe, although I'm not 100% confident, that I'm not breaking Rust safety, as follows: There is no way for (safe) code to create an ArrayProxy. Code that produces a (reference to) an ArrayProxy has to use unsafe somewhere.

See rcls/svd2rust-example@b8645fe for any example of the difference this makes (or does not make):

  • STM32L051 and LPC408x get no change.
  • Cypress PSOC gets a significant difference.

This uses const generics, so the new functionality requires the --const_generic command line optoin.

@rcls
Emit more Cluster arrays as arrays instead of lists of elements.
60aa7be 
Add ArrayProxy for handling Cluster arrays that would be emitted as a
rust array except for the need for padding between elements.
@rust-highfive
Copy link

r? @ryankurte

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Jul 19, 2021
@rcls rcls mentioned this pull request Jul 19, 2021
Closed
@burrbull
Copy link
Member

burrbull commented Jul 19, 2021 •
edited
Loading

One obvious issue with the current code: It uses const-generics, and does not build with the current minimum supported Rust version (1.40.0). Marking as draft until at least that is fixed.

svd2rust has --const_generic option that could be uses in your situation also.

@rcls
Split ArrayProxy out into its own file.
af4cc76 
Make the usage of ArrayProxy dependent on the const_generics config
option.
burrbull
burrbull reviewed Jul 20, 2021
View reviewed changes
@rcls
Copy link
Contributor Author

rcls commented Jul 20, 2021

svd2rust has --const_generic option that could be uses in your situation also.

Thanks. The changes updated to rely on that option.

@rcls rcls marked this pull request as ready for review July 20, 2021 08:26
@rcls rcls requested a review from a team as a code owner July 20, 2021 08:26
@burrbull
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Jul 21, 2021

Build succeeded:

@bors bors bot merged commit ead1f51 into rust-embedded:master Jul 21, 2021
@rcls rcls deleted the array-proxy branch July 27, 2021 17:08
@adamgreig adamgreig mentioned this pull request Oct 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@burrbull burrbull burrbull approved these changes

Assignees

@ryankurte ryankurte

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
@rcls @rust-highfive @burrbull @ryankurte