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

Make (try_)subst_and_normalize_erasing_regions take EarlyBinder #110297

Merged
merged 4 commits into from
May 8, 2023
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
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_cranelift/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
self.instance.subst_mir_and_normalize_erasing_regions(
self.tcx,
ty::ParamEnv::reveal_all(),
value,
ty::EarlyBinder(value),
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ fn make_mir_scope<'ll, 'tcx>(
let callee = cx.tcx.subst_and_normalize_erasing_regions(
instance.substs,
ty::ParamEnv::reveal_all(),
callee,
ty::EarlyBinder(callee),
);
let callee_fn_abi = cx.fn_abi_of_instance(callee, ty::List::empty());
cx.dbg_scope_fn(callee, callee_fn_abi, None)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,7 @@ impl<'ll, 'tcx> DebugInfoMethods<'tcx> for CodegenCx<'ll, 'tcx> {
let impl_self_ty = cx.tcx.subst_and_normalize_erasing_regions(
instance.substs,
ty::ParamEnv::reveal_all(),
cx.tcx.type_of(impl_def_id).skip_binder(),
cx.tcx.type_of(impl_def_id),
);

// Only "class" methods are generally understood by LLVM,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
self.instance.subst_mir_and_normalize_erasing_regions(
self.cx.tcx(),
ty::ParamEnv::reveal_all(),
value,
ty::EarlyBinder(value),
)
}
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_const_eval/src/interpret/eval_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,11 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> Result<T, InterpError<'tcx>> {
frame
.instance
.try_subst_mir_and_normalize_erasing_regions(*self.tcx, self.param_env, value)
.try_subst_mir_and_normalize_erasing_regions(
*self.tcx,
self.param_env,
ty::EarlyBinder(value),
)
.map_err(|_| err_inval!(TooGeneric))
}

Expand Down
17 changes: 9 additions & 8 deletions compiler/rustc_middle/src/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ impl<'tcx> Instance<'tcx> {
/// lifetimes erased, allowing a `ParamEnv` to be specified for use during normalization.
pub fn ty(&self, tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>) -> Ty<'tcx> {
let ty = tcx.type_of(self.def.def_id());
tcx.subst_and_normalize_erasing_regions(self.substs, param_env, ty.skip_binder())
tcx.subst_and_normalize_erasing_regions(self.substs, param_env, ty)
}

/// Finds a crate that contains a monomorphization of this instance that
Expand Down Expand Up @@ -578,14 +578,15 @@ impl<'tcx> Instance<'tcx> {
self.def.has_polymorphic_mir_body().then_some(self.substs)
}

pub fn subst_mir<T>(&self, tcx: TyCtxt<'tcx>, v: &T) -> T
pub fn subst_mir<T>(&self, tcx: TyCtxt<'tcx>, v: EarlyBinder<&T>) -> T
where
T: TypeFoldable<TyCtxt<'tcx>> + Copy,
{
let v = v.map_bound(|v| *v);
if let Some(substs) = self.substs_for_mir_body() {
EarlyBinder(*v).subst(tcx, substs)
v.subst(tcx, substs)
} else {
*v
v.skip_binder()
}
}

Expand All @@ -594,15 +595,15 @@ impl<'tcx> Instance<'tcx> {
&self,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
v: T,
v: EarlyBinder<T>,
) -> T
where
T: TypeFoldable<TyCtxt<'tcx>> + Clone,
{
if let Some(substs) = self.substs_for_mir_body() {
tcx.subst_and_normalize_erasing_regions(substs, param_env, v)
} else {
tcx.normalize_erasing_regions(param_env, v)
tcx.normalize_erasing_regions(param_env, v.skip_binder())
}
}

Expand All @@ -611,15 +612,15 @@ impl<'tcx> Instance<'tcx> {
&self,
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
v: T,
v: EarlyBinder<T>,
) -> Result<T, NormalizationError<'tcx>>
where
T: TypeFoldable<TyCtxt<'tcx>> + Clone,
{
if let Some(substs) = self.substs_for_mir_body() {
tcx.try_subst_and_normalize_erasing_regions(substs, param_env, v)
} else {
tcx.try_normalize_erasing_regions(param_env, v)
tcx.try_normalize_erasing_regions(param_env, v.skip_binder())
}
}

Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_middle/src/ty/normalize_erasing_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ impl<'tcx> TyCtxt<'tcx> {
self,
param_substs: SubstsRef<'tcx>,
param_env: ty::ParamEnv<'tcx>,
value: T,
value: EarlyBinder<T>,
) -> T
where
T: TypeFoldable<TyCtxt<'tcx>>,
Expand All @@ -151,7 +151,7 @@ impl<'tcx> TyCtxt<'tcx> {
param_env={:?})",
param_substs, value, param_env,
);
let substituted = EarlyBinder(value).subst(self, param_substs);
let substituted = value.subst(self, param_substs);
self.normalize_erasing_regions(param_env, substituted)
}

Expand All @@ -163,7 +163,7 @@ impl<'tcx> TyCtxt<'tcx> {
self,
param_substs: SubstsRef<'tcx>,
param_env: ty::ParamEnv<'tcx>,
value: T,
value: EarlyBinder<T>,
) -> Result<T, NormalizationError<'tcx>>
where
T: TypeFoldable<TyCtxt<'tcx>>,
Expand All @@ -175,7 +175,7 @@ impl<'tcx> TyCtxt<'tcx> {
param_env={:?})",
param_substs, value, param_env,
);
let substituted = EarlyBinder(value).subst(self, param_substs);
let substituted = value.subst(self, param_substs);
self.try_normalize_erasing_regions(param_env, substituted)
}
}
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ impl<'tcx> Inliner<'tcx> {
let Ok(callee_body) = callsite.callee.try_subst_mir_and_normalize_erasing_regions(
self.tcx,
self.param_env,
callee_body.clone(),
ty::EarlyBinder(callee_body.clone()),
) else {
return Err("failed to normalize callee body");
};
Expand Down Expand Up @@ -444,7 +444,9 @@ impl<'tcx> Inliner<'tcx> {
work_list.push(target);

// If the place doesn't actually need dropping, treat it like a regular goto.
let ty = callsite.callee.subst_mir(self.tcx, &place.ty(callee_body, tcx).ty);
let ty = callsite
.callee
.subst_mir(self.tcx, ty::EarlyBinder(&place.ty(callee_body, tcx).ty));
if ty.needs_drop(tcx, self.param_env) && let UnwindAction::Cleanup(unwind) = unwind {
work_list.push(unwind);
}
Expand Down Expand Up @@ -788,7 +790,9 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
match terminator.kind {
TerminatorKind::Drop { ref place, unwind, .. } => {
// If the place doesn't actually need dropping, treat it like a regular goto.
let ty = self.instance.subst_mir(tcx, &place.ty(self.callee_body, tcx).ty);
let ty = self
.instance
.subst_mir(tcx, ty::EarlyBinder(&place.ty(self.callee_body, tcx).ty));
if ty.needs_drop(tcx, self.param_env) {
self.cost += CALL_PENALTY;
if let UnwindAction::Cleanup(_) = unwind {
Expand All @@ -799,7 +803,7 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
}
}
TerminatorKind::Call { func: Operand::Constant(ref f), unwind, .. } => {
let fn_ty = self.instance.subst_mir(tcx, &f.literal.ty());
let fn_ty = self.instance.subst_mir(tcx, ty::EarlyBinder(&f.literal.ty()));
self.cost += if let ty::FnDef(def_id, _) = *fn_ty.kind() && tcx.is_intrinsic(def_id) {
// Don't give intrinsics the extra penalty for calls
INSTR_COST
Expand Down
6 changes: 5 additions & 1 deletion compiler/rustc_mir_transform/src/inline/cycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ pub(crate) fn mir_callgraph_reachable<'tcx>(
) -> bool {
trace!(%caller);
for &(callee, substs) in tcx.mir_inliner_callees(caller.def) {
let Ok(substs) = caller.try_subst_mir_and_normalize_erasing_regions(tcx, param_env, substs) else {
let Ok(substs) = caller.try_subst_mir_and_normalize_erasing_regions(
tcx,
param_env,
ty::EarlyBinder(substs),
) else {
trace!(?caller, ?param_env, ?substs, "cannot normalize, skipping");
continue;
};
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ impl<'a, 'tcx> MirNeighborCollector<'a, 'tcx> {
self.instance.subst_mir_and_normalize_erasing_regions(
self.tcx,
ty::ParamEnv::reveal_all(),
value,
ty::EarlyBinder(value),
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_monomorphize/src/partitioning/default.rs
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ fn characteristic_def_id_of_mono_item<'tcx>(
let impl_self_ty = tcx.subst_and_normalize_erasing_regions(
instance.substs,
ty::ParamEnv::reveal_all(),
tcx.type_of(impl_def_id).skip_binder(),
tcx.type_of(impl_def_id),
);
if let Some(def_id) = characteristic_def_id_of_type(impl_self_ty) {
return Some(def_id);
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_monomorphize/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ pub(crate) fn dump_closure_profile<'tcx>(tcx: TyCtxt<'tcx>, closure_instance: In
let before_feature_tys = tcx.subst_and_normalize_erasing_regions(
closure_instance.substs,
param_env,
before_feature_tys,
ty::EarlyBinder(before_feature_tys),
);
let after_feature_tys = tcx.subst_and_normalize_erasing_regions(
closure_instance.substs,
param_env,
after_feature_tys,
ty::EarlyBinder(after_feature_tys),
);

let new_size = tcx
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_ty_utils/src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ fn resolve_instance<'tcx>(
)
} else {
let ty = tcx.type_of(def);
let item_type =
tcx.subst_and_normalize_erasing_regions(substs, param_env, ty.skip_binder());
let item_type = tcx.subst_and_normalize_erasing_regions(substs, param_env, ty);

let def = match *item_type.kind() {
ty::FnDef(def_id, ..) if tcx.is_intrinsic(def_id) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,9 @@ fn can_change_type<'a>(cx: &LateContext<'a>, mut expr: &'a Expr<'a>, mut ty: Ty<
Node::Expr(parent_expr) => {
if let Some((callee_def_id, call_substs, recv, call_args)) = get_callee_substs_and_args(cx, parent_expr)
{
// FIXME: the `subst_identity()` below seems incorrect, since we eventually
// call `tcx.try_subst_and_normalize_erasing_regions` further down
// (i.e., we are explicitly not in the identity context).
let fn_sig = cx.tcx.fn_sig(callee_def_id).subst_identity().skip_binder();
if let Some(arg_index) = recv.into_iter().chain(call_args).position(|arg| arg.hir_id == expr.hir_id)
&& let Some(param_ty) = fn_sig.inputs().get(arg_index)
Expand Down Expand Up @@ -435,7 +438,7 @@ fn can_change_type<'a>(cx: &LateContext<'a>, mut expr: &'a Expr<'a>, mut ty: Ty<
let output_ty = fn_sig.output();
if output_ty.contains(*param_ty) {
if let Ok(new_ty) = cx.tcx.try_subst_and_normalize_erasing_regions(
new_subst, cx.param_env, output_ty) {
new_subst, cx.param_env, EarlyBinder(output_ty)) {
Copy link
Member

Choose a reason for hiding this comment

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

So, seems like the subst_identity().skip_binder() call to create this isn't correct...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, yeah I agree that this looks weird now. Would it be better as skip_binder().skip_binder()? Or are you envisioning a larger change here?

Copy link
Member

Choose a reason for hiding this comment

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

I have to look a bit more at this. But likely the easiest thing is to either just leave it as-is with a comment, or map/skip_binder above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay thanks! I changed it to skip_binder() above in the most recent commit, but happy to try to improve it further.

Yeah, I also thought about mapping it, it seems like we always skip the inner Binder there but want to keep the EarlyBinder around, so could do something like

let fn_sig = cx.tcx.fn_sig(def_id).map_bound(|t| t.skip_binder());

(and then replace the calls to .inputs() with .skip_binder().inputs())

but I wasn't sure if this was more or less readable than how it is now

Copy link
Member

Choose a reason for hiding this comment

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

I think this PR doesn't necessarily need to fix this, at minimum we should probably open a clippy issue about it and add a FIXME before this PR lands

expr = parent_expr;
ty = new_ty;
continue;
Expand Down