Skip to content

Use rebind instead of Binder::bind when possible #77685

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 4 commits into from
Oct 17, 2020
Merged
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
27 changes: 14 additions & 13 deletions compiler/rustc_codegen_llvm/src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -673,17 +673,9 @@ fn codegen_emcc_try(
fn gen_fn<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
name: &str,
inputs: Vec<Ty<'tcx>>,
output: Ty<'tcx>,
rust_fn_sig: ty::PolyFnSig<'tcx>,
codegen: &mut dyn FnMut(Builder<'_, 'll, 'tcx>),
) -> &'ll Value {
let rust_fn_sig = ty::Binder::bind(cx.tcx.mk_fn_sig(
inputs.into_iter(),
output,
false,
hir::Unsafety::Unsafe,
Abi::Rust,
));
let fn_abi = FnAbi::of_fn_ptr(cx, rust_fn_sig, &[]);
let llfn = cx.declare_fn(name, &fn_abi);
cx.set_frame_pointer_elimination(llfn);
Expand All @@ -710,22 +702,31 @@ fn get_rust_try_fn<'ll, 'tcx>(
// Define the type up front for the signature of the rust_try function.
let tcx = cx.tcx;
let i8p = tcx.mk_mut_ptr(tcx.types.i8);
let try_fn_ty = tcx.mk_fn_ptr(ty::Binder::bind(tcx.mk_fn_sig(
// `unsafe fn(*mut i8) -> ()`
let try_fn_ty = tcx.mk_fn_ptr(ty::Binder::dummy(tcx.mk_fn_sig(
iter::once(i8p),
tcx.mk_unit(),
false,
hir::Unsafety::Unsafe,
Abi::Rust,
)));
let catch_fn_ty = tcx.mk_fn_ptr(ty::Binder::bind(tcx.mk_fn_sig(
// `unsafe fn(*mut i8, *mut i8) -> ()`
let catch_fn_ty = tcx.mk_fn_ptr(ty::Binder::dummy(tcx.mk_fn_sig(
[i8p, i8p].iter().cloned(),
tcx.mk_unit(),
false,
hir::Unsafety::Unsafe,
Abi::Rust,
)));
let output = tcx.types.i32;
let rust_try = gen_fn(cx, "__rust_try", vec![try_fn_ty, i8p, catch_fn_ty], output, codegen);
// `unsafe fn(unsafe fn(*mut i8) -> (), *mut i8, unsafe fn(*mut i8, *mut i8) -> ()) -> i32`
let rust_fn_sig = ty::Binder::dummy(cx.tcx.mk_fn_sig(
vec![try_fn_ty, i8p, catch_fn_ty].into_iter(),
tcx.types.i32,
false,
hir::Unsafety::Unsafe,
Abi::Rust,
));
let rust_try = gen_fn(cx, "__rust_try", rust_fn_sig, codegen);
cx.rust_try_fn.set(Some(rust_try));
rust_try
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_infer/src/infer/nll_relate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ where
if let (Some(a), Some(b)) = (a.no_bound_vars(), b.no_bound_vars()) {
// Fast path for the common case.
self.relate(a, b)?;
return Ok(ty::Binder::bind(a));
return Ok(ty::Binder::dummy(a));
}

if self.ambient_covariance() {
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_infer/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,14 +126,15 @@ impl Elaborator<'tcx> {
fn elaborate(&mut self, obligation: &PredicateObligation<'tcx>) {
let tcx = self.visited.tcx;

match obligation.predicate.skip_binders() {
let bound_predicate = obligation.predicate.bound_atom();
match bound_predicate.skip_binder() {
ty::PredicateAtom::Trait(data, _) => {
// Get predicates declared on the trait.
let predicates = tcx.super_predicates_of(data.def_id());

let obligations = predicates.predicates.iter().map(|&(pred, _)| {
predicate_obligation(
pred.subst_supertrait(tcx, &ty::Binder::bind(data.trait_ref)),
pred.subst_supertrait(tcx, &bound_predicate.rebind(data.trait_ref)),
obligation.param_env,
obligation.cause.clone(),
)
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,9 +1056,21 @@ impl<'tcx> Predicate<'tcx> {
}
}

/// Converts this to a `Binder<PredicateAtom<'tcx>>`. If the value was an
/// `Atom`, then it is not allowed to contain escaping bound vars.
pub fn bound_atom(self) -> Binder<PredicateAtom<'tcx>> {
match self.kind() {
&PredicateKind::ForAll(binder) => binder,
&PredicateKind::Atom(atom) => {
debug_assert!(!atom.has_escaping_bound_vars());
Binder::dummy(atom)
}
}
}

/// Allows using a `Binder<PredicateAtom<'tcx>>` even if the given predicate previously
/// contained unbound variables by shifting these variables outwards.
pub fn bound_atom(self, tcx: TyCtxt<'tcx>) -> Binder<PredicateAtom<'tcx>> {
pub fn bound_atom_with_opt_escaping(self, tcx: TyCtxt<'tcx>) -> Binder<PredicateAtom<'tcx>> {
match self.kind() {
&PredicateKind::ForAll(binder) => binder,
&PredicateKind::Atom(atom) => Binder::wrap_nonbinding(tcx, atom),
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -618,10 +618,9 @@ pub trait PrettyPrinter<'tcx>:
// may contain unbound variables. We therefore do this manually.
//
// FIXME(lcnr): Find out why exactly this is the case :)
if let ty::PredicateAtom::Trait(pred, _) =
predicate.bound_atom(self.tcx()).skip_binder()
{
let trait_ref = ty::Binder::bind(pred.trait_ref);
let bound_predicate = predicate.bound_atom_with_opt_escaping(self.tcx());
if let ty::PredicateAtom::Trait(pred, _) = bound_predicate.skip_binder() {
let trait_ref = bound_predicate.rebind(pred.trait_ref);
// Don't print +Sized, but rather +?Sized if absent.
if Some(trait_ref.def_id()) == self.tcx().lang_items().sized_trait() {
is_sized = true;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ impl<'a, 'tcx> Lift<'tcx> for ty::PredicateAtom<'a> {
impl<'tcx, T: Lift<'tcx>> Lift<'tcx> for ty::Binder<T> {
type Lifted = ty::Binder<T::Lifted>;
fn lift_to_tcx(&self, tcx: TyCtxt<'tcx>) -> Option<Self::Lifted> {
tcx.lift(self.as_ref().skip_binder()).map(ty::Binder::bind)
tcx.lift(self.as_ref().skip_binder()).map(|v| self.rebind(v))
}
}

Expand Down
28 changes: 21 additions & 7 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -703,14 +703,16 @@ impl<'tcx> Binder<ExistentialPredicate<'tcx>> {
use crate::ty::ToPredicate;
match self.skip_binder() {
ExistentialPredicate::Trait(tr) => {
Binder(tr).with_self_ty(tcx, self_ty).without_const().to_predicate(tcx)
self.rebind(tr).with_self_ty(tcx, self_ty).without_const().to_predicate(tcx)
}
ExistentialPredicate::Projection(p) => {
Binder(p.with_self_ty(tcx, self_ty)).to_predicate(tcx)
self.rebind(p.with_self_ty(tcx, self_ty)).to_predicate(tcx)
}
ExistentialPredicate::AutoTrait(did) => {
let trait_ref =
Binder(ty::TraitRef { def_id: did, substs: tcx.mk_substs_trait(self_ty, &[]) });
let trait_ref = self.rebind(ty::TraitRef {
def_id: did,
substs: tcx.mk_substs_trait(self_ty, &[]),
});
trait_ref.without_const().to_predicate(tcx)
}
}
Expand Down Expand Up @@ -775,7 +777,7 @@ impl<'tcx> List<ExistentialPredicate<'tcx>> {

impl<'tcx> Binder<&'tcx List<ExistentialPredicate<'tcx>>> {
pub fn principal(&self) -> Option<ty::Binder<ExistentialTraitRef<'tcx>>> {
self.skip_binder().principal().map(Binder::bind)
self.map_bound(|b| b.principal()).transpose()
}

pub fn principal_def_id(&self) -> Option<DefId> {
Expand Down Expand Up @@ -858,8 +860,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() })
self.map_bound(|trait_ref| ty::TraitPredicate { trait_ref })
}
}

Expand Down Expand Up @@ -1001,6 +1002,19 @@ impl<T> Binder<T> {
Binder(f(self.0))
}

/// Wraps a `value` in a binder, using the same bound variables as the
/// current `Binder`. This should not be used if the new value *changes*
/// the bound variables. Note: the (old or new) value itself does not
/// necessarily need to *name* all the bound variables.
///
/// This currently doesn't do anything different than `bind`, because we
/// don't actually track bound vars. However, semantically, it is different
/// because bound vars aren't allowed to change here, whereas they are
/// in `bind`. This may be (debug) asserted in the future.
pub fn rebind<U>(&self, value: U) -> Binder<U> {
Copy link
Contributor

Choose a reason for hiding this comment

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

It might make sense to add a comment here which says that we do not yet track bound variables.

I am also kind of suprised that none of the methods on ty::Binder are marked as inline even though they are very frequently used and really simple. Might be interesting to check if that has a perf impact.

Copy link
Member Author

Choose a reason for hiding this comment

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

I will add a comment. As I said in other comment, leaving inline to another PR.

Binder(value)
}

/// Unwraps and returns the value within, but only if it contains
/// no bound vars at all. (In other words, if this binder --
/// and indeed any enclosing binder -- doesn't bind anything at
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_trait_selection/src/traits/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,18 +642,19 @@ impl AutoTraitFinder<'tcx> {
// We check this by calling is_of_param on the relevant types
// from the various possible predicates

match predicate.skip_binders() {
let bound_predicate = predicate.bound_atom();
match bound_predicate.skip_binder() {
ty::PredicateAtom::Trait(p, _) => {
if self.is_param_no_infer(p.trait_ref.substs)
&& !only_projections
&& is_new_pred
{
self.add_user_pred(computed_preds, predicate);
}
predicates.push_back(ty::Binder::bind(p));
predicates.push_back(bound_predicate.rebind(p));
}
ty::PredicateAtom::Projection(p) => {
let p = ty::Binder::bind(p);
let p = bound_predicate.rebind(p);
debug!(
"evaluate_nested_obligations: examining projection predicate {:?}",
predicate
Expand Down Expand Up @@ -783,13 +784,13 @@ impl AutoTraitFinder<'tcx> {
}
}
ty::PredicateAtom::RegionOutlives(binder) => {
let binder = ty::Binder::bind(binder);
let binder = bound_predicate.rebind(binder);
if select.infcx().region_outlives_predicate(&dummy_cause, binder).is_err() {
return false;
}
}
ty::PredicateAtom::TypeOutlives(binder) => {
let binder = ty::Binder::bind(binder);
let binder = bound_predicate.rebind(binder);
match (
binder.no_bound_vars(),
binder.map_bound_ref(|pred| pred.0).no_bound_vars(),
Expand Down
31 changes: 18 additions & 13 deletions compiler/rustc_trait_selection/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,10 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
return;
}

match obligation.predicate.skip_binders() {
let bound_predicate = obligation.predicate.bound_atom();
match bound_predicate.skip_binder() {
ty::PredicateAtom::Trait(trait_predicate, _) => {
let trait_predicate = ty::Binder::bind(trait_predicate);
let trait_predicate = bound_predicate.rebind(trait_predicate);
let trait_predicate = self.resolve_vars_if_possible(&trait_predicate);

if self.tcx.sess.has_errors() && trait_predicate.references_error() {
Expand Down Expand Up @@ -531,7 +532,7 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
}

ty::PredicateAtom::RegionOutlives(predicate) => {
let predicate = ty::Binder::bind(predicate);
let predicate = bound_predicate.rebind(predicate);
let predicate = self.resolve_vars_if_possible(&predicate);
let err = self
.region_outlives_predicate(&obligation.cause, predicate)
Expand Down Expand Up @@ -1078,9 +1079,10 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
}

// FIXME: It should be possible to deal with `ForAll` in a cleaner way.
let (cond, error) = match (cond.skip_binders(), error.skip_binders()) {
let bound_error = error.bound_atom();
let (cond, error) = match (cond.skip_binders(), bound_error.skip_binder()) {
(ty::PredicateAtom::Trait(..), ty::PredicateAtom::Trait(error, _)) => {
(cond, ty::Binder::bind(error))
(cond, bound_error.rebind(error))
}
_ => {
// FIXME: make this work in other cases too.
Expand All @@ -1089,9 +1091,10 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
};

for obligation in super::elaborate_predicates(self.tcx, std::iter::once(cond)) {
if let ty::PredicateAtom::Trait(implication, _) = obligation.predicate.skip_binders() {
let bound_predicate = obligation.predicate.bound_atom();
if let ty::PredicateAtom::Trait(implication, _) = bound_predicate.skip_binder() {
let error = error.to_poly_trait_ref();
let implication = ty::Binder::bind(implication.trait_ref);
let implication = bound_predicate.rebind(implication.trait_ref);
// FIXME: I'm just not taking associated types at all here.
// Eventually I'll need to implement param-env-aware
// `Γ₁ ⊦ φ₁ => Γ₂ ⊦ φ₂` logic.
Expand Down Expand Up @@ -1169,12 +1172,13 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
//
// this can fail if the problem was higher-ranked, in which
// cause I have no idea for a good error message.
if let ty::PredicateAtom::Projection(data) = predicate.skip_binders() {
let bound_predicate = predicate.bound_atom();
if let ty::PredicateAtom::Projection(data) = bound_predicate.skip_binder() {
let mut selcx = SelectionContext::new(self);
let (data, _) = self.replace_bound_vars_with_fresh_vars(
obligation.cause.span,
infer::LateBoundRegionConversionTime::HigherRankedType,
&ty::Binder::bind(data),
&bound_predicate.rebind(data),
);
let mut obligations = vec![];
let normalized_ty = super::normalize_projection_type(
Expand Down Expand Up @@ -1455,10 +1459,11 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
return;
}

let mut err = match predicate.skip_binders() {
let bound_predicate = predicate.bound_atom();
let mut err = match bound_predicate.skip_binder() {
ty::PredicateAtom::Trait(data, _) => {
let trait_ref = ty::Binder::bind(data.trait_ref);
let self_ty = trait_ref.skip_binder().self_ty();
let self_ty = data.trait_ref.self_ty();
let trait_ref = bound_predicate.rebind(data.trait_ref);
debug!("self_ty {:?} {:?} trait_ref {:?}", self_ty, self_ty.kind(), trait_ref);

if predicate.references_error() {
Expand Down Expand Up @@ -1582,7 +1587,7 @@ impl<'a, 'tcx> InferCtxtPrivExt<'tcx> for InferCtxt<'a, 'tcx> {
self.emit_inference_failure_err(body_id, span, a.into(), ErrorCode::E0282)
}
ty::PredicateAtom::Projection(data) => {
let trait_ref = ty::Binder::bind(data).to_poly_trait_ref(self.tcx);
let trait_ref = bound_predicate.rebind(data).to_poly_trait_ref(self.tcx);
let self_ty = trait_ref.skip_binder().self_ty();
let ty = data.ty;
if predicate.references_error() {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
// This means we need to pass it the bound version of our
// predicate.
ty::PredicateAtom::Trait(trait_ref, _constness) => {
let trait_obligation = obligation.with(Binder::bind(trait_ref));
let trait_obligation = obligation.with(binder.rebind(trait_ref));

self.process_trait_obligation(
obligation,
Expand All @@ -362,7 +362,7 @@ impl<'a, 'b, 'tcx> FulfillProcessor<'a, 'b, 'tcx> {
)
}
ty::PredicateAtom::Projection(data) => {
let project_obligation = obligation.with(Binder::bind(data));
let project_obligation = obligation.with(binder.rebind(data));

self.process_projection_obligation(
project_obligation,
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,8 @@ fn prune_cache_value_obligations<'a, 'tcx>(
.obligations
.iter()
.filter(|obligation| {
match obligation.predicate.skip_binders() {
let bound_predicate = obligation.predicate.bound_atom();
match bound_predicate.skip_binder() {
// We found a `T: Foo<X = U>` predicate, let's check
// if `U` references any unresolved type
// variables. In principle, we only care if this
Expand All @@ -634,7 +635,7 @@ fn prune_cache_value_obligations<'a, 'tcx>(
// but we have `T: Foo<X = ?1>` and `?1: Bar<X =
// ?0>`).
ty::PredicateAtom::Projection(data) => {
infcx.unresolved_type_vars(&ty::Binder::bind(data.ty)).is_some()
infcx.unresolved_type_vars(&bound_predicate.rebind(data.ty)).is_some()
}

// We are only interested in `T: Foo<X = U>` predicates, whre
Expand Down Expand Up @@ -907,8 +908,9 @@ fn assemble_candidates_from_predicates<'cx, 'tcx>(
let infcx = selcx.infcx();
for predicate in env_predicates {
debug!(?predicate);
let bound_predicate = predicate.bound_atom();
if let ty::PredicateAtom::Projection(data) = predicate.skip_binders() {
let data = ty::Binder::bind(data);
let data = bound_predicate.rebind(data);
let same_def_id = data.projection_def_id() == obligation.predicate.item_def_id;

let is_match = same_def_id
Expand Down
Loading