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

missing_copy_implementations probably shouldn't fire if the type implements Iterator #98348

Closed
Kixunil opened this issue Jun 21, 2022 · 0 comments · Fixed by #102406
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Kixunil
Copy link
Contributor

Kixunil commented Jun 21, 2022

Iterators shouldn't implement copy but the lint suggests it anyway.

Given the following code:

#![warn(missing_copy_implementations)]

pub struct MyIterator {
    num: u8,
}

impl Iterator for MyIterator {
    type Item = u8;
    
    fn next(&mut self) -> Option<Self::Item> {
        if self.num > 42 {
            None
        } else {
            self.num += 1;
            Some(self.num)
        }
    }
}

fn main() {
    let iter = MyIterator { num: 0 };
    for i in iter {
        println!("{}", i);
    }
}

The current output is:

warning: type could implement `Copy`; consider adding `impl Copy`
 --> src/main.rs:3:1
  |
3 | / pub struct MyIterator {
4 | |     num: u8,
5 | | }
  | |_^
  |
note: the lint level is defined here
 --> src/main.rs:1:9
  |
1 | #![warn(missing_copy_implementations)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Ideally there should be no warning.

It is widely accepted that iterators shouldn't implement Copy because of it being prone to accidentally modifying a copy instead of original (by &mut). However this lint suggests to implement Copy anyway. In case the intention is "give the user the warning anyway because he asked for it" the message should probably mention this edge-case and suggest allow instead.

@Kixunil Kixunil added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 21, 2022
Manishearth added a commit to Manishearth/rust that referenced this issue Nov 18, 2022
Make `missing_copy_implementations` more cautious

- Fixes rust-lang#98348
- Also makes the lint not fire on large types and types containing raw pointers. Thoughts?
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this issue Dec 9, 2022
Make `missing_copy_implementations` more cautious

- Fixes rust-lang#98348
- Also makes the lint not fire on large types and types containing raw pointers. Thoughts?
@bors bors closed this as completed in 4fae589 Dec 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant