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

Clippy complains about generated code #557

Closed
robamu opened this issue Dec 10, 2021 · 2 comments · Fixed by #558
Closed

Clippy complains about generated code #557

robamu opened this issue Dec 10, 2021 · 2 comments · Fixed by #558

Comments

@robamu
Copy link
Contributor

robamu commented Dec 10, 2021

This is a bit of a luxury problem but maybe something that could be addressed so I don't need to patch a SVD file which is ok in my opinion.

When faced with a register definitions like this:

        <register>
          <name>WDOGLOAD</name>
          <description>Counter Start Value</description>
          <addressOffset>0x0000</addressOffset>
          <access>read-write</access>
          <resetValue>0xFFFFFFFF</resetValue>
          <fields>
            <field>
              <name>CNT</name>
              <description>Count to load</description>
              <bitRange>[31:0]</bitRange>
            </field>
          </fields>	
        </register>		

svd2rust will generate code which clippy does not like:

error: this operation will always return zero. This is likely not the intended outcome
 --> va416xx/src/watch_dog/wdogload.rs:60:23
  |
60 |         self.w.bits = (self.w.bits & !0xffff_ffff) | (value as u32 & 0xffff_ffff);
  |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#erasing_op

warning: the operation is ineffective. Consider reducing it to `value as u32`
 --> va416xx/src/watch_dog/wdogload.rs:60:54
  |
60 |         self.w.bits = (self.w.bits & !0xffff_ffff) | (value as u32 & 0xffff_ffff);
  |                                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op

warning: the operation is ineffective. Consider reducing it to `self.bits`
 --> va416xx/src/watch_dog/wdogload.rs:68:20
  |
68 |         CNT_R::new((self.bits & 0xffff_ffff) as u32)
  |                    ^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op

warning: the operation is ineffective. Consider reducing it to `self.bits`
 --> va416xx/src/watch_dog/wdogvalue.rs:35:20
  |
35 |         CNT_R::new((self.bits & 0xffff_ffff) as u32)
  |                    ^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#identity_op

This probably requires svd2rust to check whether

  1. Only one fields exists
  2. That field has a width of 32 bits.

In that case, the code can be simplified. For example, this code:

impl<'a> CNT_W<'a> {
    #[doc = r"Writes raw bits to the field"]
    #[inline(always)]
    pub unsafe fn bits(self, value: u32) -> &'a mut W {
        self.w.bits = (self.w.bits & !0xffff_ffff) | (value as u32 & 0xffff_ffff);
        self.w
    }
}

would become

impl<'a> CNT_W<'a> {
    #[doc = r"Writes raw bits to the field"]
    #[inline(always)]
    pub unsafe fn bits(self, value: u32) -> &'a mut W {
        self.w.bits = value;
        self.w
    }
}
@burrbull
Copy link
Member

@robamu Check #558

@robamu
Copy link
Contributor Author

robamu commented Dec 10, 2021

Thanks, this fixes the issue and clippy is happy now :)

@Emilgardis Emilgardis linked a pull request Dec 12, 2021 that will close this issue
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

Successfully merging a pull request may close this issue.

3 participants