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

Generated Clone implementation triggers clippy lint about being incorrect for a Copy type #181

Closed
stefunctional opened this issue Aug 25, 2023 · 4 comments · Fixed by #183 or #184
Closed
Assignees

Comments

@stefunctional
Copy link
Contributor

This clippy lint is new in Rust 1.72.0.

Reproduction

Create a project with the following:

[package]
name = "playffi"
version = "0.1.0"
edition = "2021"

[lib]
crate-type = ["staticlib", "lib"]

[dependencies]
safer-ffi = { version = "0.1.2", features = ["proc_macros"] }
use safer_ffi::derive_ReprC;

#[derive_ReprC]
#[repr(opaque)]
pub struct Foo {
    _x: i32,
}

Run the command below:

cargo clippy

Expected result

No lints are emitted.

Actual result

error: incorrect implementation of `clone` on a `Copy` type
 --> src/lib.rs:3:1
  |
3 | #[derive_ReprC]
  | ^^^^^^^^^^^^^^^
  |
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#incorrect_clone_impl_on_copy_type
  = note: `#[deny(clippy::incorrect_clone_impl_on_copy_type)]` on by default
  = note: this error originates in the attribute macro `derive_ReprC` (in Nightly builds, run with -Z macro-backtrace for more info)

Analysis

The macro-expanded code is:

#![feature(prelude_import)]
#[prelude_import]
use std::prelude::rust_2021::*;
#[macro_use]
extern crate std;
use safer_ffi::derive_ReprC;
pub struct Foo {
    _x: i32,
}
const _: () = {
    unsafe impl ::safer_ffi::::ReprC for Foo {
        type CLayout = __opaque_Foo;
        fn is_valid(it: &'_ Self::CLayout) -> ::safer_ffi::::bool {
            match it._void {}
        }
    }
    #[allow(nonstandard_style)]
    pub struct __opaque_Foo {
        _marker: *mut Self,
        _void: ::safer_ffi::::convert::Infallible,
    }
    impl ::safer_ffi::::marker::Copy for __opaque_Foo {}
    impl ::safer_ffi::::clone::Clone for __opaque_Foo {
        fn clone(self: &'_ Self) -> Self {
            match self._void {}
        }
    }
    unsafe impl ::safer_ffi::::CType for __opaque_Foo {
        type OPAQUE_KIND = ::safer_ffi::::OpaqueKind::Opaque;
    }
    unsafe impl ::safer_ffi::::ReprC for __opaque_Foo {
        type CLayout = Self;
        fn is_valid(_: &'_ Self::CLayout) -> ::safer_ffi::::bool {
            true
        }
    }
};

The lint is likely due to the Clone::clone implementation being different from *self.

Potential fixes are:

  1. Implementing Clone::clone as returning *self.
  2. Allowing the clippy lint in the generated code.

(2) may be tricky without bumping the minimum Rust version to 1.72.0.

Let me know if you would like me to open a PR to fix this.

@stefunctional
Copy link
Contributor Author

Suggested fix (1) is implemented in stefunctional@7db6bc6. If you think this solution is appropriate and a PR would help, please let me know.

@cdata
Copy link

cdata commented Aug 28, 2023

Just came here to report this issue 👍 ran into it here: https://github.com/subconsciousnetwork/noosphere/actions/runs/6004740385/job/16285965451?pr=608

@danielhenrymantilla
Copy link
Collaborator

Thanks for the report! stefunctional@7db6bc6 does LGTM, so if you submit a PR with it I'll merge it 🙂

@stefunctional
Copy link
Contributor Author

I opened #184 as suggested. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants