Skip to content

The allow-by-default lint missing_copy_implementations does not honor !Copy impls #101980

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

Closed
fmease opened this issue Sep 18, 2022 · 3 comments · Fixed by #114248
Closed

The allow-by-default lint missing_copy_implementations does not honor !Copy impls #101980

fmease opened this issue Sep 18, 2022 · 3 comments · Fixed by #114248
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. F-negative_impls #![feature(negative_impls)]

Comments

@fmease
Copy link
Member

fmease commented Sep 18, 2022

I would expect the following code to compile successfully since we explicitly opt out of implementing Copy:

#![feature(negative_impls)]
#![deny(missing_copy_implementations)]
#![crate_type = "lib"]

pub struct Struct {
    pub field: i32,
}

impl !Copy for Struct {}

However, the code fails to compile with the following error message:

error: type could implement `Copy`; consider adding `impl Copy`
 --> src/lib.rs:5:1
  |
5 | / pub struct Struct {
6 | |     pub field: i32,
7 | | }
  | |_^
  |
note: the lint level is defined here
 --> src/lib.rs:2:9
  |
2 | #![deny(missing_copy_implementations)]
  |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^

@rustbot label C-bug A-lint F-negative_impls

@rustbot rustbot added A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. F-negative_impls #![feature(negative_impls)] labels Sep 18, 2022
@fmease
Copy link
Member Author

fmease commented Sep 18, 2022

The compiler only checks if the type implements Copy already to return early and not emit the lint diagnostic but it doesn't check if the type implements !Copy:

if ty.is_copy_modulo_regions(cx.tcx.at(item.span), param_env) {

@vincenzopalazzo
Copy link
Member

I think this makes the trick, you are right! the last think is to explore what is the code of the method

pub fn is_copy_modulo_regions(
self,
tcx_at: TyCtxtAt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
) -> bool {
self.is_trivially_pure_clone_copy() || tcx_at.is_copy_raw(param_env.and(self))
}

The code of the method looks not so trivial :)

pub fn is_trivially_pure_clone_copy(self) -> bool {
match self.kind() {
ty::Bool | ty::Char | ty::Never => true,
// These aren't even `Clone`
ty::Str | ty::Slice(..) | ty::Foreign(..) | ty::Dynamic(..) => false,
ty::Infer(ty::InferTy::FloatVar(_) | ty::InferTy::IntVar(_))
| ty::Int(..)
| ty::Uint(..)
| ty::Float(..) => true,
// The voldemort ZSTs are fine.
ty::FnDef(..) => true,
ty::Array(element_ty, _len) => element_ty.is_trivially_pure_clone_copy(),
// A 100-tuple isn't "trivial", so doing this only for reasonable sizes.
ty::Tuple(field_tys) => {
field_tys.len() <= 3 && field_tys.iter().all(Self::is_trivially_pure_clone_copy)
}
// Sometimes traits aren't implemented for every ABI or arity,
// because we can't be generic over everything yet.
ty::FnPtr(..) => false,
// Definitely absolutely not copy.
ty::Ref(_, _, hir::Mutability::Mut) => false,
// Thin pointers & thin shared references are pure-clone-copy, but for
// anything with custom metadata it might be more complicated.
ty::Ref(_, _, hir::Mutability::Not) | ty::RawPtr(..) => false,
ty::Generator(..) | ty::GeneratorWitness(..) => false,
// Might be, but not "trivial" so just giving the safe answer.
ty::Adt(..) | ty::Closure(..) | ty::Opaque(..) => false,
ty::Projection(..) | ty::Param(..) | ty::Infer(..) | ty::Error(..) => false,
ty::Bound(..) | ty::Placeholder(..) => {
bug!("`is_trivially_pure_clone_copy` applied to unexpected type: {:?}", self);
}
}
}

If you did not find already I can play with your on it by mentoring you, I had some experience in "git archeology" :)

@rustbot label +E-mentor

@rustbot rustbot added the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label Oct 14, 2022
@Kohei316
Copy link
Contributor

Kohei316 commented Mar 8, 2023

@rustbot claim

@Kohei316 Kohei316 removed their assignment Jul 7, 2023
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Aug 5, 2023
…copy-impl, r=b-naber

Make lint missing-copy-implementations honor negative `Copy` impls

Fixes rust-lang#101980.

`@rustbot` label A-lint F-negative_impls
@bors bors closed this as completed in e722f6f Aug 5, 2023
@fmease fmease removed the E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. label May 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-bug Category: This is a bug. F-negative_impls #![feature(negative_impls)]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants