Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Make derive(Clone) memcpy on enum variants containing Copy types #47848

Closed
wants to merge 7 commits into from
Closed
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
3 changes: 3 additions & 0 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<usize>),
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if we should start moving to struct-like variants for anything that has more than 3 fields throughout the compiler...


Closure(DefId, ClosureSubsts<'tcx>),
Expand Down
22 changes: 21 additions & 1 deletion src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2126,7 +2126,27 @@ 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")) {
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
}
}

ty::TyProjection(..) | ty::TyParam(..) | ty::TyAnon(..) => {
// Fallback to whatever user-defined impls exist in this case.
None
}
Expand Down
23 changes: 0 additions & 23 deletions src/librustc_errors/snippet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<CodeMapper>,
files: Vec<FileInfo>,
}

#[derive(Clone)]
pub struct FileInfo {
file: Rc<FileMap>,

/// 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<Span>,

lines: Vec<Line>,
}

#[derive(Clone, Debug, PartialOrd, Ord, PartialEq, Eq)]
pub struct Line {
pub line_index: usize,
Expand Down
88 changes: 88 additions & 0 deletions src/librustc_mir/shim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -626,6 +627,93 @@ 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())
)
);
// insert dummy first block
let entry_block = self.block(vec![], TerminatorKind::Abort, false);
let switch = self.make_enum_match(adt, substs, discr, receiver);

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,
substs: &'tcx Substs<'tcx>,
discr: Place<'tcx>,
receiver: Place<'tcx>) -> TerminatorKind<'tcx> {

let mut values = vec![];
let mut targets = vec![];
let mut blocks = 1;
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(..) => (),
Expand Down
3 changes: 3 additions & 0 deletions src/libsyntax/ext/derive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ pub fn add_derived_markers<T>(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
})
Expand Down
5 changes: 4 additions & 1 deletion src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
10 changes: 10 additions & 0 deletions src/libsyntax_ext/deriving/clone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down