From 5858317befd4ed32d0dd96b99d8b0e0f23bf7f5d Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 29 Jan 2018 12:52:34 +0530 Subject: [PATCH 1/7] Remove unused code --- src/librustc_errors/snippet.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/src/librustc_errors/snippet.rs b/src/librustc_errors/snippet.rs index c2f4701999ea9..7d416f13ffc8a 100644 --- a/src/librustc_errors/snippet.rs +++ b/src/librustc_errors/snippet.rs @@ -10,31 +10,8 @@ // Code for annotating snippets. -use syntax_pos::{Span, FileMap}; -use CodeMapper; -use std::rc::Rc; use Level; -#[derive(Clone)] -pub struct SnippetData { - codemap: Rc, - files: Vec, -} - -#[derive(Clone)] -pub struct FileInfo { - file: Rc, - - /// The "primary file", if any, gets a `-->` marker instead of - /// `>>>`, and has a line-number/column printed and not just a - /// filename. It appears first in the listing. It is known to - /// contain at least one primary span, though primary spans (which - /// are designated with `^^^`) may also occur in other files. - primary_span: Option, - - lines: Vec, -} - #[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] pub struct Line { pub line_index: usize, From 0ab52f0d8eea447d9419d633227964cd6aba3689 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 29 Jan 2018 12:53:11 +0530 Subject: [PATCH 2/7] Stop generating Clone impls when there isn't a derive(Copy) --- src/libsyntax/ext/derive.rs | 3 +++ src/libsyntax/feature_gate.rs | 5 ++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/libsyntax/ext/derive.rs b/src/libsyntax/ext/derive.rs index c7fa0331c1bd5..ab5dff07d47f0 100644 --- a/src/libsyntax/ext/derive.rs +++ b/src/libsyntax/ext/derive.rs @@ -77,6 +77,9 @@ pub fn add_derived_markers(cx: &mut ExtCtxt, span: Span, traits: &[ast::Path] if names.contains(&Symbol::intern("Copy")) { let meta = cx.meta_word(span, Symbol::intern("rustc_copy_clone_marker")); attrs.push(cx.attribute(span, meta)); + } else if names.contains(&Symbol::intern("Clone")) { + let meta = cx.meta_word(span, Symbol::intern("rustc_nocopy_clone_marker")); + attrs.push(cx.attribute(span, meta)); } attrs }) diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 8512e215ca765..b6287545ff235 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -867,7 +867,10 @@ pub const BUILTIN_ATTRIBUTES: &'static [(&'static str, AttributeType, AttributeG "rustc_attrs", "internal implementation detail", cfg_fn!(rustc_attrs))), - + ("rustc_nocopy_clone_marker", Whitelisted, Gated(Stability::Unstable, + "rustc_attrs", + "internal implementation detail", + cfg_fn!(rustc_attrs))), // FIXME: #14408 whitelist docs since rustdoc looks at them ("doc", Whitelisted, Ungated), From fd45d4883b861346efd2453337ad043adc7048a3 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 29 Jan 2018 12:53:39 +0530 Subject: [PATCH 3/7] Ask compiler to use shim for rustc_nocopy_clone_marker types --- src/librustc/traits/select.rs | 15 ++++++++++++++- src/libsyntax_ext/deriving/clone.rs | 10 ++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 55cbc890e1e91..5c4ed4d1cd80d 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2126,7 +2126,20 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { } } - ty::TyAdt(..) | ty::TyProjection(..) | ty::TyParam(..) | ty::TyAnon(..) => { + ty::TyAdt(adt, substs) => { + let attrs = self.tcx().get_attrs(adt.did); + if adt.is_enum() && attrs.iter().any(|a| a.check_name("rustc_nocopy_clone_marker")) { + // for Clone + let mut iter = substs.types() + .chain(adt.all_fields().map(|f| f.ty(self.tcx(), substs))); + Where(ty::Binder(iter.collect())) + } else { + // Fallback to whatever user-defined impls exist in this case. + None + } + } + + ty::TyProjection(..) | ty::TyParam(..) | ty::TyAnon(..) => { // Fallback to whatever user-defined impls exist in this case. None } diff --git a/src/libsyntax_ext/deriving/clone.rs b/src/libsyntax_ext/deriving/clone.rs index f23d22b0c365f..445b345e38041 100644 --- a/src/libsyntax_ext/deriving/clone.rs +++ b/src/libsyntax_ext/deriving/clone.rs @@ -55,6 +55,16 @@ pub fn expand_deriving_clone(cx: &mut ExtCtxt, substructure = combine_substructure(Box::new(|c, s, sub| { cs_clone_shallow("Clone", c, s, sub, false) })); + } else if attr::contains_name(&annitem.attrs, "rustc_nocopy_clone_marker") { + if let ItemKind::Enum(..) = annitem.node { + // Do nothing, this will be handled in a shim + return + } + bounds = vec![]; + is_shallow = false; + substructure = combine_substructure(Box::new(|c, s, sub| { + cs_clone("Clone", c, s, sub) + })); } else { bounds = vec![]; is_shallow = false; From 4aecbdb85715689dd71f076cc837a43a77ebdcc4 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 29 Jan 2018 14:11:32 +0530 Subject: [PATCH 4/7] Document the index used in AggregateKind::Adt --- src/librustc/mir/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 3aa94b3469942..98725c44faf4f 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -1520,6 +1520,9 @@ pub enum AggregateKind<'tcx> { /// active field number and is present only for union expressions /// -- e.g. for a union expression `SomeUnion { c: .. }`, the /// active field index would identity the field `c` + /// + /// For enums, the second field is the index of the variant + /// within AdtDef::fields Adt(&'tcx AdtDef, usize, &'tcx Substs<'tcx>, Option), Closure(DefId, ClosureSubsts<'tcx>), From 6a1d146fe079f84ff1de07748617eb7510a78252 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 29 Jan 2018 14:47:12 +0530 Subject: [PATCH 5/7] Add shim for simple cloning --- src/librustc_mir/shim.rs | 82 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index c206d0ea9b5fd..11ab3d56c96c5 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -310,6 +310,7 @@ fn build_clone_shim<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, ) } ty::TyTuple(tys, _) => builder.tuple_like_shim(&**tys, AggregateKind::Tuple), + ty::TyAdt(adt, substs) => builder.enum_shim(adt, substs), _ => { bug!("clone shim for `{:?}` which is not `Copy` and is not an aggregate", self_ty) } @@ -626,6 +627,87 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> { self.block(vec![], TerminatorKind::Resume, true); } + fn enum_shim(&mut self, adt: &'tcx ty::AdtDef, substs: &'tcx Substs<'tcx>) { + if !adt.is_enum() { + bug!("We only make Clone shims for enum ADTs"); + } + let receiver = Place::Local(Local::new(1+0)).deref(); + // should be u128 maybe? + let discr_ty = self.tcx.types.usize; + let discr = self.make_place(Mutability::Not, discr_ty); + let assign_discr = self.make_statement( + StatementKind::Assign( + discr.clone(), + Rvalue::Discriminant(receiver.clone()) + ) + ); + let switch = self.make_enum_match(adt, substs, discr, receiver); + self.block(vec![assign_discr], switch, false); + } + + fn make_enum_match(&mut self, adt: &'tcx ty::AdtDef, + substs: &'tcx Substs<'tcx>, + discr: Place<'tcx>, + receiver: Place<'tcx>) -> TerminatorKind<'tcx> { + + let mut values = vec![]; + let mut targets = vec![]; + let mut blocks = 0; + for (idx, variant) in adt.variants.iter().enumerate() { + values.push(adt.discriminant_for_variant(self.tcx, idx)); + + let variant_place = receiver.clone().downcast(adt, idx); + let mut returns = vec![]; + // FIXME(Manishearth) this code has a lot in common + // with tuple_like_shim + for (i, field) in variant.fields.iter().enumerate() { + let field_ty = field.ty(self.tcx, substs); + let receiver_field = variant_place.clone().field(Field::new(i), field_ty); + + // BB (blocks + 2i) + returns.push( + self.make_clone_call( + &field_ty, + receiver_field, + BasicBlock::new(blocks + 2 * i + 2), + BasicBlock::new(blocks + 2 * i + 1), + ) + ); + // BB #(2i + 1) (cleanup) + if i == 0 { + // Nothing to drop, just resume. + self.block(vec![], TerminatorKind::Resume, true); + } else { + // Drop previous field and goto previous cleanup block. + self.block(vec![], TerminatorKind::Drop { + location: returns[i - 1].clone(), + target: BasicBlock::new(blocks + 2 * i - 1), + unwind: None, + }, true); + } + } + let ret_statement = self.make_statement( + StatementKind::Assign( + Place::Local(RETURN_PLACE), + Rvalue::Aggregate( + box AggregateKind::Adt(adt, idx, substs, None), + returns.into_iter().map(Operand::Move).collect() + ) + ) + ); + targets.push(self.block(vec![ret_statement], TerminatorKind::Return, false)); + blocks += variant.fields.len() * 2 + 1; + } + // the nonexistant extra case + targets.push(self.block(vec![], TerminatorKind::Abort, true)); + TerminatorKind::SwitchInt { + discr: Operand::Move(discr), + switch_ty: self.tcx.types.usize, + values: From::from(values), + targets, + } + } + fn tuple_like_shim(&mut self, tys: &[ty::Ty<'tcx>], kind: AggregateKind<'tcx>) { match kind { AggregateKind::Tuple | AggregateKind::Closure(..) => (), From 7725ee5c63e8ab5692d3c418c52b6bbc295ec617 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 29 Jan 2018 17:41:40 +0530 Subject: [PATCH 6/7] Only trigger shims for Clone --- src/librustc/traits/select.rs | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 5c4ed4d1cd80d..605369bd327c3 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2129,10 +2129,17 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { ty::TyAdt(adt, substs) => { let attrs = self.tcx().get_attrs(adt.did); if adt.is_enum() && attrs.iter().any(|a| a.check_name("rustc_nocopy_clone_marker")) { - // for Clone - let mut iter = substs.types() - .chain(adt.all_fields().map(|f| f.ty(self.tcx(), substs))); - Where(ty::Binder(iter.collect())) + let trait_id = obligation.predicate.def_id(); + if Some(trait_id) == self.tcx().lang_items().clone_trait() { + // for Clone + // this doesn't work for recursive types (FIXME(Manishearth)) + // let mut iter = substs.types() + // .chain(adt.all_fields().map(|f| f.ty(self.tcx(), substs))); + let mut iter = substs.types(); + Where(ty::Binder(iter.collect())) + } else { + None + } } else { // Fallback to whatever user-defined impls exist in this case. None From 1f5d5457a75fd42a9ae9ea2b3a4d0f59eb7e2df0 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Mon, 29 Jan 2018 19:29:15 +0530 Subject: [PATCH 7/7] Set correct entry block --- src/librustc_mir/shim.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/shim.rs b/src/librustc_mir/shim.rs index 11ab3d56c96c5..4131fc4a29de0 100644 --- a/src/librustc_mir/shim.rs +++ b/src/librustc_mir/shim.rs @@ -635,14 +635,20 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> { // should be u128 maybe? let discr_ty = self.tcx.types.usize; let discr = self.make_place(Mutability::Not, discr_ty); + let assign_discr = self.make_statement( StatementKind::Assign( discr.clone(), Rvalue::Discriminant(receiver.clone()) ) ); + // insert dummy first block + let entry_block = self.block(vec![], TerminatorKind::Abort, false); let switch = self.make_enum_match(adt, substs, discr, receiver); - self.block(vec![assign_discr], switch, false); + + let source_info = self.source_info(); + self.blocks[entry_block].statements = vec![assign_discr]; + self.blocks[entry_block].terminator = Some(Terminator { source_info, kind: switch }); } fn make_enum_match(&mut self, adt: &'tcx ty::AdtDef, @@ -652,7 +658,7 @@ impl<'a, 'tcx> CloneShimBuilder<'a, 'tcx> { let mut values = vec![]; let mut targets = vec![]; - let mut blocks = 0; + let mut blocks = 1; for (idx, variant) in adt.variants.iter().enumerate() { values.push(adt.discriminant_for_variant(self.tcx, idx));