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

Implicit Copy Lint #9061

Open
Kile-Asmussen opened this issue Jun 28, 2022 · 3 comments
Open

Implicit Copy Lint #9061

Kile-Asmussen opened this issue Jun 28, 2022 · 3 comments
Labels
A-lint Area: New lints

Comments

@Kile-Asmussen
Copy link

Kile-Asmussen commented Jun 28, 2022

What it does

An allow-by-default lint to highlight implicit uses of the Copy trait, which has relevance in some kinds of unsafe code. Using it in deny mode would force the programmer to write .clone() everywhere they want to copy a value.

Lint Name

implicit_copy

Category

correctness

Advantage

Among other things, I suggested it in UnsafeCell should implement the Copy trait #25053 , as a potential solution. There are other cases in unsafe code.

Drawbacks

It will almost always be on allow; the use cases are very rare.

Example

Given a hypothetical Copy impl for UnsafeCell:

unsafe fn footgun(cell: UnsafeCell<u32>) {
  *cell.get() += 2
}

fn ouch_i_shot_my_foot() {
  let x = UnsafeCell::new(2);
  unsafe { footgun(x); }
  assert_eq(unsafe { *x.get() }, 4); // kaboom 
}

An example of how this would help:

// user code
unsafe fn footgun(cell: UnsafeCell<u32>) {
  *cell.get() += 2
}

#[deny(implicit_copy)]
fn ouch_i_shot_my_foot() {
  let x = UnsafeCell::new(2);
  unsafe { footgun(x); } // lint complains here!
  // programmer now has a strong hint that footgun should take an &UnsafeCell instead
  assert_eq(unsafe { *x.get() }, 4); // no kaboom 
}

// meanwhile this is possible
#[derive(Copy, Clone)]
struct MyCleverThing(UnsafeCell<u64>);
@Kile-Asmussen Kile-Asmussen added the A-lint Area: New lints label Jun 28, 2022
@correabuscar
Copy link

Could this lint, if ever implemented, be used to make the following program not compile when cargo clippy is run on it?

#![deny(clippy::all, clippy::pedantic, clippy::nursery, warnings, future_incompatible, nonstandard_style,
        non_ascii_idents, clippy::restriction, rust_2018_compatibility, rust_2021_compatibility, unused)]
#![allow(clippy::print_stdout, clippy::use_debug, clippy::missing_docs_in_private_items)]

#![allow(clippy::blanket_clippy_restriction_lints)] //workaround clippy

// might want to deny later:
#![allow(clippy::default_numeric_fallback)]
#![allow(clippy::dbg_macro)]

//src: https://users.rust-lang.org/t/rust-book-suggestion-add-a-section-regarding-copy-vs-move/1549/2
fn foo(mut x: [i32; 4]) {
    println!("x(before) = {:?}", x);
    x = [1, 2, 3, 4];
    println!("x(after) = {:?}", x);
}

fn main() {
    let a = [0; 4];
    #[deny(implicit_copy)]
    foo(a);//sneakily copied! would the lint make it error here?! that would be gr8
    println!("a = {:?}", a);//unchanged, doh! but since it was just copied above, can use it here
                            //without errors, this is the problem!
}

@Kile-Asmussen
Copy link
Author

@correabuscar

Could this lint, if ever implemented, be used to make the following program not compile when cargo clippy is run on it?

If I am reading the code correctly, and if

    #[deny(implicit_copy)]
    foo(a);

Is to mean applying the lint only to that single expression, then yes. The a is Copy-ed. If it was rewritten to

    #[deny(implicit_copy)]
    foo(a.clone());

there would be no such problem.

@correabuscar
Copy link

Yes, that is exactly what I want! When I want copy then I'll use .clone(), otherwise I want it to compile fail instead of bypassing the borrow checker ie. in my code above, I want the println! to fail to compile! It would fail with this lint.

This lint would be great! I'd actually use it crate-wide!

Thank you for your reply and for suggesting it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints
Projects
None yet
Development

No branches or pull requests

2 participants