From 46668b43e9c4d814a5f9139b949a6b92b23c57c5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 12 Jun 2024 09:24:57 +1000 Subject: [PATCH] Remove the `missing_copy_implementations` lint. This lint made a certain amount of sense for people writing code during the period leading up to to Rust 1.0. It doesn't make much sense today. There are many valid cases of types that could impl `Copy` but do not. The lint is extremely low value and moderately complicated. This commit removes it. --- compiler/rustc_lint/messages.ftl | 2 - compiler/rustc_lint/src/builtin.rs | 158 +----------------- compiler/rustc_lint/src/lib.rs | 7 +- compiler/rustc_lint/src/lints.rs | 4 - library/std/src/io/util.rs | 2 - .../crates/ide-db/src/generated/lints.rs | 4 - tests/ui/lint/issue-111359.rs | 2 - tests/ui/lint/issue-111359.stderr | 16 +- ...lint-missing-copy-implementations-allow.rs | 35 ---- .../lint/lint-missing-copy-implementations.rs | 15 -- .../lint-missing-copy-implementations.stderr | 16 -- ...sing-copy-implementations-negative-copy.rs | 15 -- ...ing-copy-implementations-non-exhaustive.rs | 25 --- tests/ui/lint/reasons-erroneous.rs | 2 +- tests/ui/lint/reasons-erroneous.stderr | 6 +- tests/ui/traits/issue-22019.rs | 1 - 16 files changed, 16 insertions(+), 294 deletions(-) delete mode 100644 tests/ui/lint/lint-missing-copy-implementations-allow.rs delete mode 100644 tests/ui/lint/lint-missing-copy-implementations.rs delete mode 100644 tests/ui/lint/lint-missing-copy-implementations.stderr delete mode 100644 tests/ui/lint/missing-copy-implementations-negative-copy.rs delete mode 100644 tests/ui/lint/missing-copy-implementations-non-exhaustive.rs diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index 0c236a4ed11f3..45a52c666d7a5 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -110,8 +110,6 @@ lint_builtin_link_section_fn = declaration of a function with `link_section` lint_builtin_link_section_static = declaration of a static with `link_section` -lint_builtin_missing_copy_impl = type could implement `Copy`; consider adding `impl Copy` - lint_builtin_missing_debug_impl = type does not implement `{$debug}`; consider adding `#[derive(Debug)]` or a manual implementation diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 8c9abeafacfe7..371a209552489 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -29,11 +29,11 @@ use crate::{ BuiltinDerefNullptr, BuiltinEllipsisInclusiveRangePatternsLint, BuiltinExplicitOutlives, BuiltinExplicitOutlivesSuggestion, BuiltinFeatureIssueNote, BuiltinIncompleteFeatures, BuiltinIncompleteFeaturesHelp, BuiltinInternalFeatures, BuiltinKeywordIdents, - BuiltinMissingCopyImpl, BuiltinMissingDebugImpl, BuiltinMissingDoc, - BuiltinMutablesTransmutes, BuiltinNamedAsmLabel, BuiltinNoMangleGeneric, - BuiltinNonShorthandFieldPatterns, BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, - BuiltinTypeAliasGenericBounds, BuiltinTypeAliasGenericBoundsSuggestion, - BuiltinTypeAliasWhereClause, BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, + BuiltinMissingDebugImpl, BuiltinMissingDoc, BuiltinMutablesTransmutes, + BuiltinNamedAsmLabel, BuiltinNoMangleGeneric, BuiltinNonShorthandFieldPatterns, + BuiltinSpecialModuleNameUsed, BuiltinTrivialBounds, BuiltinTypeAliasGenericBounds, + BuiltinTypeAliasGenericBoundsSuggestion, BuiltinTypeAliasWhereClause, + BuiltinUngatedAsyncFnTrackCaller, BuiltinUnpermittedTypeInit, BuiltinUnpermittedTypeInitSub, BuiltinUnreachablePub, BuiltinUnsafe, BuiltinUnstableFeatures, BuiltinUnusedDocComment, BuiltinUnusedDocCommentSub, BuiltinWhileTrue, SuggestChangingAssocTypes, @@ -54,11 +54,9 @@ use rustc_hir::intravisit::FnKind as HirFnKind; use rustc_hir::{Body, FnDecl, GenericParamKind, PatKind, PredicateOrigin}; use rustc_middle::bug; use rustc_middle::lint::in_external_macro; -use rustc_middle::ty::layout::LayoutOf; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::GenericArgKind; use rustc_middle::ty::TypeVisitableExt; -use rustc_middle::ty::Upcast; use rustc_middle::ty::{self, Ty, TyCtxt, VariantDef}; use rustc_session::lint::FutureIncompatibilityReason; use rustc_session::{declare_lint, declare_lint_pass, impl_lint_pass}; @@ -67,9 +65,6 @@ use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{BytePos, InnerSpan, Span}; use rustc_target::abi::Abi; -use rustc_trait_selection::infer::{InferCtxtExt, TyCtxtInferExt}; -use rustc_trait_selection::traits::query::evaluate_obligation::InferCtxtExt as _; -use rustc_trait_selection::traits::{self, misc::type_allowed_to_implement_copy}; use tracing::debug; use crate::nonstandard_style::{method_context, MethodLateContext}; @@ -598,148 +593,6 @@ impl<'tcx> LateLintPass<'tcx> for MissingDoc { } } -declare_lint! { - /// The `missing_copy_implementations` lint detects potentially-forgotten - /// implementations of [`Copy`] for public types. - /// - /// [`Copy`]: https://doc.rust-lang.org/std/marker/trait.Copy.html - /// - /// ### Example - /// - /// ```rust,compile_fail - /// #![deny(missing_copy_implementations)] - /// pub struct Foo { - /// pub field: i32 - /// } - /// # fn main() {} - /// ``` - /// - /// {{produces}} - /// - /// ### Explanation - /// - /// Historically (before 1.0), types were automatically marked as `Copy` - /// if possible. This was changed so that it required an explicit opt-in - /// by implementing the `Copy` trait. As part of this change, a lint was - /// added to alert if a copyable type was not marked `Copy`. - /// - /// This lint is "allow" by default because this code isn't bad; it is - /// common to write newtypes like this specifically so that a `Copy` type - /// is no longer `Copy`. `Copy` types can result in unintended copies of - /// large data which can impact performance. - pub MISSING_COPY_IMPLEMENTATIONS, - Allow, - "detects potentially-forgotten implementations of `Copy`" -} - -declare_lint_pass!(MissingCopyImplementations => [MISSING_COPY_IMPLEMENTATIONS]); - -impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations { - fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) { - if !cx.effective_visibilities.is_reachable(item.owner_id.def_id) { - return; - } - let (def, ty) = match item.kind { - hir::ItemKind::Struct(_, ast_generics) => { - if !ast_generics.params.is_empty() { - return; - } - let def = cx.tcx.adt_def(item.owner_id); - (def, Ty::new_adt(cx.tcx, def, ty::List::empty())) - } - hir::ItemKind::Union(_, ast_generics) => { - if !ast_generics.params.is_empty() { - return; - } - let def = cx.tcx.adt_def(item.owner_id); - (def, Ty::new_adt(cx.tcx, def, ty::List::empty())) - } - hir::ItemKind::Enum(_, ast_generics) => { - if !ast_generics.params.is_empty() { - return; - } - let def = cx.tcx.adt_def(item.owner_id); - (def, Ty::new_adt(cx.tcx, def, ty::List::empty())) - } - _ => return, - }; - if def.has_dtor(cx.tcx) { - return; - } - - // If the type contains a raw pointer, it may represent something like a handle, - // and recommending Copy might be a bad idea. - for field in def.all_fields() { - let did = field.did; - if cx.tcx.type_of(did).instantiate_identity().is_unsafe_ptr() { - return; - } - } - if ty.is_copy_modulo_regions(cx.tcx, cx.param_env) { - return; - } - if type_implements_negative_copy_modulo_regions(cx.tcx, ty, cx.param_env) { - return; - } - if def.is_variant_list_non_exhaustive() - || def.variants().iter().any(|variant| variant.is_field_list_non_exhaustive()) - { - return; - } - - // We shouldn't recommend implementing `Copy` on stateful things, - // such as iterators. - if let Some(iter_trait) = cx.tcx.get_diagnostic_item(sym::Iterator) - && cx - .tcx - .infer_ctxt() - .build() - .type_implements_trait(iter_trait, [ty], cx.param_env) - .must_apply_modulo_regions() - { - return; - } - - // Default value of clippy::trivially_copy_pass_by_ref - const MAX_SIZE: u64 = 256; - - if let Some(size) = cx.layout_of(ty).ok().map(|l| l.size.bytes()) { - if size > MAX_SIZE { - return; - } - } - - if type_allowed_to_implement_copy( - cx.tcx, - cx.param_env, - ty, - traits::ObligationCause::misc(item.span, item.owner_id.def_id), - ) - .is_ok() - { - cx.emit_span_lint(MISSING_COPY_IMPLEMENTATIONS, item.span, BuiltinMissingCopyImpl); - } - } -} - -/// Check whether a `ty` has a negative `Copy` implementation, ignoring outlives constraints. -fn type_implements_negative_copy_modulo_regions<'tcx>( - tcx: TyCtxt<'tcx>, - ty: Ty<'tcx>, - param_env: ty::ParamEnv<'tcx>, -) -> bool { - let trait_ref = ty::TraitRef::new(tcx, tcx.require_lang_item(hir::LangItem::Copy, None), [ty]); - let pred = ty::TraitPredicate { trait_ref, polarity: ty::PredicatePolarity::Negative }; - let obligation = traits::Obligation { - cause: traits::ObligationCause::dummy(), - param_env, - recursion_depth: 0, - predicate: pred.upcast(tcx), - }; - - tcx.infer_ctxt().build().predicate_must_hold_modulo_regions(&obligation) -} - declare_lint! { /// The `missing_debug_implementations` lint detects missing /// implementations of [`fmt::Debug`] for public types. @@ -1644,7 +1497,6 @@ declare_lint_pass!( NON_SHORTHAND_FIELD_PATTERNS, UNSAFE_CODE, MISSING_DOCS, - MISSING_COPY_IMPLEMENTATIONS, MISSING_DEBUG_IMPLEMENTATIONS, ANONYMOUS_PARAMETERS, UNUSED_DOC_COMMENTS, diff --git a/compiler/rustc_lint/src/lib.rs b/compiler/rustc_lint/src/lib.rs index bcb714ae4ce2a..29d71a0dcca01 100644 --- a/compiler/rustc_lint/src/lib.rs +++ b/compiler/rustc_lint/src/lib.rs @@ -195,8 +195,6 @@ late_lint_methods!( NonUpperCaseGlobals: NonUpperCaseGlobals, NonShorthandFieldPatterns: NonShorthandFieldPatterns, UnusedAllocation: UnusedAllocation, - // Depends on types used in type definitions - MissingCopyImplementations: MissingCopyImplementations, // Depends on referenced function signatures in expressions PtrNullChecks: PtrNullChecks, MutableTransmutes: MutableTransmutes, @@ -549,6 +547,11 @@ fn register_builtins(store: &mut LintStore) { "converted into hard error, see RFC #3535 \ for more information", ); + store.register_removed( + "missing_copy_implementations", + "it existed mostly for historical reasons, \ + and not implementing `Copy` is common and reasonable", + ); } fn register_internals(store: &mut LintStore) { diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index c493a989d913a..4ba1f49854714 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -133,10 +133,6 @@ pub struct BuiltinMissingDoc<'a> { pub desc: &'a str, } -#[derive(LintDiagnostic)] -#[diag(lint_builtin_missing_copy_impl)] -pub struct BuiltinMissingCopyImpl; - pub struct BuiltinMissingDebugImpl<'a> { pub tcx: TyCtxt<'a>, pub def_id: DefId, diff --git a/library/std/src/io/util.rs b/library/std/src/io/util.rs index b4c4dffc371c1..3d3acb5914bb8 100644 --- a/library/std/src/io/util.rs +++ b/library/std/src/io/util.rs @@ -1,5 +1,3 @@ -#![allow(missing_copy_implementations)] - #[cfg(test)] mod tests; diff --git a/src/tools/rust-analyzer/crates/ide-db/src/generated/lints.rs b/src/tools/rust-analyzer/crates/ide-db/src/generated/lints.rs index c92d4e78ffa71..25d889babca8d 100644 --- a/src/tools/rust-analyzer/crates/ide-db/src/generated/lints.rs +++ b/src/tools/rust-analyzer/crates/ide-db/src/generated/lints.rs @@ -305,10 +305,6 @@ pub const DEFAULT_LINTS: &[Lint] = &[ description: r##"possible meta-variable misuse at macro definition"##, }, Lint { label: "missing_abi", description: r##"No declared ABI for extern declaration"## }, - Lint { - label: "missing_copy_implementations", - description: r##"detects potentially-forgotten implementations of `Copy`"##, - }, Lint { label: "missing_debug_implementations", description: r##"detects missing implementations of Debug"##, diff --git a/tests/ui/lint/issue-111359.rs b/tests/ui/lint/issue-111359.rs index e390c3fc565a6..3fd3334bd0541 100644 --- a/tests/ui/lint/issue-111359.rs +++ b/tests/ui/lint/issue-111359.rs @@ -1,12 +1,10 @@ #[deny(missing_debug_implementations)] -#[deny(missing_copy_implementations)] mod priv_mod { use std::convert::TryFrom; pub struct BarPub; //~^ ERROR type does not implement `Debug`; consider adding `#[derive(Debug)]` or a manual implementation - //~| ERROR type could implement `Copy`; consider adding `impl Copy` struct BarPriv; impl<'a> TryFrom for u8 { diff --git a/tests/ui/lint/issue-111359.stderr b/tests/ui/lint/issue-111359.stderr index 0aef5007a2bc5..172a3a8885d64 100644 --- a/tests/ui/lint/issue-111359.stderr +++ b/tests/ui/lint/issue-111359.stderr @@ -1,17 +1,5 @@ -error: type could implement `Copy`; consider adding `impl Copy` - --> $DIR/issue-111359.rs:7:5 - | -LL | pub struct BarPub; - | ^^^^^^^^^^^^^^^^^^ - | -note: the lint level is defined here - --> $DIR/issue-111359.rs:2:8 - | -LL | #[deny(missing_copy_implementations)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - error: type does not implement `Debug`; consider adding `#[derive(Debug)]` or a manual implementation - --> $DIR/issue-111359.rs:7:5 + --> $DIR/issue-111359.rs:6:5 | LL | pub struct BarPub; | ^^^^^^^^^^^^^^^^^^ @@ -22,5 +10,5 @@ note: the lint level is defined here LL | #[deny(missing_debug_implementations)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 2 previous errors +error: aborting due to 1 previous error diff --git a/tests/ui/lint/lint-missing-copy-implementations-allow.rs b/tests/ui/lint/lint-missing-copy-implementations-allow.rs deleted file mode 100644 index d688dfe95eeca..0000000000000 --- a/tests/ui/lint/lint-missing-copy-implementations-allow.rs +++ /dev/null @@ -1,35 +0,0 @@ -//@ check-pass -#![deny(missing_copy_implementations)] - -// Don't recommend implementing Copy on something stateful like an iterator. -pub struct MyIterator { - num: u8, -} - -impl Iterator for MyIterator { - type Item = u8; - - fn next(&mut self) -> Option { - todo!() - } -} - -pub struct Handle { - inner: *mut (), -} - -pub struct Handle2 { - inner: *const (), -} - -pub enum MaybeHandle { - Ptr(*mut ()), -} - -pub union UnionHandle { - ptr: *mut (), -} - -pub struct Array([u8; 2048]); - -fn main() {} diff --git a/tests/ui/lint/lint-missing-copy-implementations.rs b/tests/ui/lint/lint-missing-copy-implementations.rs deleted file mode 100644 index 918f40de15378..0000000000000 --- a/tests/ui/lint/lint-missing-copy-implementations.rs +++ /dev/null @@ -1,15 +0,0 @@ -// See issue 19712 - -#![deny(missing_copy_implementations)] - -mod inner { - pub struct Foo { //~ ERROR type could implement `Copy`; consider adding `impl Copy` - pub field: i32 - } -} - -pub fn foo() -> inner::Foo { - inner::Foo { field: 42 } -} - -fn main() {} diff --git a/tests/ui/lint/lint-missing-copy-implementations.stderr b/tests/ui/lint/lint-missing-copy-implementations.stderr deleted file mode 100644 index 37c9192d2a49c..0000000000000 --- a/tests/ui/lint/lint-missing-copy-implementations.stderr +++ /dev/null @@ -1,16 +0,0 @@ -error: type could implement `Copy`; consider adding `impl Copy` - --> $DIR/lint-missing-copy-implementations.rs:6:5 - | -LL | / pub struct Foo { -LL | | pub field: i32 -LL | | } - | |_____^ - | -note: the lint level is defined here - --> $DIR/lint-missing-copy-implementations.rs:3:9 - | -LL | #![deny(missing_copy_implementations)] - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - -error: aborting due to 1 previous error - diff --git a/tests/ui/lint/missing-copy-implementations-negative-copy.rs b/tests/ui/lint/missing-copy-implementations-negative-copy.rs deleted file mode 100644 index 7860f54e7f32e..0000000000000 --- a/tests/ui/lint/missing-copy-implementations-negative-copy.rs +++ /dev/null @@ -1,15 +0,0 @@ -// Regression test for issue #101980. -// Ensure that we don't suggest impl'ing `Copy` for a type if it already impl's `!Copy`. - -//@ check-pass - -#![feature(negative_impls)] -#![deny(missing_copy_implementations)] - -pub struct Struct { - pub field: i32, -} - -impl !Copy for Struct {} - -fn main() {} diff --git a/tests/ui/lint/missing-copy-implementations-non-exhaustive.rs b/tests/ui/lint/missing-copy-implementations-non-exhaustive.rs deleted file mode 100644 index 16f448674b29a..0000000000000 --- a/tests/ui/lint/missing-copy-implementations-non-exhaustive.rs +++ /dev/null @@ -1,25 +0,0 @@ -// Test for issue #116766. -// Ensure that we don't suggest impl'ing `Copy` for a type if it or at least one -// of it's variants are marked as `non_exhaustive`. - -//@ check-pass - -#![deny(missing_copy_implementations)] - -#[non_exhaustive] -pub enum MyEnum { - A, -} - -#[non_exhaustive] -pub struct MyStruct { - foo: usize, -} - -pub enum MyEnum2 { - #[non_exhaustive] - A, - B, -} - -fn main() {} diff --git a/tests/ui/lint/reasons-erroneous.rs b/tests/ui/lint/reasons-erroneous.rs index 7366a03232f3f..6e2e973fe766a 100644 --- a/tests/ui/lint/reasons-erroneous.rs +++ b/tests/ui/lint/reasons-erroneous.rs @@ -23,7 +23,7 @@ #![warn(keyword_idents, reason = "root in rubble", macro_use_extern_crate)] //~^ ERROR malformed lint attribute //~| NOTE reason in lint attribute must come last -#![warn(missing_copy_implementations, reason)] +#![warn(missing_debug_implementations, reason)] //~^ WARN unknown lint //~| NOTE `#[warn(unknown_lints)]` on by default diff --git a/tests/ui/lint/reasons-erroneous.stderr b/tests/ui/lint/reasons-erroneous.stderr index 003da5673704f..6b79760e3cdc5 100644 --- a/tests/ui/lint/reasons-erroneous.stderr +++ b/tests/ui/lint/reasons-erroneous.stderr @@ -41,10 +41,10 @@ LL | #![warn(keyword_idents, reason = "root in rubble", macro_use_extern_crate)] | ^^^^^^^^^^^^^^^^^^^^^^^^^ reason in lint attribute must come last warning: unknown lint: `reason` - --> $DIR/reasons-erroneous.rs:26:39 + --> $DIR/reasons-erroneous.rs:26:40 | -LL | #![warn(missing_copy_implementations, reason)] - | ^^^^^^ +LL | #![warn(missing_debug_implementations, reason)] + | ^^^^^^ | = note: `#[warn(unknown_lints)]` on by default diff --git a/tests/ui/traits/issue-22019.rs b/tests/ui/traits/issue-22019.rs index 120f611ccb69c..060396de79546 100644 --- a/tests/ui/traits/issue-22019.rs +++ b/tests/ui/traits/issue-22019.rs @@ -5,7 +5,6 @@ //@ pretty-expanded FIXME #23616 -#![allow(missing_copy_implementations)] #![allow(unused_variables)] pub struct CFGNode;