Skip to content

don't clone types that are copy, round two. #68459

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

Merged
merged 2 commits into from
Jan 28, 2020
Merged
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
2 changes: 1 addition & 1 deletion src/librustc/infer/fudge.rs
Original file line number Diff line number Diff line change
@@ -19,7 +19,7 @@ fn const_vars_since_snapshot<'tcx>(
(
range.start..range.end,
(range.start.index..range.end.index)
.map(|index| table.probe_value(ConstVid::from_index(index)).origin.clone())
.map(|index| table.probe_value(ConstVid::from_index(index)).origin)
.collect(),
)
}
4 changes: 2 additions & 2 deletions src/librustc/infer/lexical_region_resolve/mod.rs
Original file line number Diff line number Diff line change
@@ -611,7 +611,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {

errors.push(RegionResolutionError::GenericBoundFailure(
verify.origin.clone(),
verify.kind.clone(),
verify.kind,
sub,
));
}
@@ -761,7 +761,7 @@ impl<'cx, 'tcx> LexicalResolver<'cx, 'tcx> {

for upper_bound in &upper_bounds {
if !self.region_rels.is_subregion_of(effective_lower_bound, upper_bound.region) {
let origin = self.var_infos[node_idx].origin.clone();
let origin = self.var_infos[node_idx].origin;
debug!(
"region inference error at {:?} for {:?}: SubSupConflict sub: {:?} \
sup: {:?}",
2 changes: 1 addition & 1 deletion src/librustc/infer/region_constraints/mod.rs
Original file line number Diff line number Diff line change
@@ -832,7 +832,7 @@ impl<'tcx> RegionConstraintCollector<'tcx> {
(
range.clone(),
(range.start.index()..range.end.index())
.map(|index| self.var_infos[ty::RegionVid::from(index)].origin.clone())
.map(|index| self.var_infos[ty::RegionVid::from(index)].origin)
.collect(),
)
}
2 changes: 1 addition & 1 deletion src/librustc/infer/type_variable.rs
Original file line number Diff line number Diff line change
@@ -306,7 +306,7 @@ impl<'tcx> TypeVariableTable<'tcx> {
(
range.start.vid..range.end.vid,
(range.start.vid.index..range.end.vid.index)
.map(|index| self.values.get(index as usize).origin.clone())
.map(|index| self.values.get(index as usize).origin)
.collect(),
)
}
24 changes: 16 additions & 8 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
@@ -1988,7 +1988,7 @@ impl<'tcx> Operand<'tcx> {
pub fn to_copy(&self) -> Self {
match *self {
Operand::Copy(_) | Operand::Constant(_) => self.clone(),
Operand::Move(ref place) => Operand::Copy(place.clone()),
Operand::Move(place) => Operand::Copy(place),
}
}
}
@@ -2462,11 +2462,15 @@ impl<'tcx> TypeFoldable<'tcx> for UserTypeProjection {
let projs: Vec<_> = self
.projs
.iter()
.map(|elem| match elem {
.map(|&elem| match elem {
Deref => Deref,
Field(f, ()) => Field(f.clone(), ()),
Field(f, ()) => Field(f, ()),
Index(()) => Index(()),
elem => elem.clone(),
Downcast(symbol, variantidx) => Downcast(symbol, variantidx),
ConstantIndex { offset, min_length, from_end } => {
ConstantIndex { offset, min_length, from_end }
}
Subslice { from, to, from_end } => Subslice { from, to, from_end },
})
.collect();

@@ -2862,11 +2866,15 @@ impl<'tcx> TypeFoldable<'tcx> for PlaceElem<'tcx> {
fn super_fold_with<F: TypeFolder<'tcx>>(&self, folder: &mut F) -> Self {
use crate::mir::ProjectionElem::*;

match self {
match *self {
Deref => Deref,
Field(f, ty) => Field(*f, ty.fold_with(folder)),
Field(f, ty) => Field(f, ty.fold_with(folder)),
Index(v) => Index(v.fold_with(folder)),
elem => elem.clone(),
Downcast(symbol, variantidx) => Downcast(symbol, variantidx),
ConstantIndex { offset, min_length, from_end } => {
ConstantIndex { offset, min_length, from_end }
}
Subslice { from, to, from_end } => Subslice { from, to, from_end },
}
}

@@ -2911,7 +2919,7 @@ impl<'tcx, R: Idx, C: Idx> TypeFoldable<'tcx> for BitMatrix<R, C> {
impl<'tcx> TypeFoldable<'tcx> for Constant<'tcx> {
fn super_fold_with<F: TypeFolder<'tcx>>(&self, folder: &mut F) -> Self {
Constant {
span: self.span.clone(),
span: self.span,
user_ty: self.user_ty.fold_with(folder),
literal: self.literal.fold_with(folder),
}
2 changes: 1 addition & 1 deletion src/librustc/mir/mono.rs
Original file line number Diff line number Diff line change
@@ -362,7 +362,7 @@ impl<'tcx> CodegenUnit<'tcx> {
}

pub fn codegen_dep_node(&self, tcx: TyCtxt<'tcx>) -> DepNode {
DepNode::new(tcx, DepConstructor::CompileCodegenUnit(self.name().clone()))
DepNode::new(tcx, DepConstructor::CompileCodegenUnit(self.name()))
}
}

2 changes: 1 addition & 1 deletion src/librustc/traits/auto_trait.rs
Original file line number Diff line number Diff line change
@@ -535,7 +535,7 @@ impl AutoTraitFinder<'tcx> {
}

while !vid_map.is_empty() {
let target = vid_map.keys().next().expect("Keys somehow empty").clone();
let target = *vid_map.keys().next().expect("Keys somehow empty");
let deps = vid_map.remove(&target).expect("Entry somehow missing");

for smaller in deps.smaller.iter() {
9 changes: 3 additions & 6 deletions src/librustc/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
@@ -54,10 +54,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
span,
predicates
.iter()
.map(|predicate| ErrorDescriptor {
predicate: predicate.clone(),
index: None,
})
.map(|&predicate| ErrorDescriptor { predicate, index: None })
.collect(),
)
})
@@ -73,7 +70,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}

error_map.entry(span).or_default().push(ErrorDescriptor {
predicate: error.obligation.predicate.clone(),
predicate: error.obligation.predicate,
index: Some(index),
});

@@ -137,7 +134,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
};

for implication in super::elaborate_predicates(self.tcx, vec![cond.clone()]) {
for implication in super::elaborate_predicates(self.tcx, vec![*cond]) {
if let ty::Predicate::Trait(implication, _) = implication {
let error = error.to_poly_trait_ref();
let implication = implication.to_poly_trait_ref();
4 changes: 2 additions & 2 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
@@ -377,7 +377,7 @@ impl<'a, 'b, 'tcx> TypeFolder<'tcx> for AssocTypeNormalizer<'a, 'b, 'tcx> {
let normalized_ty = normalize_projection_type(
self.selcx,
self.param_env,
data.clone(),
*data,
self.cause.clone(),
self.depth,
&mut self.obligations,
@@ -433,7 +433,7 @@ pub fn normalize_projection_type<'a, 'b, 'tcx>(
opt_normalize_projection_type(
selcx,
param_env,
projection_ty.clone(),
projection_ty,
cause.clone(),
depth,
obligations,
11 changes: 4 additions & 7 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
@@ -2068,7 +2068,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}
}

_ => candidates.vec.push(AutoImplCandidate(def_id.clone())),
_ => candidates.vec.push(AutoImplCandidate(def_id)),
}
}

@@ -2132,10 +2132,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
// but `Foo` is declared as `trait Foo: Bar<u32>`.
let upcast_trait_refs = util::supertraits(self.tcx(), poly_trait_ref)
.filter(|upcast_trait_ref| {
self.infcx.probe(|_| {
let upcast_trait_ref = upcast_trait_ref.clone();
self.match_poly_trait_ref(obligation, upcast_trait_ref).is_ok()
})
self.infcx
.probe(|_| self.match_poly_trait_ref(obligation, *upcast_trait_ref).is_ok())
})
.count();

@@ -2243,7 +2241,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
let def_id = obligation.predicate.def_id();

if self.tcx().is_trait_alias(def_id) {
candidates.vec.push(TraitAliasCandidate(def_id.clone()));
candidates.vec.push(TraitAliasCandidate(def_id));
}

Ok(())
@@ -3249,7 +3247,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
obligation_trait_ref: ty::PolyTraitRef<'tcx>,
expected_trait_ref: ty::PolyTraitRef<'tcx>,
) -> Result<Vec<PredicateObligation<'tcx>>, SelectionError<'tcx>> {
let obligation_trait_ref = obligation_trait_ref.clone();
self.infcx
.at(&obligation_cause, obligation_param_env)
.sup(obligation_trait_ref, expected_trait_ref)
4 changes: 2 additions & 2 deletions src/librustc/traits/util.rs
Original file line number Diff line number Diff line change
@@ -526,11 +526,11 @@ pub fn predicates_for_generics<'tcx>(
generic_bounds
.predicates
.iter()
.map(|predicate| Obligation {
.map(|&predicate| Obligation {
cause: cause.clone(),
recursion_depth,
param_env,
predicate: predicate.clone(),
predicate,
})
.collect()
}
3 changes: 1 addition & 2 deletions src/librustc/traits/wf.rs
Original file line number Diff line number Diff line change
@@ -318,8 +318,7 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
if let Elaborate::All = elaborate {
let trait_assoc_items = tcx.associated_items(trait_ref.def_id);

let predicates =
obligations.iter().map(|obligation| obligation.predicate.clone()).collect();
let predicates = obligations.iter().map(|obligation| obligation.predicate).collect();
let implied_obligations = traits::elaborate_predicates(tcx, predicates);
let implied_obligations = implied_obligations.map(|pred| {
let mut cause = cause.clone();
2 changes: 1 addition & 1 deletion src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
@@ -1294,7 +1294,7 @@ impl<'tcx> TyCtxt<'tcx> {
// statements within the query system and we'd run into endless
// recursion otherwise.
let (crate_name, crate_disambiguator) = if def_id.is_local() {
(self.crate_name.clone(), self.sess.local_crate_disambiguator())
(self.crate_name, self.sess.local_crate_disambiguator())
} else {
(
self.cstore.crate_name_untracked(def_id.krate),
8 changes: 4 additions & 4 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
@@ -1326,7 +1326,7 @@ pub trait ToPolyTraitRef<'tcx> {

impl<'tcx> ToPolyTraitRef<'tcx> for TraitRef<'tcx> {
fn to_poly_trait_ref(&self) -> PolyTraitRef<'tcx> {
ty::Binder::dummy(self.clone())
ty::Binder::dummy(*self)
}
}

@@ -1372,19 +1372,19 @@ impl<'tcx> ToPredicate<'tcx> for ConstnessAnd<&PolyTraitRef<'tcx>> {

impl<'tcx> ToPredicate<'tcx> for PolyRegionOutlivesPredicate<'tcx> {
fn to_predicate(&self) -> Predicate<'tcx> {
Predicate::RegionOutlives(self.clone())
Predicate::RegionOutlives(*self)
}
}

impl<'tcx> ToPredicate<'tcx> for PolyTypeOutlivesPredicate<'tcx> {
fn to_predicate(&self) -> Predicate<'tcx> {
Predicate::TypeOutlives(self.clone())
Predicate::TypeOutlives(*self)
}
}

impl<'tcx> ToPredicate<'tcx> for PolyProjectionPredicate<'tcx> {
fn to_predicate(&self) -> Predicate<'tcx> {
Predicate::Projection(self.clone())
Predicate::Projection(*self)
}
}

2 changes: 1 addition & 1 deletion src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
@@ -838,7 +838,7 @@ impl<'tcx> PolyTraitRef<'tcx> {

pub fn to_poly_trait_predicate(&self) -> ty::PolyTraitPredicate<'tcx> {
// Note that we preserve binding levels
Binder(ty::TraitPredicate { trait_ref: self.skip_binder().clone() })
Binder(ty::TraitPredicate { trait_ref: *self.skip_binder() })
}
}

2 changes: 1 addition & 1 deletion src/librustc_builtin_macros/format.rs
Original file line number Diff line number Diff line change
@@ -708,7 +708,7 @@ impl<'a, 'b> Context<'a, 'b> {
// Before consuming the expressions, we have to remember spans for
// count arguments as they are now generated separate from other
// arguments, hence have no access to the `P<ast::Expr>`'s.
let spans_pos: Vec<_> = self.args.iter().map(|e| e.span.clone()).collect();
let spans_pos: Vec<_> = self.args.iter().map(|e| e.span).collect();

// Right now there is a bug such that for the expression:
// foo(bar(&1))
12 changes: 6 additions & 6 deletions src/librustc_errors/emitter.rs
Original file line number Diff line number Diff line change
@@ -400,7 +400,7 @@ pub trait Emitter {
}
if sm.span_to_filename(sp_label.span.clone()).is_macros() && !always_backtrace {
if let Some(use_site) = sp_label.span.macro_backtrace().last() {
before_after.push((sp_label.span.clone(), use_site.call_site.clone()));
before_after.push((sp_label.span, use_site.call_site));
}
}
}
@@ -1184,13 +1184,13 @@ impl EmitterWriter {
let level_str = level.to_string();
// The failure note level itself does not provide any useful diagnostic information
if *level != Level::FailureNote && !level_str.is_empty() {
buffer.append(0, &level_str, Style::Level(level.clone()));
buffer.append(0, &level_str, Style::Level(*level));
Copy link
Member

Choose a reason for hiding this comment

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

You might want to leave it for another PR but level should probably be Level, not &Level.
@petrochenkov and/or @estebank might know more about what's happening in this code though.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think at some point Level wasn't Copy, this is probably just a leftover from that.

}
// only render error codes, not lint codes
if let Some(DiagnosticId::Error(ref code)) = *code {
buffer.append(0, "[", Style::Level(level.clone()));
buffer.append(0, &code, Style::Level(level.clone()));
buffer.append(0, "]", Style::Level(level.clone()));
buffer.append(0, "[", Style::Level(*level));
buffer.append(0, &code, Style::Level(*level));
buffer.append(0, "]", Style::Level(*level));
}
if *level != Level::FailureNote && !level_str.is_empty() {
buffer.append(0, ": ", header_style);
@@ -1495,7 +1495,7 @@ impl EmitterWriter {
// Render the suggestion message
let level_str = level.to_string();
if !level_str.is_empty() {
buffer.append(0, &level_str, Style::Level(level.clone()));
buffer.append(0, &level_str, Style::Level(*level));
buffer.append(0, ": ", Style::HeaderMsg);
}
self.msg_to_buffer(
4 changes: 2 additions & 2 deletions src/librustc_mir/borrow_check/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
@@ -691,7 +691,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let root_place_projection = self.infcx.tcx.intern_place_elems(root_place.projection);

if self.access_place_error_reported.contains(&(
Place { local: root_place.local.clone(), projection: root_place_projection },
Place { local: *root_place.local, projection: root_place_projection },
Copy link
Member

Choose a reason for hiding this comment

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

cc @matthewjasper Why is there a reference to a Local here - is it a mutable reference perhaps?

borrow_span,
)) {
debug!(
@@ -702,7 +702,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}

self.access_place_error_reported.insert((
Place { local: root_place.local.clone(), projection: root_place_projection },
Place { local: *root_place.local, projection: root_place_projection },
borrow_span,
));

2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
@@ -178,7 +178,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
};
grouped_errors.push(GroupedMoveError::MovesFromPlace {
span,
move_from: match_place.clone(),
move_from: *match_place,
original_path,
kind,
binds_to,
10 changes: 4 additions & 6 deletions src/librustc_mir/borrow_check/mod.rs
Original file line number Diff line number Diff line change
@@ -883,7 +883,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// Check is_empty() first because it's the common case, and doing that
// way we avoid the clone() call.
if !self.access_place_error_reported.is_empty()
&& self.access_place_error_reported.contains(&(place_span.0.clone(), place_span.1))
&& self.access_place_error_reported.contains(&(*place_span.0, place_span.1))
Copy link
Member

Choose a reason for hiding this comment

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

@oli-obk @spastorino Maybe we need to consider passing Place by value more often?
(it's a pair of an integer and a pointer, so it should be really compact)

Copy link
Contributor

Choose a reason for hiding this comment

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

That doesn't work if there are subslices of the projection involved. We do have a PlaceRef type for exactly that use case though

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, except in the cases @oli-obk mentions I'd change most of the uses to take Place by value. I need to do a bunch of cleanups still :).

{
debug!(
"access_place: suppressing error place_span=`{:?}` kind=`{:?}`",
@@ -911,7 +911,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if conflict_error || mutability_error {
debug!("access_place: logging error place_span=`{:?}` kind=`{:?}`", place_span, kind);

self.access_place_error_reported.insert((place_span.0.clone(), place_span.1));
self.access_place_error_reported.insert((*place_span.0, place_span.1));
}
}

@@ -1011,10 +1011,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// the 2018 edition so we emit it as a warning. We buffer
// these sepately so that we only emit a warning if borrow
// checking was otherwise successful.
this.reservation_warnings.insert(
bi,
(place_span.0.clone(), place_span.1, location, bk, borrow.clone()),
);
this.reservation_warnings
.insert(bi, (*place_span.0, place_span.1, location, bk, borrow.clone()));

// Don't suppress actual errors.
Control::Continue
Loading