rust-lang / rust-lang/rust-clippy

FP clippy::drop_copy ?: new() called inside drop()

Open
#3,939 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

I-false-positive
Dominant language
Rust
Stars
13.5k
Forks
2.2k
Avg merge
2d 10h
Merged PRs (30d)
32

Description

I was looking at rls which has this warning

error: calls to `std::mem::drop` with a value that implements Copy. Dropping a copy leaves the original intact.
  --> rls-rustc/src/lib.rs:22:5
   |
22 |     drop(env_logger::init());
   |     ^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: #[deny(clippy::drop_copy)] on by default

code:


pub fn run() {
    drop(env_logger::init());
    let result = rustc_driver::report_ices_to_stderr_if_any(|| {
        let args = env::args_os()
            .enumerate()
            .map(|(i, arg)| {
                arg.into_string().unwrap_or_else(|arg| {
                    early_error(
                        ErrorOutputType::default(),
                        &format!("Argument {} is not valid Unicode: {:?}", i, arg),
                    )
                })
            })
            .collect::<Vec<_>>();

        run_compiler(&args, &mut ShimCalls, None, None)
    })
    .and_then(|result| result);
    process::exit(result.is_err() as i32);
}

struct ShimCalls;

impl Callbacks for ShimCalls {
    fn config(&mut self, config: &mut interface::Config) {
        config.opts.debugging_opts.continue_parse_after_error = true;
        config.opts.debugging_opts.save_analysis = true;
    }
}

I think this might be a false positive in this case because the value has no binding / is created inside the drop()?

I tried to come up with some example code

pub fn a(x: i32) {
    std::mem::drop(A::new());
}

#[derive(Copy, Clone)]
struct A {
    a: i32,
}

impl A {
    fn new() -> Self {
        println!("yay, side effects preventing compiler from optimizing this away");
        A { a: 4 }
    }
}

I think what the user originally wanted to do is make sure that A::new() works/is executed (it may have side effects) and is not optimized away by the compiler. However since the created object is not needed further, throw it away with drop() immediately.

Should the lint not warn in case there is no variable binding?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the warning from rls-rustc/src/lib.rs:22 and the A::new() example, starting at run() and the drop(env_logger::init()) call. Check whether clippy::drop_copy should distinguish temporaries from bound values, then define the expected lint behavior for constructors with side effects and add or update coverage if the relevant test location is found.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.