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

[matthiaskrgr]

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?

[oli-obk]

In case there are no variable bindings, I think we should suggest to remove the surrounding drop, as that is equivalent.

[Arnavion]

drop(env_logger::init()); -> env_logger::init(); looks fine, but drop(A::new()); -> A::new(); looks like a typo. I think it would be better to suggest let _ = env_logger::init(); and let _ = A::new();

[GnomedDev]

Should this be marked as needing discussion?