From ce1143e94d37107b0c89a99bb12ad3e91b02a9f5 Mon Sep 17 00:00:00 2001 From: bstrie <865233+bstrie@users.noreply.github.com> Date: Sat, 5 Jun 2021 17:17:35 -0400 Subject: [PATCH 1/3] impl Copy/Clone for arrays in std, not in compiler --- compiler/rustc_mir_transform/src/shim.rs | 149 ------------------ .../rustc_trait_selection/src/traits/misc.rs | 3 +- .../src/traits/select/mod.rs | 8 +- compiler/rustc_ty_utils/src/instance.rs | 2 +- library/core/src/array/mod.rs | 18 +++ src/test/ui/builtin-clone-unwind.rs | 2 +- src/test/ui/chalkify/builtin-copy-clone.rs | 5 +- src/test/ui/error-codes/E0206.rs | 6 - src/test/ui/error-codes/E0206.stderr | 24 +-- 9 files changed, 30 insertions(+), 187 deletions(-) diff --git a/compiler/rustc_mir_transform/src/shim.rs b/compiler/rustc_mir_transform/src/shim.rs index f2ea5fedc625c..f59aaa664f3ad 100644 --- a/compiler/rustc_mir_transform/src/shim.rs +++ b/compiler/rustc_mir_transform/src/shim.rs @@ -310,7 +310,6 @@ fn build_clone_shim<'tcx>(tcx: TyCtxt<'tcx>, def_id: DefId, self_ty: Ty<'tcx>) - match self_ty.kind() { _ if is_copy => builder.copy_shim(), - ty::Array(ty, len) => builder.array_shim(dest, src, ty, len), ty::Closure(_, substs) => { builder.tuple_like_shim(dest, src, substs.as_closure().upvar_tys()) } @@ -459,154 +458,6 @@ impl CloneShimBuilder<'tcx> { ); } - fn loop_header( - &mut self, - beg: Place<'tcx>, - end: Place<'tcx>, - loop_body: BasicBlock, - loop_end: BasicBlock, - is_cleanup: bool, - ) { - let tcx = self.tcx; - - let cond = self.make_place(Mutability::Mut, tcx.types.bool); - let compute_cond = self.make_statement(StatementKind::Assign(Box::new(( - cond, - Rvalue::BinaryOp(BinOp::Ne, Box::new((Operand::Copy(end), Operand::Copy(beg)))), - )))); - - // `if end != beg { goto loop_body; } else { goto loop_end; }` - self.block( - vec![compute_cond], - TerminatorKind::if_(tcx, Operand::Move(cond), loop_body, loop_end), - is_cleanup, - ); - } - - fn make_usize(&self, value: u64) -> Box> { - Box::new(Constant { - span: self.span, - user_ty: None, - literal: ty::Const::from_usize(self.tcx, value).into(), - }) - } - - fn array_shim( - &mut self, - dest: Place<'tcx>, - src: Place<'tcx>, - ty: Ty<'tcx>, - len: &'tcx ty::Const<'tcx>, - ) { - let tcx = self.tcx; - let span = self.span; - - let beg = self.local_decls.push(LocalDecl::new(tcx.types.usize, span)); - let end = self.make_place(Mutability::Not, tcx.types.usize); - - // BB #0 - // `let mut beg = 0;` - // `let end = len;` - // `goto #1;` - let inits = vec![ - self.make_statement(StatementKind::Assign(Box::new(( - Place::from(beg), - Rvalue::Use(Operand::Constant(self.make_usize(0))), - )))), - self.make_statement(StatementKind::Assign(Box::new(( - end, - Rvalue::Use(Operand::Constant(Box::new(Constant { - span: self.span, - user_ty: None, - literal: len.into(), - }))), - )))), - ]; - self.block(inits, TerminatorKind::Goto { target: BasicBlock::new(1) }, false); - - // BB #1: loop { - // BB #2; - // BB #3; - // } - // BB #4; - self.loop_header(Place::from(beg), end, BasicBlock::new(2), BasicBlock::new(4), false); - - // BB #2 - // `dest[i] = Clone::clone(src[beg])`; - // Goto #3 if ok, #5 if unwinding happens. - let dest_field = self.tcx.mk_place_index(dest, beg); - let src_field = self.tcx.mk_place_index(src, beg); - self.make_clone_call(dest_field, src_field, ty, BasicBlock::new(3), BasicBlock::new(5)); - - // BB #3 - // `beg = beg + 1;` - // `goto #1`; - let statements = vec![self.make_statement(StatementKind::Assign(Box::new(( - Place::from(beg), - Rvalue::BinaryOp( - BinOp::Add, - Box::new((Operand::Copy(Place::from(beg)), Operand::Constant(self.make_usize(1)))), - ), - ))))]; - self.block(statements, TerminatorKind::Goto { target: BasicBlock::new(1) }, false); - - // BB #4 - // `return dest;` - self.block(vec![], TerminatorKind::Return, false); - - // BB #5 (cleanup) - // `let end = beg;` - // `let mut beg = 0;` - // goto #6; - let end = beg; - let beg = self.local_decls.push(LocalDecl::new(tcx.types.usize, span)); - let init = self.make_statement(StatementKind::Assign(Box::new(( - Place::from(beg), - Rvalue::Use(Operand::Constant(self.make_usize(0))), - )))); - self.block(vec![init], TerminatorKind::Goto { target: BasicBlock::new(6) }, true); - - // BB #6 (cleanup): loop { - // BB #7; - // BB #8; - // } - // BB #9; - self.loop_header( - Place::from(beg), - Place::from(end), - BasicBlock::new(7), - BasicBlock::new(9), - true, - ); - - // BB #7 (cleanup) - // `drop(dest[beg])`; - self.block( - vec![], - TerminatorKind::Drop { - place: self.tcx.mk_place_index(dest, beg), - target: BasicBlock::new(8), - unwind: None, - }, - true, - ); - - // BB #8 (cleanup) - // `beg = beg + 1;` - // `goto #6;` - let statement = self.make_statement(StatementKind::Assign(Box::new(( - Place::from(beg), - Rvalue::BinaryOp( - BinOp::Add, - Box::new((Operand::Copy(Place::from(beg)), Operand::Constant(self.make_usize(1)))), - ), - )))); - self.block(vec![statement], TerminatorKind::Goto { target: BasicBlock::new(6) }, true); - - // BB #9 (resume) - self.block(vec![], TerminatorKind::Resume, true); - } - fn tuple_like_shim(&mut self, dest: Place<'tcx>, src: Place<'tcx>, tys: I) where I: Iterator>, diff --git a/compiler/rustc_trait_selection/src/traits/misc.rs b/compiler/rustc_trait_selection/src/traits/misc.rs index cedd1aa54b876..fa8890fc35292 100644 --- a/compiler/rustc_trait_selection/src/traits/misc.rs +++ b/compiler/rustc_trait_selection/src/traits/misc.rs @@ -33,7 +33,8 @@ pub fn can_type_implement_copy( | ty::Char | ty::RawPtr(..) | ty::Never - | ty::Ref(_, _, hir::Mutability::Not) => return Ok(()), + | ty::Ref(_, _, hir::Mutability::Not) + | ty::Array(..) => return Ok(()), ty::Adt(adt, substs) => (adt, substs), diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index 481bfa4a26b36..2aa214694cb14 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -1859,7 +1859,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { | ty::Char | ty::RawPtr(..) | ty::Never - | ty::Ref(_, _, hir::Mutability::Not) => { + | ty::Ref(_, _, hir::Mutability::Not) + | ty::Array(..) => { // Implementations provided in libcore None } @@ -1872,11 +1873,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { | ty::Foreign(..) | ty::Ref(_, _, hir::Mutability::Mut) => None, - ty::Array(element_ty, _) => { - // (*) binder moved here - Where(obligation.predicate.rebind(vec![element_ty])) - } - ty::Tuple(tys) => { // (*) binder moved here Where(obligation.predicate.rebind(tys.iter().map(|k| k.expect_ty()).collect())) diff --git a/compiler/rustc_ty_utils/src/instance.rs b/compiler/rustc_ty_utils/src/instance.rs index 87b729faa54e0..13ffb2a5adc86 100644 --- a/compiler/rustc_ty_utils/src/instance.rs +++ b/compiler/rustc_ty_utils/src/instance.rs @@ -362,7 +362,7 @@ fn resolve_associated_item<'tcx>( let is_copy = self_ty.is_copy_modulo_regions(tcx.at(DUMMY_SP), param_env); match self_ty.kind() { _ if is_copy => (), - ty::Array(..) | ty::Closure(..) | ty::Tuple(..) => {} + ty::Closure(..) | ty::Tuple(..) => {} _ => return Ok(None), }; diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 811850af3678d..98cd7e8e1ae72 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -330,6 +330,24 @@ impl Ord for [T; N] { } } +#[cfg(not(bootstrap))] +#[stable(feature = "copy_clone_array_lib", since = "1.55.0")] +impl Copy for [T; N] {} + +#[cfg(not(bootstrap))] +#[stable(feature = "copy_clone_array_lib", since = "1.55.0")] +impl Clone for [T; N] { + fn clone(&self) -> Self { + // SAFETY: we know for certain that this iterator will yield exactly `N` + // items. + unsafe { collect_into_array_unchecked(&mut self.iter().cloned()) } + } + + fn clone_from(&mut self, other: &Self) { + self.clone_from_slice(other); + } +} + // The Default impls cannot be done with const generics because `[T; 0]` doesn't // require Default to be implemented, and having different impl blocks for // different numbers isn't supported yet. diff --git a/src/test/ui/builtin-clone-unwind.rs b/src/test/ui/builtin-clone-unwind.rs index 339bcfa1060a4..2caedb649a35a 100644 --- a/src/test/ui/builtin-clone-unwind.rs +++ b/src/test/ui/builtin-clone-unwind.rs @@ -53,7 +53,7 @@ fn main() { ].clone(); }); - assert!(result.is_err()); + assert!(child.is_err()); assert_eq!( 1, Rc::strong_count(&counter) diff --git a/src/test/ui/chalkify/builtin-copy-clone.rs b/src/test/ui/chalkify/builtin-copy-clone.rs index d403514b553b0..4323e87b08de7 100644 --- a/src/test/ui/chalkify/builtin-copy-clone.rs +++ b/src/test/ui/chalkify/builtin-copy-clone.rs @@ -23,11 +23,12 @@ fn test_copy_clone(arg: T) { fn foo() { } fn main() { + // FIXME: add closures when they're considered WF test_copy_clone(foo); let f: fn() = foo; test_copy_clone(f); - // FIXME: add closures when they're considered WF - test_copy_clone([1; 56]); + // FIXME(#86252): reinstate array test after chalk upgrade + //test_copy_clone([1; 56]); test_copy_clone((1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1)); test_copy_clone((1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, true, 'a', 1.1)); test_copy_clone(()); diff --git a/src/test/ui/error-codes/E0206.rs b/src/test/ui/error-codes/E0206.rs index bace046758112..0f3d427ce11ac 100644 --- a/src/test/ui/error-codes/E0206.rs +++ b/src/test/ui/error-codes/E0206.rs @@ -1,9 +1,3 @@ -type Foo = [u8; 256]; - -impl Copy for Foo { } -//~^ ERROR the trait `Copy` may not be implemented for this type -//~| ERROR only traits defined in the current crate can be implemented for arbitrary types - #[derive(Copy, Clone)] struct Bar; diff --git a/src/test/ui/error-codes/E0206.stderr b/src/test/ui/error-codes/E0206.stderr index e4ad4ffb45fee..57ae2647d339c 100644 --- a/src/test/ui/error-codes/E0206.stderr +++ b/src/test/ui/error-codes/E0206.stderr @@ -1,27 +1,9 @@ error[E0206]: the trait `Copy` may not be implemented for this type - --> $DIR/E0206.rs:3:15 - | -LL | impl Copy for Foo { } - | ^^^ type is not a structure or enumeration - -error[E0206]: the trait `Copy` may not be implemented for this type - --> $DIR/E0206.rs:10:15 + --> $DIR/E0206.rs:4:15 | LL | impl Copy for &'static mut Bar { } | ^^^^^^^^^^^^^^^^ type is not a structure or enumeration -error[E0117]: only traits defined in the current crate can be implemented for arbitrary types - --> $DIR/E0206.rs:3:1 - | -LL | impl Copy for Foo { } - | ^^^^^^^^^^^^^^--- - | | | - | | this is not defined in the current crate because arrays are always foreign - | impl doesn't use only types from inside the current crate - | - = note: define and implement a trait or new type instead - -error: aborting due to 3 previous errors +error: aborting due to previous error -Some errors have detailed explanations: E0117, E0206. -For more information about an error, try `rustc --explain E0117`. +For more information about this error, try `rustc --explain E0206`. From 3024efff59368d6271dab1a8ac78434175647e9b Mon Sep 17 00:00:00 2001 From: bstrie <865233+bstrie@users.noreply.github.com> Date: Sat, 5 Jun 2021 17:20:51 -0400 Subject: [PATCH 2/3] Update Copy/Clone documentation WRT arrays --- compiler/rustc_error_codes/src/error_codes/E0206.md | 9 +++------ library/core/src/clone.rs | 1 - library/core/src/marker.rs | 1 - 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_error_codes/src/error_codes/E0206.md b/compiler/rustc_error_codes/src/error_codes/E0206.md index da53b671ad0cc..4405a2149ceb8 100644 --- a/compiler/rustc_error_codes/src/error_codes/E0206.md +++ b/compiler/rustc_error_codes/src/error_codes/E0206.md @@ -4,15 +4,12 @@ enum. Erroneous code example: ```compile_fail,E0206 -type Foo = [u8; 256]; -impl Copy for Foo { } // error! - #[derive(Copy, Clone)] struct Bar; impl Copy for &'static mut Bar { } // error! ``` -You can only implement `Copy` for a struct or an enum. Both of the previous -examples will fail, because neither `[u8; 256]` nor `&'static mut Bar` -(mutable reference to `Bar`) is a struct or enum. +You can only implement `Copy` for a struct or an enum. +The previous example will fail because `&'static mut Bar` +is not a struct or enum. diff --git a/library/core/src/clone.rs b/library/core/src/clone.rs index 281ff3badfbd8..6f9579043c37d 100644 --- a/library/core/src/clone.rs +++ b/library/core/src/clone.rs @@ -93,7 +93,6 @@ /// /// * Function item types (i.e., the distinct types defined for each function) /// * Function pointer types (e.g., `fn() -> i32`) -/// * Array types, for all sizes, if the item type also implements `Clone` (e.g., `[i32; 123456]`) /// * Tuple types, if each component also implements `Clone` (e.g., `()`, `(i32, bool)`) /// * Closure types, if they capture no value from the environment /// or if all such captured values implement `Clone` themselves. diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 37446bafacb24..71eea43aa54e1 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -359,7 +359,6 @@ pub trait StructuralEq { /// /// * Function item types (i.e., the distinct types defined for each function) /// * Function pointer types (e.g., `fn() -> i32`) -/// * Array types, for all sizes, if the item type also implements `Copy` (e.g., `[i32; 123456]`) /// * Tuple types, if each component also implements `Copy` (e.g., `()`, `(i32, bool)`) /// * Closure types, if they capture no value from the environment /// or if all such captured values implement `Copy` themselves. From 61b1394ac7d16bcb73684952c77f8239effe9dbe Mon Sep 17 00:00:00 2001 From: bstrie <865233+bstrie@users.noreply.github.com> Date: Mon, 8 Nov 2021 15:51:56 -0500 Subject: [PATCH 3/3] Attempt to address perf regressions with #[inline] --- library/core/src/array/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/library/core/src/array/mod.rs b/library/core/src/array/mod.rs index 98cd7e8e1ae72..089f1f3639062 100644 --- a/library/core/src/array/mod.rs +++ b/library/core/src/array/mod.rs @@ -331,18 +331,20 @@ impl Ord for [T; N] { } #[cfg(not(bootstrap))] -#[stable(feature = "copy_clone_array_lib", since = "1.55.0")] +#[stable(feature = "copy_clone_array_lib", since = "1.58.0")] impl Copy for [T; N] {} #[cfg(not(bootstrap))] -#[stable(feature = "copy_clone_array_lib", since = "1.55.0")] +#[stable(feature = "copy_clone_array_lib", since = "1.58.0")] impl Clone for [T; N] { + #[inline] fn clone(&self) -> Self { // SAFETY: we know for certain that this iterator will yield exactly `N` // items. unsafe { collect_into_array_unchecked(&mut self.iter().cloned()) } } + #[inline] fn clone_from(&mut self, other: &Self) { self.clone_from_slice(other); }