Skip to content
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

Move generic.rs contents to other crate #427

Closed
burrbull opened this issue Jan 9, 2020 · 8 comments
Closed

Move generic.rs contents to other crate #427

burrbull opened this issue Jan 9, 2020 · 8 comments

Comments

@burrbull
Copy link
Member

burrbull commented Jan 9, 2020

I don't see sense to built this code in each file that svd2rust generates.

I propose to move it in new crate with will be dependency for pacs.

It should have short name. Maybe (pa or ra as abbreviation of Peripheral Access an Register Access).

I see only advantages, no disadvantages:

  • we can simplify svd2rust code, remove some tricks in stm32-rs
  • we can use Cargo.toml for optionally enabling features like atomic operations (Support for atomic bitwise operations in MSP430 PAC API #407)
  • use "semver" to control that crates have identical API
  • avoid potential name conflicts when reexport generic.rs
@burrbull
Copy link
Member Author

burrbull commented Jan 9, 2020

cc @therealprof @Disasm @adamgreig

@therealprof
Copy link
Contributor

I can see one disadvantage: We need to keep the generated code and this new crate in lockstep which makes development a tad more complicated. But other than that I agree in principal.

Due to the impact I'm convinced this needs be covered by an RFC, though.

@Disasm
Copy link
Member

Disasm commented Jan 9, 2020

I agree with @therealprof regarding more complicated development. As an alternative, we may have an option to provide register.rs on demand. For example, in stm32ral such common code exists in repository and is not touched by generator. One can update this file only if generator (svd2rust) version is changed.

@adamgreig
Copy link
Member

@Disasm, is having a file that is generated occasionally that much simpler than having a crate that is updated occasionally? We can have the extra crate live in this repository the same as cortex-m-rt-macros for instance, and so the git repository would always be consistent.

@therealprof
Copy link
Contributor

@adamgreig That still adds additional complications on top of the already tedious testing procedure, a lot of changes/improvements need to be tested in the context of a HAL to make sense, e.g. binary size.

Not saying it can't be done or is not worthwhile, just something to consider.

@burrbull
Copy link
Member Author

burrbull commented Jan 9, 2020

Unfortunately, I found serious difficulty in implementing the idea. A lot of:

error[E0116]: cannot define inherent `impl` for a type outside of the crate where the type is defined

I apologize for disturbing you.

@burrbull
Copy link
Member Author

burrbull commented Jan 9, 2020

We need specialization for this.

@jonas-schievink
Copy link
Contributor

Specialization doesn't cover inherent impls. This does not look implementable by any in-progress language feature.

bors bot added a commit that referenced this issue Aug 15, 2020
467: Allow unused FieldReader r=therealprof a=couchand

I'm developing code that's general across AVR chips.  I'd like to depend on the shared parts of Rahix/avr-device (see Rahix/avr-device#44).  When compiling without any specific device feature enabled, there are no users of the `FieldReader::new` method.  This PR skips the lint for this case.

This is somewhat related to #427 but not exactly.

Co-authored-by: Andrew Dona-Couch <hi@andrewcou.ch>
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

No branches or pull requests

5 participants