rust-lang / rust-clippy

A bunch of lints to catch common mistakes and improve your Rust code. Book: https://doc.rust-lang.org/clippy/
https://rust-lang.github.io/rust-clippy/
Other
11.03k stars 1.48k forks source link

Error with latest nightly #12856

Open amircodota opened 1 month ago

amircodota commented 1 month ago

Summary

When I run clippy on the reproducer code I get

warning: the borrowed expression implements the required traits
   --> src/lib.rs:124:39
    |
124 |         let mut result = Aligned::new(&self.data);
    |                                       ^^^^^^^^^^ help: change this to: `self.data`
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrows_for_generic_args
    = note: `#[warn(clippy::needless_borrows_for_generic_args)]` on by default

However, if I remove the borrow, I get

cannot move out of `self.data` which is behind a shared reference
   --> src/lib.rs:124:39
    |
124 |         let mut result = Aligned::new(self.data);
    |                                       ^^^^^^^^^ move occurs because `self.data` has type `&mut [T]`, which does not implement the `Copy` trait

For more information about this error, try `rustc --explain E0507`.

Reproducer

I tried this code:

#![allow(clippy::manual_slice_size_calculation)]

use std::{
    alloc::Layout,
    fmt::{self, Debug},
    ops::{Deref, DerefMut},
};

pub struct Aligned<T: Copy + 'static> {
    data: &'static mut [T],
    len: usize,
}

const ALIGNMENT: usize = 128;

impl<T: Copy> Aligned<T> {
    pub fn new(v: impl AsRef<[T]>) -> Self {
        let v = v.as_ref();
        let layout =
            Layout::from_size_align(v.len() * std::mem::size_of::<T>(), ALIGNMENT).unwrap();
        unsafe {
            let ptr = std::alloc::alloc(layout);
            let ptr = ptr.cast::<T>();
            let new_v = std::slice::from_raw_parts_mut(ptr, v.len());
            new_v.copy_from_slice(v);
            Aligned {
                data: new_v,
                len: v.len(),
            }
        }
    }

    pub fn zeroed(len: usize) -> Self {
        let layout = Layout::from_size_align(len * std::mem::size_of::<T>(), ALIGNMENT).unwrap();
        unsafe {
            let ptr = std::alloc::alloc_zeroed(layout);
            let ptr = ptr.cast::<T>();
            let data = std::slice::from_raw_parts_mut(ptr, len);
            Aligned { data, len }
        }
    }

    pub fn extend(&mut self, input: &[T]) {
        let new_len = self.len + input.len();
        assert!(new_len <= self.data.len(), "no room to extend");
        self.data[self.len..new_len].copy_from_slice(input);
        self.len = new_len;
    }

    pub fn shorten(&mut self, amount: usize) {
        assert!(amount <= self.len, "no room to shorten");
        self.len -= amount;
    }

    pub fn truncate(&mut self, len: usize) {
        assert!(
            len <= self.len,
            "truncating to later position (len={}, truncating to {})",
            self.len,
            len
        );
        self.len = len;
    }

    pub fn as_u8_mut(&mut self) -> &mut [u8] {
        let slice: &mut [T] = &mut *self;
        let ptr = slice.as_mut_ptr();
        let len = slice.len() * std::mem::size_of::<T>();
        unsafe {
            let ptr = ptr.cast::<u8>();
            std::slice::from_raw_parts_mut(ptr, len)
        }
    }

    pub fn capacity(&self) -> usize {
        self.data.len()
    }
}

impl<T: Copy + Default> Aligned<T> {
    pub fn with_capacity(x: usize) -> Self {
        let mut result = Self::new(vec![T::default(); x]);
        result.len = 0;
        result
    }
}

impl<T: Copy> Drop for Aligned<T> {
    fn drop(&mut self) {
        let layout =
            Layout::from_size_align(self.data.len() * std::mem::size_of::<T>(), ALIGNMENT).unwrap();
        unsafe {
            let ptr = self.data.as_mut_ptr().cast::<u8>();
            std::alloc::dealloc(ptr, layout);
        }
    }
}

impl<T: Copy> Deref for Aligned<T> {
    type Target = [T];
    fn deref(&self) -> &[T] {
        &self.data[..self.len]
    }
}

impl<T: Copy> DerefMut for Aligned<T> {
    fn deref_mut(&mut self) -> &mut [T] {
        &mut self.data[..self.len]
    }
}

impl Debug for Aligned<half::f16> {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        (**self)
            .iter()
            .map(|x| f32::from(*x))
            .collect::<Vec<_>>()
            .fmt(f)
    }
}

impl<T: Copy + 'static> Clone for Aligned<T> {
    fn clone(&self) -> Aligned<T> {
        let mut result = Aligned::new(&self.data);
        result.len = self.len;
        result
    }
}

In stable clippy this works fine.

In latest nightly I get the above error (in the summary).

Version

rustc 1.80.0-nightly (1ba35e9bb 2024-05-25)
binary: rustc
commit-hash: 1ba35e9bb44d416fc2ebf897855454258b650b01
commit-date: 2024-05-25
host: x86_64-unknown-linux-gnu
release: 1.80.0-nightly
LLVM version: 18.1.6

Additional Labels

No response

jwiesler commented 1 month ago

Another example using loops:

pub fn serialize_to_buffer<F: serde::Serialize>(
    mut buffer: &mut Vec<u8>,
    iter: &[&str],
) {
    for item in iter {
        serde_json::to_writer(&mut buffer, &item).unwrap();
        //                    ^ suggestion: remove &mut
        // obviously this makes buffer get moved after first iteration
        buffer.extend(b"\n");
    }
}
J-ZhengLi commented 1 month ago

minimal repro (playground) thanks to @jwiesler:

fn foo<T: std::io::Write>(_buffer: T) {}

fn bar(mut buffer: &mut Vec<u8>) {
    foo(&mut buffer);
    buffer.extend(b"\n");
}
meithecatte commented 4 weeks ago

Bisected this to this clippy update: https://github.com/rust-lang/rust/commit/72d8d8d9f91122c59601b52c7f495f4e4dc50e29

Running a bisect on the clippy repo now

meithecatte commented 4 weeks ago

Perhaps unsurprisingly, the first bad commit is 79a14dea86fe8e8f5588fb2a32f5b73f065529a3. I'm planning to look into this, but in case I get distracted and drop this, don't hesitate to ping me if you'd like to take over.