From c6566a8037f9b3597df741f0ce1b59289441bd23 Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Mon, 24 Jul 2023 19:58:13 +0200 Subject: [PATCH 1/3] Explain more clearly why `fn() -> T` can't be `#[derive(Clone)]` --- library/core/src/clone.rs | 40 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index a6d6230d3a62b..d7ca9c22dada8 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -86,6 +86,46 @@ /// } /// ``` /// +/// If we `derive`: +/// +/// ``` +/// #[derive(Copy, Clone)] +/// struct Generate(fn() -> T); +/// ``` +/// +/// the auto-derived implementations will have unnecessary `T: Copy` and `T: Clone` bounds: +/// +/// ``` +/// # struct Generate(fn() -> T); +/// +/// // Automatically derived +/// impl Copy for Generate { } +/// +/// // Automatically derived +/// impl Clone for Generate { +/// fn clone(&self) -> Generate { +/// Generate(Clone::clone(&self.0)) +/// } +/// } +/// ``` +/// +/// The bounds are unnecessary because clearly the function itself should be +/// copy- and cloneable even if its return type is not: +/// +/// ```compile_fail,E0599 +/// #[derive(Copy, Clone)] +/// struct Generate(fn() -> T); +/// +/// struct NotCloneable; +/// +/// fn generate_not_cloneable() -> NotCloneable { +/// NotCloneable +/// } +/// +/// Generate(generate_not_cloneable).clone(); // error: trait bounds were not satisfied +/// // Note: With the manual implementations the above line will compile. +/// ``` +/// /// ## Additional implementors /// /// In addition to the [implementors listed below][impls], From 2b8a3b44bfa63c729841c29d70cc2d3e66ca3842 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Sun, 30 Jul 2023 13:33:26 +0200 Subject: [PATCH 2/3] Make lint missing-copy-implementations honor negative Copy impls --- compiler/rustc_lint/src/builtin.rs | 23 +++++++++++++++++++ ...sing-copy-implementations-negative-copy.rs | 15 ++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 tests/ui/lint/missing-copy-implementations-negative-copy.rs diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index e6917f4b2d3b7..401e0ad734b41 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -62,6 +62,7 @@ use rustc_middle::lint::in_external_macro; use rustc_middle::ty::layout::{LayoutError, LayoutOf}; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::GenericArgKind; +use rustc_middle::ty::ToPredicate; use rustc_middle::ty::TypeVisitableExt; use rustc_middle::ty::{self, Instance, Ty, TyCtxt, VariantDef}; use rustc_session::config::ExpectedValues; @@ -72,6 +73,7 @@ use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{BytePos, InnerSpan, Span}; use rustc_target::abi::{Abi, FIRST_VARIANT}; 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 crate::nonstandard_style::{method_context, MethodLateContext}; @@ -710,6 +712,9 @@ impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations { if ty.is_copy_modulo_regions(cx.tcx, param_env) { return; } + if type_implements_negative_copy_modulo_regions(cx.tcx, ty, param_env) { + return; + } // We shouldn't recommend implementing `Copy` on stateful things, // such as iterators. @@ -745,6 +750,24 @@ impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations { } } +/// 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::ImplPolarity::Negative }; + let obligation = traits::Obligation { + cause: traits::ObligationCause::dummy(), + param_env, + recursion_depth: 0, + predicate: ty::Binder::dummy(pred).to_predicate(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. diff --git a/tests/ui/lint/missing-copy-implementations-negative-copy.rs b/tests/ui/lint/missing-copy-implementations-negative-copy.rs new file mode 100644 index 0000000000000..b29d2209fa9f9 --- /dev/null +++ b/tests/ui/lint/missing-copy-implementations-negative-copy.rs @@ -0,0 +1,15 @@ +// 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() {} From 88cb2bba15ebae39ebb13a35d52152e58f2d054a Mon Sep 17 00:00:00 2001 From: yukang Date: Sat, 5 Aug 2023 12:44:20 +0800 Subject: [PATCH 3/3] Print tidy command with bless tidy check failure --- src/tools/tidy/src/fluent_alphabetical.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/tidy/src/fluent_alphabetical.rs b/src/tools/tidy/src/fluent_alphabetical.rs index 5f8eaebf531fd..67b745373f019 100644 --- a/src/tools/tidy/src/fluent_alphabetical.rs +++ b/src/tools/tidy/src/fluent_alphabetical.rs @@ -23,7 +23,7 @@ fn check_alphabetic(filename: &str, fluent: &str, bad: &mut bool) { tidy_error!( bad, "{filename}: message `{}` appears before `{}`, but is alphabetically later than it -run tidy with `--bless` to sort the file correctly", +run `./x.py test tidy --bless` to sort the file correctly", name.as_str(), next.as_str() );