Skip to content

Commit

Permalink
Rollup merge of rust-lang#54295 - ljedrz:cleanups_rustc_traits, r=nik…
Browse files Browse the repository at this point in the history
…omatsakis

A few cleanups and minor improvements to rustc/traits

It's a little bigger than usual, so bear with me ^^:

- introduce `TyCtxt::all_impls` and use it to avoid inefficiently allocating push loops
- modify `ArgKind::from_expected_ty` to take an `Option<Span>` argument to make it more versatile
- replace `ArgKind::Arg("_".to_owned(), "_".to_owned())` with `ArgKind::empty`
- move early `return`s earlier where possible
- if all branches of a `match` end with the same expression, move it after it
- change a hacky `match` expression to an `if else` chain
- move the `push` out from a push loop closure to reduce the number of allocations
- correct the vector size for `pretty_predicates` (under `specialize`)
- take advantage of the fact that `if else` is an expression
- prefer `cloned()` to `map(|&x| x)` and `map(|x| *x)`
- prefer `vec![x; y.len()]` to `y.map(|_| x).collect()`
- use `unwrap_or_else` instead of `match` where applicable
- use `if let` instead of `match` when only one branch matters
- prefer `to_owned` to `to_string` for string literals
- remove explicit `return`s
- remove superfluous braces
- whitespace fixes
- several other minor improvements
  • Loading branch information
kennytm authored Sep 20, 2018
2 parents cf06e03 + a8ec8e5 commit 2d0262e
Show file tree
Hide file tree
Showing 18 changed files with 414 additions and 466 deletions.
18 changes: 10 additions & 8 deletions src/librustc/traits/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,17 +112,18 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
orig_params,
trait_pred.to_poly_trait_predicate(),
));

match result {
Ok(Some(Vtable::VtableImpl(_))) => {
debug!(
"find_auto_trait_generics(did={:?}, trait_did={:?}, generics={:?}): \
manual impl found, bailing out",
did, trait_did, generics
);
return true;
true
}
_ => return false,
};
_ => false
}
});

// If an explicit impl exists, it always takes priority over an auto impl
Expand Down Expand Up @@ -426,6 +427,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
if new_trait.def_id() == old_trait.def_id() {
let new_substs = new_trait.skip_binder().trait_ref.substs;
let old_substs = old_trait.skip_binder().trait_ref.substs;

if !new_substs.types().eq(old_substs.types()) {
// We can't compare lifetimes if the types are different,
// so skip checking old_pred
Expand Down Expand Up @@ -489,12 +491,12 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {

pub fn get_lifetime(&self, region: Region, names_map: &FxHashMap<String, String>) -> String {
self.region_name(region)
.map(|name| {
names_map.get(&name).unwrap_or_else(|| {
.map(|name|
names_map.get(&name).unwrap_or_else(||
panic!("Missing lifetime with name {:?} for {:?}", name, region)
})
})
.unwrap_or(&"'static".to_string())
)
)
.unwrap_or(&"'static".to_owned())
.clone()
}

Expand Down
31 changes: 12 additions & 19 deletions src/librustc/traits/codegen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ pub fn codegen_fulfill_obligation<'a, 'tcx>(ty: TyCtxt<'a, 'tcx, 'tcx>,
let trait_ref = ty.erase_regions(&trait_ref);

debug!("codegen_fulfill_obligation(trait_ref={:?}, def_id={:?})",
(param_env, trait_ref), trait_ref.def_id());
(param_env, trait_ref), trait_ref.def_id());

// Do the initial selection for the obligation. This yields the
// shallow result we are looking for -- that is, what specific impl.
Expand All @@ -48,8 +48,8 @@ pub fn codegen_fulfill_obligation<'a, 'tcx>(ty: TyCtxt<'a, 'tcx, 'tcx>,

let obligation_cause = ObligationCause::dummy();
let obligation = Obligation::new(obligation_cause,
param_env,
trait_ref.to_poly_trait_predicate());
param_env,
trait_ref.to_poly_trait_predicate());

let selection = match selcx.select(&obligation) {
Ok(Some(selection)) => selection,
Expand All @@ -61,12 +61,11 @@ pub fn codegen_fulfill_obligation<'a, 'tcx>(ty: TyCtxt<'a, 'tcx, 'tcx>,
// overflow bug, since I believe this is the only case
// where ambiguity can result.
bug!("Encountered ambiguity selecting `{:?}` during codegen, \
presuming due to overflow",
trait_ref)
presuming due to overflow",
trait_ref)
}
Err(e) => {
bug!("Encountered error `{:?}` selecting `{:?}` during codegen",
e, trait_ref)
bug!("Encountered error `{:?}` selecting `{:?}` during codegen", e, trait_ref)
}
};

Expand Down Expand Up @@ -163,22 +162,16 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
// In principle, we only need to do this so long as `result`
// contains unbound type parameters. It could be a slight
// optimization to stop iterating early.
match fulfill_cx.select_all_or_error(self) {
Ok(()) => { }
Err(errors) => {
span_bug!(span, "Encountered errors `{:?}` resolving bounds after type-checking",
errors);
}
if let Err(errors) = fulfill_cx.select_all_or_error(self) {
span_bug!(span, "Encountered errors `{:?}` resolving bounds after type-checking",
errors);
}

let result = self.resolve_type_vars_if_possible(result);
let result = self.tcx.erase_regions(&result);

match self.tcx.lift_to_global(&result) {
Some(result) => result,
None => {
span_bug!(span, "Uninferred types/regions in `{:?}`", result);
}
}
self.tcx.lift_to_global(&result).unwrap_or_else(||
span_bug!(span, "Uninferred types/regions in `{:?}`", result)
)
}
}
21 changes: 8 additions & 13 deletions src/librustc/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,9 +115,7 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
b_def_id: DefId)
-> Option<OverlapResult<'tcx>>
{
debug!("overlap(a_def_id={:?}, b_def_id={:?})",
a_def_id,
b_def_id);
debug!("overlap(a_def_id={:?}, b_def_id={:?})", a_def_id, b_def_id);

// For the purposes of this check, we don't bring any skolemized
// types into scope; instead, we replace the generic types with
Expand All @@ -133,10 +131,9 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,

// Do `a` and `b` unify? If not, no overlap.
let obligations = match selcx.infcx().at(&ObligationCause::dummy(), param_env)
.eq_impl_headers(&a_impl_header, &b_impl_header) {
Ok(InferOk { obligations, value: () }) => {
obligations
}
.eq_impl_headers(&a_impl_header, &b_impl_header)
{
Ok(InferOk { obligations, value: () }) => obligations,
Err(_) => return None
};

Expand Down Expand Up @@ -164,7 +161,7 @@ fn overlap<'cx, 'gcx, 'tcx>(selcx: &mut SelectionContext<'cx, 'gcx, 'tcx>,
return None
}

let impl_header = selcx.infcx().resolve_type_vars_if_possible(&a_impl_header);
let impl_header = selcx.infcx().resolve_type_vars_if_possible(&a_impl_header);
let intercrate_ambiguity_causes = selcx.take_intercrate_ambiguity_causes();
debug!("overlap: intercrate_ambiguity_causes={:#?}", intercrate_ambiguity_causes);
Some(OverlapResult { impl_header, intercrate_ambiguity_causes })
Expand Down Expand Up @@ -471,14 +468,12 @@ fn ty_is_local_constructor(ty: Ty, in_crate: InCrate) -> bool {
ty::Foreign(did) => def_id_is_local(did, in_crate),

ty::Dynamic(ref tt, ..) => {
tt.principal().map_or(false, |p| {
tt.principal().map_or(false, |p|
def_id_is_local(p.def_id(), in_crate)
})
)
}

ty::Error => {
true
}
ty::Error => true,

ty::Closure(..) |
ty::Generator(..) |
Expand Down
Loading

0 comments on commit 2d0262e

Please sign in to comment.