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

instance: only polymorphize upvar substs #75337

Merged
merged 1 commit into from
Aug 11, 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
6 changes: 0 additions & 6 deletions src/librustc_middle/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ impl FlagComputation {
}

&ty::Generator(_, ref substs, _) => {
self.add_flags(TypeFlags::MAY_POLYMORPHIZE);

let substs = substs.as_generator();
let should_remove_further_specializable =
!self.flags.contains(TypeFlags::STILL_FURTHER_SPECIALIZABLE);
Expand All @@ -109,8 +107,6 @@ impl FlagComputation {
}

&ty::Closure(_, substs) => {
self.add_flags(TypeFlags::MAY_POLYMORPHIZE);

let substs = substs.as_closure();
let should_remove_further_specializable =
!self.flags.contains(TypeFlags::STILL_FURTHER_SPECIALIZABLE);
Expand Down Expand Up @@ -196,8 +192,6 @@ impl FlagComputation {
}

&ty::FnDef(_, substs) => {
self.add_flags(TypeFlags::MAY_POLYMORPHIZE);

self.add_substs(substs);
}

Expand Down
6 changes: 0 additions & 6 deletions src/librustc_middle/ty/fold.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,12 +150,6 @@ pub trait TypeFoldable<'tcx>: fmt::Debug + Clone {
self.has_type_flags(TypeFlags::STILL_FURTHER_SPECIALIZABLE)
}

/// Does this value contain closures, generators or functions such that it may require
/// polymorphization?
fn may_polymorphize(&self) -> bool {
self.has_type_flags(TypeFlags::MAY_POLYMORPHIZE)
}

/// A visitor that does not recurse into types, works like `fn walk_shallow` in `Ty`.
fn visit_tys_shallow(&self, visit: impl FnMut(Ty<'tcx>) -> bool) -> bool {
pub struct Visitor<F>(F);
Expand Down
53 changes: 33 additions & 20 deletions src/librustc_middle/ty/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,20 @@ fn polymorphize<'tcx>(
let unused = tcx.unused_generic_params(def_id);
debug!("polymorphize: unused={:?}", unused);

// If this is a closure or generator then we need to handle the case where another closure
// from the function is captured as an upvar and hasn't been polymorphized. In this case,
// the unpolymorphized upvar closure would result in a polymorphized closure producing
// multiple mono items (and eventually symbol clashes).
Comment on lines +495 to +498
Copy link
Member

Choose a reason for hiding this comment

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

Wondering if we should move back to not tracking that information in the ty::Closure at all. cc @nikomatsakis

let upvars_ty = if tcx.is_closure(def_id) {
Some(substs.as_closure().tupled_upvars_ty())
} else if tcx.type_of(def_id).is_generator() {
Some(substs.as_generator().tupled_upvars_ty())
} else {
None
};
let has_upvars = upvars_ty.map(|ty| ty.tuple_fields().count() > 0).unwrap_or(false);
debug!("polymorphize: upvars_ty={:?} has_upvars={:?}", upvars_ty, has_upvars);

struct PolymorphizationFolder<'tcx> {
tcx: TyCtxt<'tcx>,
};
Expand All @@ -512,14 +526,6 @@ fn polymorphize<'tcx>(
self.tcx.mk_closure(def_id, polymorphized_substs)
}
}
ty::FnDef(def_id, substs) => {
let polymorphized_substs = polymorphize(self.tcx, def_id, substs);
if substs == polymorphized_substs {
ty
} else {
self.tcx.mk_fn_def(def_id, polymorphized_substs)
}
}
ty::Generator(def_id, substs, movability) => {
let polymorphized_substs = polymorphize(self.tcx, def_id, substs);
if substs == polymorphized_substs {
Expand All @@ -537,24 +543,31 @@ fn polymorphize<'tcx>(
let is_unused = unused.contains(param.index).unwrap_or(false);
debug!("polymorphize: param={:?} is_unused={:?}", param, is_unused);
match param.kind {
// If parameter is a const or type parameter..
// Upvar case: If parameter is a type parameter..
ty::GenericParamDefKind::Type { .. } if
// ..and has upvars..
has_upvars &&
// ..and this param has the same type as the tupled upvars..
upvars_ty == Some(substs[param.index as usize].expect_ty()) => {
// ..then double-check that polymorphization marked it used..
debug_assert!(!is_unused);
// ..and polymorphize any closures/generators captured as upvars.
let upvars_ty = upvars_ty.unwrap();
let polymorphized_upvars_ty = upvars_ty.fold_with(
&mut PolymorphizationFolder { tcx });
debug!("polymorphize: polymorphized_upvars_ty={:?}", polymorphized_upvars_ty);
ty::GenericArg::from(polymorphized_upvars_ty)
},

// Simple case: If parameter is a const or type parameter..
ty::GenericParamDefKind::Const | ty::GenericParamDefKind::Type { .. } if
// ..and is within range and unused..
unused.contains(param.index).unwrap_or(false) =>
// ..then use the identity for this parameter.
tcx.mk_param_from_def(param),

// If the parameter does not contain any closures or generators, then use the
// substitution directly.
_ if !substs.may_polymorphize() => substs[param.index as usize],

// Otherwise, use the substitution after polymorphizing.
_ => {
let arg = substs[param.index as usize];
let polymorphized_arg = arg.fold_with(&mut PolymorphizationFolder { tcx });
debug!("polymorphize: arg={:?} polymorphized_arg={:?}", arg, polymorphized_arg);
ty::GenericArg::from(polymorphized_arg)
}
// Otherwise, use the parameter as before.
_ => substs[param.index as usize],
}
})
}
Expand Down
4 changes: 0 additions & 4 deletions src/librustc_middle/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,10 +575,6 @@ bitflags! {
/// Does this value have parameters/placeholders/inference variables which could be
/// replaced later, in a way that would change the results of `impl` specialization?
const STILL_FURTHER_SPECIALIZABLE = 1 << 17;

/// Does this value contain closures, generators or functions such that it may require
/// polymorphization?
const MAY_POLYMORPHIZE = 1 << 18;
}
}

Expand Down
52 changes: 0 additions & 52 deletions src/test/codegen-units/polymorphization/pr-75255.rs

This file was deleted.