From fd41bdeff0ff56ad2e46b00ca55daafb68a8ea08 Mon Sep 17 00:00:00 2001 From: David Wood Date: Sun, 9 Aug 2020 14:53:33 +0100 Subject: [PATCH] instance: only polymorphize upvar substs This commit restricts the substitution polymorphization added in #75255 to only apply to the tupled upvar substitution, rather than all substitutions, fixing a bunch of regressions when polymorphization is enabled. Signed-off-by: David Wood --- src/librustc_middle/ty/flags.rs | 6 --- src/librustc_middle/ty/fold.rs | 6 --- src/librustc_middle/ty/instance.rs | 53 ++++++++++++------- src/librustc_middle/ty/mod.rs | 4 -- .../polymorphization/pr-75255.rs | 52 ------------------ 5 files changed, 33 insertions(+), 88 deletions(-) delete mode 100644 src/test/codegen-units/polymorphization/pr-75255.rs diff --git a/src/librustc_middle/ty/flags.rs b/src/librustc_middle/ty/flags.rs index 81f7474962c8d..27f50c240db67 100644 --- a/src/librustc_middle/ty/flags.rs +++ b/src/librustc_middle/ty/flags.rs @@ -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); @@ -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); @@ -196,8 +192,6 @@ impl FlagComputation { } &ty::FnDef(_, substs) => { - self.add_flags(TypeFlags::MAY_POLYMORPHIZE); - self.add_substs(substs); } diff --git a/src/librustc_middle/ty/fold.rs b/src/librustc_middle/ty/fold.rs index 87434f3f26777..492f8ce9ef1a9 100644 --- a/src/librustc_middle/ty/fold.rs +++ b/src/librustc_middle/ty/fold.rs @@ -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); diff --git a/src/librustc_middle/ty/instance.rs b/src/librustc_middle/ty/instance.rs index cf876db26bc76..2def000da6480 100644 --- a/src/librustc_middle/ty/instance.rs +++ b/src/librustc_middle/ty/instance.rs @@ -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>, }; @@ -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 { @@ -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], } }) } diff --git a/src/librustc_middle/ty/mod.rs b/src/librustc_middle/ty/mod.rs index 0102225b9b567..6798addb8aaa3 100644 --- a/src/librustc_middle/ty/mod.rs +++ b/src/librustc_middle/ty/mod.rs @@ -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; } } diff --git a/src/test/codegen-units/polymorphization/pr-75255.rs b/src/test/codegen-units/polymorphization/pr-75255.rs deleted file mode 100644 index af47b440640af..0000000000000 --- a/src/test/codegen-units/polymorphization/pr-75255.rs +++ /dev/null @@ -1,52 +0,0 @@ -// compile-flags:-Zpolymorphize=on -Zprint-mono-items=lazy -Copt-level=1 -// ignore-tidy-linelength - -#![crate_type = "rlib"] - -// Test that only one copy of `Iter::map` and `iter::repeat` are generated. - -fn unused() -> u64 { - 42 -} - -fn foo() { - let x = [1, 2, 3, std::mem::size_of::()]; - x.iter().map(|_| ()); -} - -//~ MONO_ITEM fn core::iter[0]::adapters[0]::{{impl}}[29]::new[0], pr_75255::foo[0]::{{closure}}[0]> @@ pr_75255-cgu.0[External] -//~ MONO_ITEM fn core::iter[0]::traits[0]::iterator[0]::Iterator[0]::map[0], (), pr_75255::foo[0]::{{closure}}[0]> @@ pr_75255-cgu.1[Internal] - -fn bar() { - std::iter::repeat(unused::); -} - -//~ MONO_ITEM fn core::iter[0]::sources[0]::repeat[0] u64> @@ pr_75255-cgu.1[Internal] - -pub fn dispatch() { - foo::(); - foo::>(); - - bar::(); - bar::>(); -} - -// These are all the items that aren't relevant to the test. -//~ MONO_ITEM fn core::mem[0]::size_of[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::mem[0]::size_of[0]> @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::mem[0]::size_of[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::add[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::is_null[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::offset[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::wrapping_add[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::const_ptr[0]::{{impl}}[0]::wrapping_offset[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::non_null[0]::{{impl}}[3]::new_unchecked[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::ptr[0]::null[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::slice[0]::{{impl}}[0]::as_ptr[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::slice[0]::{{impl}}[0]::iter[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn core::slice[0]::{{impl}}[0]::len[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn pr_75255::dispatch[0] @@ pr_75255-cgu.1[External] -//~ MONO_ITEM fn pr_75255::foo[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn pr_75255::foo[0]> @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn pr_75255::bar[0] @@ pr_75255-cgu.1[Internal] -//~ MONO_ITEM fn pr_75255::bar[0]> @@ pr_75255-cgu.1[Internal]