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

False negative on mem_replace_option_with_none #9824

Closed
and-reas-se opened this issue Nov 9, 2022 · 2 comments · Fixed by #10594
Closed

False negative on mem_replace_option_with_none #9824

and-reas-se opened this issue Nov 9, 2022 · 2 comments · Fixed by #10594
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't

Comments

@and-reas-se
Copy link

and-reas-se commented Nov 9, 2022

Summary

I was going trough the Learning Rust with entirely too many linked lists tutorial. I have my editor set to give me warnings from clippy. In section 3.1 the tutorial has you replacing a type with an Option, and then changing a couple of of mem::replace instances with the take() method on Option. I thought to myself "huh, that's always better, that should be a lint". So I googled, and turns out it is a lint, but for some reason the lint doesn't trigger on the code in the tutorial.

Lint Name

mem_replace_option_with_none

Reproducer

This is the code from the tutorial, but with the irrelevant parts cut out:

use std::mem;

pub struct List {
    head: Option<Box<Node>>,
}

struct Node {
    #[allow(dead_code)] // to get rid of that warning since I cut the code that read from it
    next: Option<Box<Node>>,
}

impl List {
    pub fn push(&mut self) {
        let new_node = Box::new(Node {
            next: mem::replace(&mut self.head, None),
        });

        self.head = Some(new_node);
    }
}

To reproduce create a new with cargo library crate, paste into lib.rs, and run cargo clippy.
The lint is not triggered for the mem::replace on line 15.

Version

rustc 1.65.0 (897e37553 2022-11-02)
binary: rustc
commit-hash: 897e37553bba8b42751c67658967889d11ecd120
commit-date: 2022-11-02
host: x86_64-unknown-linux-gnu
release: 1.65.0
LLVM version: 15.0.0
@and-reas-se and-reas-se added C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't labels Nov 9, 2022
@J-ZhengLi
Copy link
Member

this can further simplified as:

struct T(Option<u8>);

fn main() {
    let mut t = T(Some(1));
    let test = std::mem::replace(&mut t.0, None);
    println!("{:?}", test);
}

seems like the original lint ignores member variables like t.0, self.head for some reason.

@J-ZhengLi
Copy link
Member

@rustbot claim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-negative Issue: The lint should have been triggered on code, but wasn't
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants