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

Remove Binder::bind() and use Binder::dummy() #83825

Closed
wants to merge 1 commit 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
8 changes: 0 additions & 8 deletions compiler/rustc_middle/src/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use self::TyKind::*;

use crate::infer::canonical::Canonical;
use crate::ty::fold::BoundVarsCollector;
use crate::ty::fold::ValidateBoundVars;
use crate::ty::subst::{GenericArg, InternalSubsts, Subst, SubstsRef};
use crate::ty::InferTy::{self, *};
Expand Down Expand Up @@ -970,13 +969,6 @@ where
Binder(value, ty::List::empty())
}

/// Wraps `value` in a binder, binding higher-ranked vars (if any).
pub fn bind(value: T, tcx: TyCtxt<'tcx>) -> Binder<'tcx, T> {
let mut collector = BoundVarsCollector::new();
value.visit_with(&mut collector);
Binder(value, collector.into_vars(tcx))
}

pub fn bind_with_vars(value: T, vars: &'tcx List<BoundVariableKind>) -> Binder<'tcx, T> {
if cfg!(debug_assertions) {
let mut validator = ValidateBoundVars::new(vars);
Expand Down
9 changes: 3 additions & 6 deletions compiler/rustc_mir/src/transform/check_consts/validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -822,12 +822,9 @@ impl Visitor<'tcx> for Validator<'mir, 'tcx> {
let obligation = Obligation::new(
ObligationCause::dummy(),
param_env,
Binder::bind(
TraitPredicate {
trait_ref: TraitRef::from_method(tcx, trait_id, substs),
},
tcx,
),
Binder::dummy(TraitPredicate {
trait_ref: TraitRef::from_method(tcx, trait_id, substs),
Copy link
Member

Choose a reason for hiding this comment

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

Preexisting, but why not jsut use the trait_ref create directly above

}),
);

let implsrc = tcx.infer_ctxt().enter(|infcx| {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1301,7 +1301,7 @@ fn confirm_pointee_candidate<'cx, 'tcx>(
ty: self_ty.ptr_metadata_ty(tcx),
};

confirm_param_env_candidate(selcx, obligation, ty::Binder::bind(predicate, tcx), false)
confirm_param_env_candidate(selcx, obligation, ty::Binder::dummy(predicate), false)
}

fn confirm_fn_pointer_candidate<'cx, 'tcx>(
Expand Down
8 changes: 7 additions & 1 deletion compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use rustc_errors::ErrorReported;
use rustc_hir::def_id::{DefId, LocalDefId};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_middle::ty::fold::BoundVarsCollector;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, Instance, TyCtxt, TypeFoldable};
use rustc_span::{sym, DUMMY_SP};
Expand Down Expand Up @@ -115,7 +116,12 @@ fn resolve_associated_item<'tcx>(
);

let trait_ref = ty::TraitRef::from_method(tcx, trait_id, rcvr_substs);
let vtbl = tcx.codegen_fulfill_obligation((param_env, ty::Binder::bind(trait_ref, tcx)))?;
// FIXME: we should instead track bound variables from their origin,
Copy link
Member

Choose a reason for hiding this comment

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

I really don't want someone to start using BoundVarsCollector for something else. As much as it isn't the right place for it, I think it's better to just move that here until this is fixed. (With a very clear comment on why)

// rather than using the `BoundVarsCollector` (cc #83825)
let mut bound_vars_collector = BoundVarsCollector::new();
trait_ref.visit_with(&mut bound_vars_collector);
let trait_binder = ty::Binder::bind_with_vars(trait_ref, bound_vars_collector.into_vars(tcx));
let vtbl = tcx.codegen_fulfill_obligation((param_env, trait_binder))?;

// Now that we know which impl is being used, we can dispatch to
// the actual function:
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/astconv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1694,7 +1694,7 @@ impl<'o, 'tcx> dyn AstConv<'tcx> + 'o {
};

self.one_bound_for_assoc_type(
|| traits::supertraits(tcx, ty::Binder::bind(trait_ref, tcx)),
|| traits::supertraits(tcx, ty::Binder::dummy(trait_ref)),
|| "Self".to_string(),
assoc_ident,
span,
Expand Down
6 changes: 3 additions & 3 deletions compiler/rustc_typeck/src/check/compare_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ fn compare_predicate_entailment<'tcx>(
let (impl_m_own_bounds, _) = infcx.replace_bound_vars_with_fresh_vars(
impl_m_span,
infer::HigherRankedType,
ty::Binder::bind(impl_m_own_bounds.predicates, tcx),
ty::Binder::dummy(impl_m_own_bounds.predicates),
);
Comment on lines 225 to 229
Copy link
Member

Choose a reason for hiding this comment

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

This is weird. If we don't ever have bound vars, why do we have to replace_bound_vars_with_fresh_vars

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I think we can remove replace_bound_vars_with_fresh_vars here because impl_m_predicates is simply the Predicates of the impl, which obviously won't have escaping bound vars. And impl_to_placeholder_substs will only have substs for the impl itself, none of which are late-bound.

for predicate in impl_m_own_bounds {
let traits::Normalized { value: predicate, obligations } =
Expand Down Expand Up @@ -258,14 +258,14 @@ fn compare_predicate_entailment<'tcx>(
);
let impl_sig =
inh.normalize_associated_types_in(impl_m_span, impl_m_hir_id, param_env, impl_sig);
let impl_fty = tcx.mk_fn_ptr(ty::Binder::bind(impl_sig, tcx));
let impl_fty = tcx.mk_fn_ptr(ty::Binder::dummy(impl_sig));
debug!("compare_impl_method: impl_fty={:?}", impl_fty);

let trait_sig = tcx.liberate_late_bound_regions(impl_m.def_id, tcx.fn_sig(trait_m.def_id));
let trait_sig = trait_sig.subst(tcx, trait_to_placeholder_substs);
let trait_sig =
inh.normalize_associated_types_in(impl_m_span, impl_m_hir_id, param_env, trait_sig);
let trait_fty = tcx.mk_fn_ptr(ty::Binder::bind(trait_sig, tcx));
let trait_fty = tcx.mk_fn_ptr(ty::Binder::dummy(trait_sig));

debug!("compare_impl_method: trait_fty={:?}", trait_fty);

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/method/confirm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl<'a, 'tcx> ConfirmContext<'a, 'tcx> {
// We won't add these if we encountered an illegal sized bound, so that we can use
// a custom error in that case.
if illegal_sized_bound.is_none() {
let method_ty = self.tcx.mk_fn_ptr(ty::Binder::bind(method_sig, self.tcx));
let method_ty = self.tcx.mk_fn_ptr(ty::Binder::dummy(method_sig));
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this up?

self.add_obligations(method_ty, all_substs, method_predicates);
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
obligations.extend(traits::predicates_for_generics(cause.clone(), self.param_env, bounds));

// Also add an obligation for the method type being well-formed.
let method_ty = tcx.mk_fn_ptr(ty::Binder::bind(fn_sig, tcx));
let method_ty = tcx.mk_fn_ptr(ty::Binder::dummy(fn_sig));
Copy link
Member

Choose a reason for hiding this comment

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

Can you move this up?

debug!(
"lookup_in_trait_adjusted: matched method method_ty={:?} obligation={:?}",
method_ty, obligation
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_typeck/src/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1087,14 +1087,13 @@ fn check_method_receiver<'fcx, 'tcx>(
debug!("check_method_receiver: sig={:?}", sig);

let self_ty = fcx.normalize_associated_types_in(span, self_ty);
let self_ty =
fcx.tcx.liberate_late_bound_regions(method.def_id, ty::Binder::bind(self_ty, fcx.tcx));
let self_ty = fcx.tcx.liberate_late_bound_regions(method.def_id, ty::Binder::dummy(self_ty));

let receiver_ty = sig.inputs()[0];

let receiver_ty = fcx.normalize_associated_types_in(span, receiver_ty);
let receiver_ty =
fcx.tcx.liberate_late_bound_regions(method.def_id, ty::Binder::bind(receiver_ty, fcx.tcx));
fcx.tcx.liberate_late_bound_regions(method.def_id, ty::Binder::dummy(receiver_ty));
Comment on lines -1090 to +1096
Copy link
Member

Choose a reason for hiding this comment

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

These again are a bit sus: There aren't late-bound vars, but we're calling liberate_late_bound_regions? (mental note to future me: this bit of code is touched in #85499, did we remove these calls?)

Copy link
Member

Choose a reason for hiding this comment

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

Okay yeah, we liberate_late_bound_regions above, we shouldn't need to do that again here.


if fcx.tcx.features().arbitrary_self_types {
if !receiver_is_valid(fcx, span, receiver_ty, self_ty, true) {
Expand Down
15 changes: 9 additions & 6 deletions compiler/rustc_typeck/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1767,7 +1767,7 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> {
}
diag.emit();

ty::Binder::bind(fn_sig, tcx)
ty::Binder::dummy(fn_sig)
Copy link
Member

Choose a reason for hiding this comment

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

Can you just move this up to right under fn_sig is created? Then when used, just skip_binder()

}
None => <dyn AstConv<'_>>::ty_of_fn(
&icx,
Expand Down Expand Up @@ -1811,10 +1811,13 @@ fn fn_sig(tcx: TyCtxt<'_>, def_id: DefId) -> ty::PolyFnSig<'_> {
let ty = tcx.type_of(tcx.hir().get_parent_did(hir_id).to_def_id());
let inputs =
data.fields().iter().map(|f| tcx.type_of(tcx.hir().local_def_id(f.hir_id)));
ty::Binder::bind(
tcx.mk_fn_sig(inputs, ty, false, hir::Unsafety::Normal, abi::Abi::Rust),
tcx,
)
ty::Binder::dummy(tcx.mk_fn_sig(
inputs,
ty,
false,
hir::Unsafety::Normal,
abi::Abi::Rust,
))
}

Expr(&hir::Expr { kind: hir::ExprKind::Closure(..), .. }) => {
Expand Down Expand Up @@ -2098,7 +2101,7 @@ fn gather_explicit_predicates_of(tcx: TyCtxt<'_>, def_id: DefId) -> ty::GenericP
param.bounds.iter().for_each(|bound| match bound {
hir::GenericBound::Outlives(lt) => {
let bound = <dyn AstConv<'_>>::ast_region_to_region(&icx, &lt, None);
let outlives = ty::Binder::bind(ty::OutlivesPredicate(region, bound), tcx);
let outlives = ty::Binder::dummy(ty::OutlivesPredicate(region, bound));
predicates.insert((outlives.to_predicate(tcx), lt.span));
}
_ => bug!(),
Expand Down