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

Fix some clippy warnings #692

Merged
merged 3 commits into from
Jan 31, 2024
Merged

Fix some clippy warnings #692

merged 3 commits into from
Jan 31, 2024

Conversation

Enselic
Copy link
Contributor

@Enselic Enselic commented Jan 29, 2024

This PR makes these commands return no warnings (except "unknown lint" from 1.60 for clippy::bool_to_int_with_if and clippy::derive_partial_eq_without_eq).

cargo +1.75 clippy --lib    # current stable version
cargo +1.65 clippy --lib
cargo +1.60 clippy --lib --no-default-features --features read

In the case of #![allow(clippy::unnecessary_cast)] the current code is by my estimation clearer and more symmetric than the code clippy would want, so I ended up allowing unnecessary casts.

@philipc
Copy link
Collaborator

philipc commented Jan 30, 2024

In the case of #![allow(clippy::unnecessary_cast)] the current code is by my estimation clearer and more symmetric than the code clippy would want, so I ended up allowing unnecessary casts.

Do you mean these changes? They seem okay to me at first glance.

diff --git a/src/read/endian_reader.rs b/src/read/endian_reader.rs
index 8852b380..ef927e52 100644
--- a/src/read/endian_reader.rs
+++ b/src/read/endian_reader.rs
@@ -393,8 +393,8 @@ where
 
     #[inline]
     fn offset_from(&self, base: &EndianReader<Endian, T>) -> usize {
-        let base_ptr = base.bytes().as_ptr() as *const u8 as usize;
-        let ptr = self.bytes().as_ptr() as *const u8 as usize;
+        let base_ptr = base.bytes().as_ptr() as usize;
+        let ptr = self.bytes().as_ptr() as usize;
         debug_assert!(base_ptr <= ptr);
         debug_assert!(ptr + self.bytes().len() <= base_ptr + base.bytes().len());
         ptr - base_ptr
diff --git a/src/read/endian_slice.rs b/src/read/endian_slice.rs
index 0db28dac..b327a84f 100644
--- a/src/read/endian_slice.rs
+++ b/src/read/endian_slice.rs
@@ -68,8 +68,8 @@ where
     /// of the given slice.
     #[inline]
     pub fn offset_from(&self, base: EndianSlice<'input, Endian>) -> usize {
-        let base_ptr = base.slice.as_ptr() as *const u8 as usize;
-        let ptr = self.slice.as_ptr() as *const u8 as usize;
+        let base_ptr = base.slice.as_ptr() as usize;
+        let ptr = self.slice.as_ptr() as usize;
         debug_assert!(base_ptr <= ptr);
         debug_assert!(ptr + self.slice.len() <= base_ptr + base.slice.len());
         ptr - base_ptr

@@ -1046,7 +1046,7 @@ impl<R: Reader> Unit<R> {
/// The returned value is relative to this unit's `comp_dir`.
pub fn dwo_name(&self) -> Result<Option<AttributeValue<R>>> {
let mut entries = self.entries();
if let None = entries.next_entry()? {
if entries.next_entry()?.is_none() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I prefer:

        entries.next_entry()?;
        let entry = entries.current().ok_or(Error::MissingUnitDie)?;

This is more than a clippy fix, but it matches how we handle this case elsewhere in the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, why not. I'll create a separate PR for that shorter in case we need to continue iterating on the details.

@Enselic
Copy link
Contributor Author

Enselic commented Jan 30, 2024

Do you mean these changes? They seem okay to me at first glance.

Ok, let's go with those then. I pushed a commit for that now.

@philipc philipc merged commit 49d872e into gimli-rs:master Jan 31, 2024
20 checks passed
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 this pull request may close these issues.

2 participants