Skip to content

Commit

Permalink
Auto merge of #72494 - lcnr:predicate-cleanup, r=nikomatsakis
Browse files Browse the repository at this point in the history
Pass more `Copy` types by value.

There are a lot of locations where we pass `&T where T: Copy` by reference,
which should both be slightly less performant and less readable IMO.

This PR currently consists of three fairly self contained commits:

- passes `ty::Predicate` by value and stops depending on `AsRef<ty::Predicate>`.
- changes `<&List<_>>::into_iter` to iterate over the elements by value. This would break `List`s
  of non copy types. But as the only list constructor requires `T` to be copy anyways, I think
  the improved readability is worth this potential future restriction.
- passes `mir::PlaceElem` by value. Mir currently has quite a few copy types which are passed by reference, e.g. `Local`. As I don't have a lot of experience working with MIR, I mostly did this to get some feedback from people who use MIR more frequently
- tries to reuse `ty::Predicate` in case it did not change in some places, which should hopefully
  fix the regression caused by #72055

r? @nikomatsakis for the first commit, which continues the work of #72055 and makes adding `PredicateKind::ForAll` slightly more pleasant. Feel free to reassign though
  • Loading branch information
bors committed May 28, 2020
2 parents 664fcd3 + f15e4b3 commit 4512721
Show file tree
Hide file tree
Showing 59 changed files with 194 additions and 196 deletions.
2 changes: 1 addition & 1 deletion src/librustc_codegen_ssa/debuginfo/type_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ pub fn push_debuginfo_type_name<'tcx>(
}
ty::Tuple(component_types) => {
output.push('(');
for &component_type in component_types {
for component_type in component_types {
push_debuginfo_type_name(tcx, component_type.expect_ty(), true, output, visited);
output.push_str(", ");
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_codegen_ssa/mir/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ impl<Bx: BuilderMethods<'a, 'tcx>> LocalAnalyzer<'mir, 'a, 'tcx, Bx> {
) {
let cx = self.fx.cx;

if let [proj_base @ .., elem] = place_ref.projection {
if let &[ref proj_base @ .., elem] = place_ref.projection {
let mut base_context = if context.is_mutating_use() {
PlaceContext::MutatingUse(MutatingUseContext::Projection)
} else {
Expand Down Expand Up @@ -186,7 +186,7 @@ impl<Bx: BuilderMethods<'a, 'tcx>> LocalAnalyzer<'mir, 'a, 'tcx, Bx> {
// now that we have moved to the "slice of projections" representation.
if let mir::ProjectionElem::Index(local) = elem {
self.visit_local(
local,
&local,
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
location,
);
Expand Down
14 changes: 7 additions & 7 deletions src/librustc_codegen_ssa/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
self.codegen_consume(bx, mir::PlaceRef { local, projection: proj_base })
.deref(bx.cx())
}
mir::PlaceRef { local, projection: [proj_base @ .., elem] } => {
mir::PlaceRef { local, projection: &[ref proj_base @ .., elem] } => {
// FIXME turn this recursion into iteration
let cg_base =
self.codegen_place(bx, mir::PlaceRef { local, projection: proj_base });
Expand All @@ -440,7 +440,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
cg_base.project_field(bx, field.index())
}
mir::ProjectionElem::Index(index) => {
let index = &mir::Operand::Copy(mir::Place::from(*index));
let index = &mir::Operand::Copy(mir::Place::from(index));
let index = self.codegen_operand(bx, index);
let llindex = index.immediate();
cg_base.project_index(bx, llindex)
Expand All @@ -450,22 +450,22 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
from_end: false,
min_length: _,
} => {
let lloffset = bx.cx().const_usize(*offset as u64);
let lloffset = bx.cx().const_usize(offset as u64);
cg_base.project_index(bx, lloffset)
}
mir::ProjectionElem::ConstantIndex {
offset,
from_end: true,
min_length: _,
} => {
let lloffset = bx.cx().const_usize(*offset as u64);
let lloffset = bx.cx().const_usize(offset as u64);
let lllen = cg_base.len(bx.cx());
let llindex = bx.sub(lllen, lloffset);
cg_base.project_index(bx, llindex)
}
mir::ProjectionElem::Subslice { from, to, from_end } => {
let mut subslice =
cg_base.project_index(bx, bx.cx().const_usize(*from as u64));
cg_base.project_index(bx, bx.cx().const_usize(from as u64));
let projected_ty =
PlaceTy::from_ty(cg_base.layout.ty).projection_ty(tcx, elem).ty;
subslice.layout = bx.cx().layout_of(self.monomorphize(&projected_ty));
Expand All @@ -474,7 +474,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
assert!(from_end, "slice subslices should be `from_end`");
subslice.llextra = Some(bx.sub(
cg_base.llextra.unwrap(),
bx.cx().const_usize((*from as u64) + (*to as u64)),
bx.cx().const_usize((from as u64) + (to as u64)),
));
}

Expand All @@ -487,7 +487,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {

subslice
}
mir::ProjectionElem::Downcast(_, v) => cg_base.project_downcast(bx, *v),
mir::ProjectionElem::Downcast(_, v) => cg_base.project_downcast(bx, v),
}
}
};
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_infer/infer/canonical/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
) -> CanonicalVarValues<'tcx> {
let var_values: IndexVec<BoundVar, GenericArg<'tcx>> = variables
.iter()
.map(|info| self.instantiate_canonical_var(span, *info, &universe_map))
.map(|info| self.instantiate_canonical_var(span, info, &universe_map))
.collect();

CanonicalVarValues { var_values }
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_infer/infer/canonical/query_response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,12 +464,12 @@ impl<'cx, 'tcx> InferCtxt<'cx, 'tcx> {
if info.is_existential() {
match opt_values[BoundVar::new(index)] {
Some(k) => k,
None => self.instantiate_canonical_var(cause.span, *info, |u| {
None => self.instantiate_canonical_var(cause.span, info, |u| {
universe_map[u.as_usize()]
}),
}
} else {
self.instantiate_canonical_var(cause.span, *info, |u| {
self.instantiate_canonical_var(cause.span, info, |u| {
universe_map[u.as_usize()]
})
}
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_infer/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -970,7 +970,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
&self,
cause: &ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
predicate: &ty::PolySubtypePredicate<'tcx>,
predicate: ty::PolySubtypePredicate<'tcx>,
) -> Option<InferResult<'tcx, ()>> {
// Subtle: it's ok to skip the binder here and resolve because
// `shallow_resolve` just ignores anything that is not a type
Expand All @@ -993,7 +993,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {

Some(self.commit_if_ok(|snapshot| {
let (ty::SubtypePredicate { a_is_expected, a, b }, placeholder_map) =
self.replace_bound_vars_with_placeholders(predicate);
self.replace_bound_vars_with_placeholders(&predicate);

let ok = self.at(cause, param_env).sub_exp(a_is_expected, a, b)?;

Expand All @@ -1006,11 +1006,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
pub fn region_outlives_predicate(
&self,
cause: &traits::ObligationCause<'tcx>,
predicate: &ty::PolyRegionOutlivesPredicate<'tcx>,
predicate: ty::PolyRegionOutlivesPredicate<'tcx>,
) -> UnitResult<'tcx> {
self.commit_if_ok(|snapshot| {
let (ty::OutlivesPredicate(r_a, r_b), placeholder_map) =
self.replace_bound_vars_with_placeholders(predicate);
self.replace_bound_vars_with_placeholders(&predicate);
let origin = SubregionOrigin::from_obligation_cause(cause, || {
RelateRegionParamBound(cause.span)
});
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_infer/infer/outlives/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
// for further background and discussion.
let mut bounds = substs
.iter()
.filter_map(|&child| match child.unpack() {
.filter_map(|child| match child.unpack() {
GenericArgKind::Type(ty) => Some(self.type_bound(ty)),
GenericArgKind::Lifetime(_) => None,
GenericArgKind::Const(_) => Some(self.recursive_bound(child)),
Expand Down Expand Up @@ -334,10 +334,10 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> {
fn collect_outlives_from_predicate_list(
&self,
compare_ty: impl Fn(Ty<'tcx>) -> bool,
predicates: impl Iterator<Item = impl AsRef<ty::Predicate<'tcx>>>,
predicates: impl Iterator<Item = ty::Predicate<'tcx>>,
) -> impl Iterator<Item = ty::OutlivesPredicate<Ty<'tcx>, ty::Region<'tcx>>> {
predicates
.filter_map(|p| p.as_ref().to_opt_type_outlives())
.filter_map(|p| p.to_opt_type_outlives())
.filter_map(|p| p.no_bound_vars())
.filter(move |p| compare_ty(p.0))
}
Expand Down
46 changes: 20 additions & 26 deletions src/librustc_infer/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,52 +8,46 @@ use rustc_span::Span;

pub fn anonymize_predicate<'tcx>(
tcx: TyCtxt<'tcx>,
pred: &ty::Predicate<'tcx>,
pred: ty::Predicate<'tcx>,
) -> ty::Predicate<'tcx> {
match pred.kind() {
let kind = pred.kind();
let new = match kind {
&ty::PredicateKind::Trait(ref data, constness) => {
ty::PredicateKind::Trait(tcx.anonymize_late_bound_regions(data), constness)
.to_predicate(tcx)
}

ty::PredicateKind::RegionOutlives(data) => {
ty::PredicateKind::RegionOutlives(tcx.anonymize_late_bound_regions(data))
.to_predicate(tcx)
}

ty::PredicateKind::TypeOutlives(data) => {
ty::PredicateKind::TypeOutlives(tcx.anonymize_late_bound_regions(data))
.to_predicate(tcx)
}

ty::PredicateKind::Projection(data) => {
ty::PredicateKind::Projection(tcx.anonymize_late_bound_regions(data)).to_predicate(tcx)
ty::PredicateKind::Projection(tcx.anonymize_late_bound_regions(data))
}

&ty::PredicateKind::WellFormed(data) => {
ty::PredicateKind::WellFormed(data).to_predicate(tcx)
}
&ty::PredicateKind::WellFormed(data) => ty::PredicateKind::WellFormed(data),

&ty::PredicateKind::ObjectSafe(data) => {
ty::PredicateKind::ObjectSafe(data).to_predicate(tcx)
}
&ty::PredicateKind::ObjectSafe(data) => ty::PredicateKind::ObjectSafe(data),

&ty::PredicateKind::ClosureKind(closure_def_id, closure_substs, kind) => {
ty::PredicateKind::ClosureKind(closure_def_id, closure_substs, kind).to_predicate(tcx)
ty::PredicateKind::ClosureKind(closure_def_id, closure_substs, kind)
}

ty::PredicateKind::Subtype(data) => {
ty::PredicateKind::Subtype(tcx.anonymize_late_bound_regions(data)).to_predicate(tcx)
ty::PredicateKind::Subtype(tcx.anonymize_late_bound_regions(data))
}

&ty::PredicateKind::ConstEvaluatable(def_id, substs) => {
ty::PredicateKind::ConstEvaluatable(def_id, substs).to_predicate(tcx)
ty::PredicateKind::ConstEvaluatable(def_id, substs)
}

ty::PredicateKind::ConstEquate(c1, c2) => {
ty::PredicateKind::ConstEquate(c1, c2).to_predicate(tcx)
}
}
ty::PredicateKind::ConstEquate(c1, c2) => ty::PredicateKind::ConstEquate(c1, c2),
};

if new != *kind { new.to_predicate(tcx) } else { pred }
}

struct PredicateSet<'tcx> {
Expand All @@ -66,7 +60,7 @@ impl PredicateSet<'tcx> {
Self { tcx, set: Default::default() }
}

fn insert(&mut self, pred: &ty::Predicate<'tcx>) -> bool {
fn insert(&mut self, pred: ty::Predicate<'tcx>) -> bool {
// We have to be careful here because we want
//
// for<'a> Foo<&'a int>
Expand All @@ -81,10 +75,10 @@ impl PredicateSet<'tcx> {
}
}

impl<T: AsRef<ty::Predicate<'tcx>>> Extend<T> for PredicateSet<'tcx> {
fn extend<I: IntoIterator<Item = T>>(&mut self, iter: I) {
impl Extend<ty::Predicate<'tcx>> for PredicateSet<'tcx> {
fn extend<I: IntoIterator<Item = ty::Predicate<'tcx>>>(&mut self, iter: I) {
for pred in iter {
self.insert(pred.as_ref());
self.insert(pred);
}
}
}
Expand Down Expand Up @@ -132,7 +126,7 @@ pub fn elaborate_obligations<'tcx>(
mut obligations: Vec<PredicateObligation<'tcx>>,
) -> Elaborator<'tcx> {
let mut visited = PredicateSet::new(tcx);
obligations.retain(|obligation| visited.insert(&obligation.predicate));
obligations.retain(|obligation| visited.insert(obligation.predicate));
Elaborator { stack: obligations, visited }
}

Expand Down Expand Up @@ -172,7 +166,7 @@ impl Elaborator<'tcx> {
// cases. One common case is when people define
// `trait Sized: Sized { }` rather than `trait Sized { }`.
let visited = &mut self.visited;
let obligations = obligations.filter(|o| visited.insert(&o.predicate));
let obligations = obligations.filter(|o| visited.insert(o.predicate));

self.stack.extend(obligations);
}
Expand Down Expand Up @@ -260,7 +254,7 @@ impl Elaborator<'tcx> {
}
})
.map(|predicate_kind| predicate_kind.to_predicate(tcx))
.filter(|predicate| visited.insert(predicate))
.filter(|&predicate| visited.insert(predicate))
.map(|predicate| predicate_obligation(predicate, None)),
);
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_middle/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2077,10 +2077,10 @@ impl Debug for Place<'_> {
ProjectionElem::ConstantIndex { offset, min_length, from_end: true } => {
write!(fmt, "[-{:?} of {:?}]", offset, min_length)?;
}
ProjectionElem::Subslice { from, to, from_end: true } if *to == 0 => {
ProjectionElem::Subslice { from, to, from_end: true } if to == 0 => {
write!(fmt, "[{:?}:]", from)?;
}
ProjectionElem::Subslice { from, to, from_end: true } if *from == 0 => {
ProjectionElem::Subslice { from, to, from_end: true } if from == 0 => {
write!(fmt, "[:-{:?}]", to)?;
}
ProjectionElem::Subslice { from, to, from_end: true } => {
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_middle/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ impl<'tcx> PlaceTy<'tcx> {
/// Convenience wrapper around `projection_ty_core` for
/// `PlaceElem`, where we can just use the `Ty` that is already
/// stored inline on field projection elems.
pub fn projection_ty(self, tcx: TyCtxt<'tcx>, elem: &PlaceElem<'tcx>) -> PlaceTy<'tcx> {
self.projection_ty_core(tcx, ty::ParamEnv::empty(), elem, |_, _, ty| ty)
pub fn projection_ty(self, tcx: TyCtxt<'tcx>, elem: PlaceElem<'tcx>) -> PlaceTy<'tcx> {
self.projection_ty_core(tcx, ty::ParamEnv::empty(), &elem, |_, _, ty| ty)
}

/// `place_ty.projection_ty_core(tcx, elem, |...| { ... })`
Expand Down Expand Up @@ -124,7 +124,7 @@ impl<'tcx> Place<'tcx> {
{
projection
.iter()
.fold(PlaceTy::from_ty(local_decls.local_decls()[local].ty), |place_ty, elem| {
.fold(PlaceTy::from_ty(local_decls.local_decls()[local].ty), |place_ty, &elem| {
place_ty.projection_ty(tcx, elem)
})
}
Expand Down
16 changes: 8 additions & 8 deletions src/librustc_middle/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ macro_rules! visit_place_fns {
let mut projection = Cow::Borrowed(projection);

for i in 0..projection.len() {
if let Some(elem) = projection.get(i) {
if let Some(&elem) = projection.get(i) {
if let Some(elem) = self.process_projection_elem(elem, location) {
// This converts the borrowed projection into `Cow::Owned(_)` and returns a
// clone of the projection so we can mutate and reintern later.
Expand All @@ -921,19 +921,19 @@ macro_rules! visit_place_fns {

fn process_projection_elem(
&mut self,
elem: &PlaceElem<'tcx>,
elem: PlaceElem<'tcx>,
location: Location,
) -> Option<PlaceElem<'tcx>> {
match elem {
PlaceElem::Index(local) => {
let mut new_local = *local;
let mut new_local = local;
self.visit_local(
&mut new_local,
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
location,
);

if new_local == *local { None } else { Some(PlaceElem::Index(new_local)) }
if new_local == local { None } else { Some(PlaceElem::Index(new_local)) }
}
PlaceElem::Deref
| PlaceElem::Field(..)
Expand All @@ -959,7 +959,7 @@ macro_rules! visit_place_fns {
&mut self,
local: Local,
proj_base: &[PlaceElem<'tcx>],
elem: &PlaceElem<'tcx>,
elem: PlaceElem<'tcx>,
context: PlaceContext,
location: Location,
) {
Expand Down Expand Up @@ -990,7 +990,7 @@ macro_rules! visit_place_fns {
location: Location,
) {
let mut cursor = projection;
while let [proj_base @ .., elem] = cursor {
while let &[ref proj_base @ .., elem] = cursor {
cursor = proj_base;
self.visit_projection_elem(local, cursor, elem, context, location);
}
Expand All @@ -1000,7 +1000,7 @@ macro_rules! visit_place_fns {
&mut self,
_local: Local,
_proj_base: &[PlaceElem<'tcx>],
elem: &PlaceElem<'tcx>,
elem: PlaceElem<'tcx>,
_context: PlaceContext,
location: Location,
) {
Expand All @@ -1010,7 +1010,7 @@ macro_rules! visit_place_fns {
}
ProjectionElem::Index(local) => {
self.visit_local(
local,
&local,
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
location,
);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_middle/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl FlagComputation {
&ty::Dynamic(ref obj, r) => {
let mut computation = FlagComputation::new();
for predicate in obj.skip_binder().iter() {
match *predicate {
match predicate {
ty::ExistentialPredicate::Trait(tr) => computation.add_substs(tr.substs),
ty::ExistentialPredicate::Projection(p) => {
let mut proj_computation = FlagComputation::new();
Expand Down
Loading

0 comments on commit 4512721

Please sign in to comment.