Ignore non snake case types that can exist in SVD files #397

mvertescher · 2019-11-12T14:04:01Z

Hi! 👋 Thanks for this awesome project!

I've run into a build error generating bindings for the psoc6-pac. Here's a example of one of the many build failures:

error: structure field `data_list_sent_update__status` should have a snake case name
   --> src/ble.rs:187:9
    |
187 |     pub data_list_sent_update__status: self::blell::DATA_LIST_SENT_UPDATE__STATUS,
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert the identifier to snake case: `data_list_sent_update_status`

This issue is caused by #![deny(warnings)] and should be fixed by adding the #![allow(non_snake_case)] exception. There may be alternative fixes, I'm open to suggestions!

rust-highfive · 2019-11-12T14:04:05Z

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

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

therealprof · 2019-11-12T14:23:12Z

The dreaded #![deny(warnings)] strikes again. I'd rather remove that and add explicit deny rules for the things we really do not want to allow.

therealprof · 2019-11-12T14:23:48Z

cf. https://github.com/rust-unofficial/patterns/blob/master/anti_patterns/deny-warnings.md

mvertescher · 2019-11-12T14:31:12Z

I'd rather remove that and add explicit deny rules for the things we really do not want to allow.

SGTM, I'll update this PR to instead deny a (hopefully safe) list of warnings.

therealprof · 2019-11-12T15:30:58Z

If you would be so kind to update the CHANGELOG.md and rebase I'd give this a CI run...

mvertescher · 2019-11-12T15:34:13Z

@therealprof I've updated to only deny a subset of warning adopted from the patterns repo (the warnings referenced there are a little out of date, I'll fix that up later).

I think the set of denied warnings should not affect users, but I could be wrong.

mvertescher · 2019-11-12T15:51:16Z

Hrm, taking a closer look at this, there are some rustc lints denied in this change that are explicitly allowed (rustc -W help) by the Rust 1.39.0 compiler:

missing_debug_implementations,
trivial_casts,
unused_import_braces,
unused_results,

I believe that by denying these, we could potentially break future PAC builds. What do you think @therealprof ?

therealprof · 2019-11-12T18:16:39Z

No idea to be honest. Can you rebase, then we can give it a spin? I'm totally expecting that we will need to tweak those...

mvertescher · 2019-11-13T04:43:43Z

Cool, that sounds good. This branch should be up to date with master. I think we can run CI?

burrbull · 2019-11-13T06:11:33Z

bors try

therealprof · 2019-11-13T07:53:09Z

Cool, that sounds good. This branch should be up to date with master. I think we can run CI?

It's still separate commits. ;) Maybe I should have said rebase and squash. It looks a weird to merge a PR that does one thing in a commit and reverses course in the next.

mvertescher · 2019-11-13T08:29:03Z

It's still separate commits. ;) Maybe I should have said rebase and squash. It looks a weird to merge a PR that does one thing in a commit and reverses course in the next.

Ah yes, that makes more sense. :) I'll rebase and squash!

therealprof · 2019-11-13T09:07:27Z

bors try

bors · 2019-11-13T09:07:28Z

try

Already running a review

bors · 2019-11-13T11:11:36Z

try

Timed out

therealprof · 2019-11-13T11:35:57Z

bors try

therealprof · 2019-11-13T11:36:31Z

I think it got confused by the rebase in the middle of the try run. ;)

therealprof · 2019-11-13T11:49:31Z

As expected tons of errors here:
https://travis-ci.org/rust-embedded/svd2rust/builds/611328824

Not sure whether this is the only one but it's certainly the most common one:
error: trivial cast: &uart0::RegisterBlockas*const uart0::RegisterBlock`

bors · 2019-11-13T11:57:38Z

try

Build failed

continuous-integration/travis-ci/push

therealprof · 2019-11-14T08:18:01Z

@mvertescher Can you have a look at the logs and adjust the PR accordingly?

This change decreases the chance of Rust build failures when SVD files have strange formatting for types. The changelog has also been updated.

mvertescher · 2019-11-16T20:43:42Z

So, I've removed any deprecated (bad-style) or now allowed lints (like trivial-casts) from the denied lint list. I believe this new configuration should not break any existing crates, but we'll see!

bors · 2019-11-16T20:44:05Z

🔒 Permission denied

Existing reviewers: click here to make mvertescher a reviewer

mvertescher · 2019-11-16T20:44:33Z

@therealprof can you give CI a run?

therealprof

LGTM

therealprof · 2019-11-16T20:47:31Z

bors r+

397: Ignore non snake case types that can exist in SVD files r=therealprof a=mvertescher Hi! 👋 Thanks for this awesome project! I've run into a build error generating bindings for the [psoc6-pac](https://github.com/psoc-rs/psoc6-pac). Here's a example of one of the many build failures: ``` error: structure field `data_list_sent_update__status` should have a snake case name --> src/ble.rs:187:9 | 187 | pub data_list_sent_update__status: self::blell::DATA_LIST_SENT_UPDATE__STATUS, | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: convert the identifier to snake case: `data_list_sent_update_status` ``` This issue is caused by `#![deny(warnings)]` and should be fixed by adding the `#![allow(non_snake_case)]` exception. There may be alternative fixes, I'm open to suggestions! Co-authored-by: Matt Vertescher <[email protected]>

therealprof · 2019-11-16T20:48:43Z

bors d+

bors · 2019-11-16T20:48:44Z

✌️ mvertescher can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

therealprof · 2019-11-16T20:48:57Z

Just in case something fails.

bors · 2019-11-16T21:18:53Z

Build succeeded

continuous-integration/travis-ci/push

mvertescher requested a review from a team as a code owner November 12, 2019 14:04

rust-highfive assigned therealprof Nov 12, 2019

rust-highfive added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-tools labels Nov 12, 2019

mvertescher mentioned this pull request Nov 12, 2019

Fix the readme link to travis CI psoc-rs/psoc6-pac#1

Merged

bors bot added a commit that referenced this pull request Nov 13, 2019

Try #397:

8ab87dc

mvertescher force-pushed the non-snake-case-types branch from 85e0184 to cb5d147 Compare November 13, 2019 08:34

bors bot added a commit that referenced this pull request Nov 13, 2019

Try #397:

5a6b185

Deny a subset of warnings instead of all in the generated lib.rs

73cfa74

This change decreases the chance of Rust build failures when SVD files have strange formatting for types. The changelog has also been updated.

mvertescher force-pushed the non-snake-case-types branch from cb5d147 to 73cfa74 Compare November 16, 2019 20:40

therealprof approved these changes Nov 16, 2019

View reviewed changes

bors bot merged commit 73cfa74 into rust-embedded:master Nov 16, 2019

mvertescher deleted the non-snake-case-types branch November 16, 2019 21:23

Ignore non snake case types that can exist in SVD files #397

Ignore non snake case types that can exist in SVD files #397

Uh oh!

Conversation

mvertescher commented Nov 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Nov 12, 2019

Uh oh!

therealprof commented Nov 12, 2019

Uh oh!

therealprof commented Nov 12, 2019

Uh oh!

mvertescher commented Nov 12, 2019

Uh oh!

therealprof commented Nov 12, 2019

Uh oh!

mvertescher commented Nov 12, 2019

Uh oh!

mvertescher commented Nov 12, 2019

Uh oh!

therealprof commented Nov 12, 2019

Uh oh!

mvertescher commented Nov 13, 2019

Uh oh!

burrbull commented Nov 13, 2019

Uh oh!

therealprof commented Nov 13, 2019

Uh oh!

mvertescher commented Nov 13, 2019

Uh oh!

therealprof commented Nov 13, 2019

Uh oh!

bors bot commented Nov 13, 2019

try

Uh oh!

bors bot commented Nov 13, 2019

try

Timed out

Uh oh!

therealprof commented Nov 13, 2019

Uh oh!

therealprof commented Nov 13, 2019

Uh oh!

therealprof commented Nov 13, 2019

Uh oh!

bors bot commented Nov 13, 2019

try

Build failed

Uh oh!

therealprof commented Nov 14, 2019

Uh oh!

mvertescher commented Nov 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors bot commented Nov 16, 2019

Uh oh!

mvertescher commented Nov 16, 2019

Uh oh!

therealprof left a comment

Choose a reason for hiding this comment

Uh oh!

therealprof commented Nov 16, 2019

Uh oh!

therealprof commented Nov 16, 2019

Uh oh!

bors bot commented Nov 16, 2019

Uh oh!

therealprof commented Nov 16, 2019

Uh oh!

bors bot commented Nov 16, 2019

Build succeeded

Uh oh!

Uh oh!

mvertescher commented Nov 12, 2019 •

edited

Loading

mvertescher commented Nov 16, 2019 •

edited

Loading