Disjoint arrays

[n8tlarsen]

Solution 2 for #660 which pre-parses ERC type names and provides expansion information to the render functions. Works great with a few SVDs from the Renesas RA DFP which tend to use disjoint arrays quite frequently.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @reitermarkus (or someone else) soon.

Please see the contribution instructions for more information.

[burrbull]

Let's make release before merging this. @Emilgardis ?

[Emilgardis]

Let's make release before merging this. @Emilgardis ?

Yes lets!

[burrbull]

rebase, please

[n8tlarsen]

I know re-writing history that is publicly available is generally bad practice, but there were a number of conflicts with master that needed to be resolved in order to rebase.

[Emilgardis]

I know re-writing history that is publicly available is generally bad practice

we generally approve of doing that in prs, especially since thats when you have the opportunity, feel free to squash the commits, for now it's fine though as we're still reviewing this

[n8tlarsen]

I found an issue with the check_erc_reprs function when there are registers that use derivedFrom intermingled with registers that could be matched to a disjoint array

[n8tlarsen]

I also think the include_info fields of Repr could be simplified by instead utilizing the derived_from field of an erc, which I'd like to do before merging. Should I move this PR to a draft or is it ok for now?

[Emilgardis]

convert it to a draft if you feel there's more to be done 👍🏼

[n8tlarsen]

Hey everyone I reworked this to take advantage of the deriveFrom system and I think the logic is a bit easier to follow. Essentially instead of selectively including the register info, I'm allowing regex matched registers to implicitly deriveFrom others. I need to update some of the comments yet, but I'm running into an issue where derived registers each append a pub use <derived_name> in the library, causing a a bunch of rustc error[E0255]: the name <derived_name> is defined multiple times. Here's an example elc.rs.txt generated from R7FA2A1AB.svd.

[n8tlarsen]

I think the derive info method is preferable as it adds the least complexity. Let me know and I can squash out the old Representation method.

[burrbull]

cc @Emilgardis something with CI

[n8tlarsen]

Sorry for the late change, but I found some cases where deriving two arrays with the same name, but different dim_indexes caused naming collisions. I also squashed insignificant commits.

[burrbull]

yet one rebase on master, please

[burrbull]

Could you also show examples of source -> generated code?

[n8tlarsen]

Happy to share some examples. Here's a patched source from the Renesas FSP r7fa2a1ab.svd.patched. Patch addressed some enumeratedValues issues only. The file includes both explicit deriveFrom keys as well as some registers which the PR identifies as ImplicitDerive. The first interesting generated file contains instances handling ImplicitDerive cases: elc.rs. This file is quite long but illustrates good handling of multiple deriveFrom keys where the registers are arrays and the PR renames them to avoid name collisions: pfs.rs. Specifically one instance of P40%sPFS derives from P000PFS while another derives from P407PFS. You can find other sources in PAC project I'm working on: https://github.com/ra-rs/ra. I'm just waiting on this PR to push PACs for many of the SVDs.

[burrbull]

ok. LGFM now. Fix changelog

[burrbull]

Could you also check issues: https://github.com/rust-embedded/svd2rust/issues/283, https://github.com/rust-embedded/svd2rust/issues/169 and https://github.com/rust-embedded/svd2rust/issues/357 ?

[n8tlarsen]

After merging, this might be a good opportunity to improve/expand the CI a bit by adding tests against one or more of the Renesas SVDs I've referenced since they exercise this feature extensively. Not sure how that's accomplished but I'm happy to help.

[n8tlarsen]

Fixed a comment typo when I did a find/replace:

-    // Locate conflicting regions; we'll need to use unions to derive_infoesent them.
+    // Locate conflicting regions; we'll need to use unions to represent them.

[n8tlarsen]

Also cleaned up / reverted some debug items:

Click to expand

``` --- a/src/generate/peripheral.rs +++ b/src/generate/peripheral.rs @@ -1227,7 +1227,6 @@ fn render_ercs( RegisterCluster::Register(reg) => { trace!("Register: {}, DeriveInfo: {}", reg.name, derive_info); let mut rpath = None; - let before_name = reg.name.to_string(); if let DeriveInfo::Implicit(rp) = derive_info { rpath = Some(rp.clone()); } else { @@ -1236,14 +1235,14 @@ fn render_ercs( rpath = derive_register(reg, &dpath, path, index)?; } } + let reg_name = ®.name; let rendered_reg = register::render(reg, path, rpath, index, config).with_context(|| { let descrip = reg.description.as_deref().unwrap_or("No description"); format!( - "Error rendering register\nName: {before_name}\nDescription: {descrip}" + "Error rendering register\nName: {reg_name}\nDescription: {descrip}" ) })?; - reg.name = before_name; mod_items.extend(rendered_reg) } } ```

[burrbull]

bors r+

[bors[bot]]

Build succeeded:

• CI
• Rustfmt