Skip to content

Commit

Permalink
Rollup merge of rust-lang#75337 - davidtwco:polymorphization-75255-fi…
Browse files Browse the repository at this point in the history
…xes, r=eddyb

instance: only polymorphize upvar substs

This PR restricts the substitution polymorphization added in rust-lang#75255 to only apply to the tupled upvar substitution, rather than all substitutions, fixing a bunch of regressions when polymorphization is
enabled.

Due to an oversight on my part, when landing rust-lang#75260 and rust-lang#75255, some tests started failing when polymorphization was enabled that I didn't notice until after landing - this PR fixes the regressions from rust-lang#75255. rust-lang#75336 has been filed to make sure that we don't forget to try make this change again in future, as it does enable some optimisations.

r? @lcnr
  • Loading branch information
Dylan-DPC authored Aug 10, 2020
2 parents 2932638 + fd41bde commit 1e41af3
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 88 deletions.
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).
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.

0 comments on commit 1e41af3

Please sign in to comment.