Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 8 additions & 1 deletion compiler/rustc_resolve/src/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,7 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
// is applied, so they have to be produced by the container's expansion rather
// than by individual derives.
// - Derives in the container need to know whether one of them is a built-in `Copy`.
// (But see the comment mentioning #124794 below.)
// Temporarily take the data to avoid borrow checker conflicts.
let mut derive_data = mem::take(&mut self.derive_data);
let entry = derive_data.entry(expn_id).or_insert_with(|| DeriveData {
Expand Down Expand Up @@ -440,7 +441,13 @@ impl<'ra, 'tcx> ResolverExpand for Resolver<'ra, 'tcx> {
.collect();
self.helper_attrs.insert(expn_id, helper_attrs);
// Mark this derive as having `Copy` either if it has `Copy` itself or if its parent derive
// has `Copy`, to support cases like `#[derive(Clone, Copy)] #[derive(Debug)]`.
// has `Copy`, to support `#[derive(Copy, Clone)]`, `#[derive(Clone, Copy)]`, or
// `#[derive(Copy)] #[derive(Clone)]`. We do this because the code generated for
// `derive(Clone)` changes if `derive(Copy)` is also present.
//
// FIXME(#124794): unfortunately this doesn't work with `#[derive(Clone)] #[derive(Copy)]`.
// When the `Clone` impl is generated the `#[derive(Copy)]` hasn't been processed and
// `has_derive_copy` hasn't been set yet.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done as a MIR pass, or something similar, IMO, instead of having a special hack in resolve.

The current optimization is a terrible hack, that is a best effort attempt to improve on the previous similar optimization hack that was both terrible and incorrect.

if entry.has_derive_copy || self.has_derive_copy(parent_scope.expansion) {
self.containers_deriving_copy.insert(expn_id);
}
Expand Down
31 changes: 24 additions & 7 deletions tests/ui/deriving/deriving-all-codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,15 @@ use std::from::From;
#[derive(Clone, Copy, Debug, Default, Hash, PartialEq, Eq, PartialOrd, Ord)]
struct Empty;

// A basic struct. Note: because this derives `Copy`, it gets the simple
// A basic struct. Note: because this derives `Copy`, it gets the trivial
// `clone` implemention that just does `*self`.
#[derive(Clone, Copy, Debug, Default, Hash, PartialEq, Eq, PartialOrd, Ord)]
struct Point {
x: u32,
y: u32,
}

// A basic packed struct. Note: because this derives `Copy`, it gets the simple
// A basic packed struct. Note: because this derives `Copy`, it gets the trivial
// `clone` implemention that just does `*self`.
#[derive(Clone, Copy, Debug, Default, Hash, PartialEq, Eq, PartialOrd, Ord)]
#[repr(packed)]
Expand All @@ -49,7 +49,7 @@ struct SingleField {
foo: bool,
}

// A large struct. Note: because this derives `Copy`, it gets the simple
// A large struct. Note: because this derives `Copy`, it gets the trivial
// `clone` implemention that just does `*self`.
#[derive(Clone, Copy, Debug, Default, Hash, PartialEq, Eq, PartialOrd, Ord)]
struct Big {
Expand Down Expand Up @@ -79,25 +79,25 @@ struct Reorder {
b10: &'static *const bool,
}

// A struct that doesn't impl `Copy`, which means it gets the non-simple
// A struct that doesn't impl `Copy`, which means it gets the non-trivial
// `clone` implemention that clones the fields individually.
#[derive(Clone)]
struct NonCopy(u32);

// A packed struct that doesn't impl `Copy`, which means it gets the non-simple
// A packed struct that doesn't impl `Copy`, which means it gets the non-trivial
// `clone` implemention that clones the fields individually.
#[derive(Clone)]
#[repr(packed)]
struct PackedNonCopy(u32);

// A struct that impls `Copy` manually, which means it gets the non-simple
// A struct that impls `Copy` manually, which means it gets the non-trivial
// `clone` implemention that clones the fields individually.
#[derive(Clone)]
struct ManualCopy(u32);
impl Copy for ManualCopy {}

// A packed struct that impls `Copy` manually, which means it gets the
// non-simple `clone` implemention that clones the fields individually.
// non-trivial `clone` implemention that clones the fields individually.
#[derive(Clone)]
#[repr(packed)]
struct PackedManualCopy(u32);
Expand Down Expand Up @@ -218,3 +218,20 @@ pub union Union {
pub u: u32,
pub i: i32,
}

#[derive(Copy, Clone)]
struct FooCopyClone(i32);

#[derive(Clone, Copy)]
struct FooCloneCopy(i32);

#[derive(Copy)]
#[derive(Clone)]
struct FooCopyAndClone(i32);

// FIXME(#124794): the previous three structs all have a trivial `Copy`-aware
// `clone`. But this one doesn't because when the `clone` is generated the
// `derive(Copy)` hasn't yet been seen.
#[derive(Clone)]
#[derive(Copy)]
struct FooCloneAndCopy(i32);
73 changes: 66 additions & 7 deletions tests/ui/deriving/deriving-all-codegen.stdout
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl ::core::cmp::Ord for Empty {
}
}

// A basic struct. Note: because this derives `Copy`, it gets the simple
// A basic struct. Note: because this derives `Copy`, it gets the trivial
// `clone` implemention that just does `*self`.
struct Point {
x: u32,
Expand Down Expand Up @@ -171,7 +171,7 @@ impl ::core::cmp::Ord for Point {
}
}

// A basic packed struct. Note: because this derives `Copy`, it gets the simple
// A basic packed struct. Note: because this derives `Copy`, it gets the trivial
// `clone` implemention that just does `*self`.
#[repr(packed)]
struct PackedPoint {
Expand Down Expand Up @@ -409,7 +409,7 @@ impl ::core::cmp::Ord for SingleField {
}
}

// A large struct. Note: because this derives `Copy`, it gets the simple
// A large struct. Note: because this derives `Copy`, it gets the trivial
// `clone` implemention that just does `*self`.
struct Big {
b1: u32,
Expand Down Expand Up @@ -676,7 +676,7 @@ impl ::core::cmp::PartialOrd for Reorder {
}
}

// A struct that doesn't impl `Copy`, which means it gets the non-simple
// A struct that doesn't impl `Copy`, which means it gets the non-trivial
// `clone` implemention that clones the fields individually.
struct NonCopy(u32);
#[automatically_derived]
Expand All @@ -687,7 +687,7 @@ impl ::core::clone::Clone for NonCopy {
}
}

// A packed struct that doesn't impl `Copy`, which means it gets the non-simple
// A packed struct that doesn't impl `Copy`, which means it gets the non-trivial
// `clone` implemention that clones the fields individually.
#[repr(packed)]
struct PackedNonCopy(u32);
Expand All @@ -699,7 +699,7 @@ impl ::core::clone::Clone for PackedNonCopy {
}
}

// A struct that impls `Copy` manually, which means it gets the non-simple
// A struct that impls `Copy` manually, which means it gets the non-trivial
// `clone` implemention that clones the fields individually.
struct ManualCopy(u32);
#[automatically_derived]
Expand All @@ -712,7 +712,7 @@ impl ::core::clone::Clone for ManualCopy {
impl Copy for ManualCopy {}

// A packed struct that impls `Copy` manually, which means it gets the
// non-simple `clone` implemention that clones the fields individually.
// non-trivial `clone` implemention that clones the fields individually.
#[repr(packed)]
struct PackedManualCopy(u32);
#[automatically_derived]
Expand Down Expand Up @@ -1791,3 +1791,62 @@ impl ::core::clone::Clone for Union {
}
#[automatically_derived]
impl ::core::marker::Copy for Union { }

struct FooCopyClone(i32);
#[automatically_derived]
impl ::core::marker::Copy for FooCopyClone { }
#[automatically_derived]
#[doc(hidden)]
unsafe impl ::core::clone::TrivialClone for FooCopyClone { }
#[automatically_derived]
impl ::core::clone::Clone for FooCopyClone {
#[inline]
fn clone(&self) -> FooCopyClone {
let _: ::core::clone::AssertParamIsClone<i32>;
*self
}
}

struct FooCloneCopy(i32);
#[automatically_derived]
#[doc(hidden)]
unsafe impl ::core::clone::TrivialClone for FooCloneCopy { }
#[automatically_derived]
impl ::core::clone::Clone for FooCloneCopy {
#[inline]
fn clone(&self) -> FooCloneCopy {
let _: ::core::clone::AssertParamIsClone<i32>;
*self
}
}
#[automatically_derived]
impl ::core::marker::Copy for FooCloneCopy { }

struct FooCopyAndClone(i32);
#[automatically_derived]
#[doc(hidden)]
unsafe impl ::core::clone::TrivialClone for FooCopyAndClone { }
#[automatically_derived]
impl ::core::clone::Clone for FooCopyAndClone {
#[inline]
fn clone(&self) -> FooCopyAndClone {
let _: ::core::clone::AssertParamIsClone<i32>;
*self
}
}
#[automatically_derived]
impl ::core::marker::Copy for FooCopyAndClone { }

// FIXME(#124794): the previous three structs all have a trivial `Copy`-aware
// `clone`. But this one doesn't because when the `clone` is generated the
// `derive(Copy)` hasn't yet been seen.
struct FooCloneAndCopy(i32);
#[automatically_derived]
impl ::core::marker::Copy for FooCloneAndCopy { }
#[automatically_derived]
impl ::core::clone::Clone for FooCloneAndCopy {
#[inline]
fn clone(&self) -> FooCloneAndCopy {
FooCloneAndCopy(::core::clone::Clone::clone(&self.0))
}
}
Loading