Skip to content

Commit

Permalink
[HACK] rustc: tuple all closure/generator synthetics, not just upvars.
Browse files Browse the repository at this point in the history
  • Loading branch information
eddyb committed Mar 15, 2020
1 parent cb6cdc0 commit a43fead
Show file tree
Hide file tree
Showing 29 changed files with 157 additions and 183 deletions.
81 changes: 37 additions & 44 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::mir::interpret::ConstValue;
use crate::mir::interpret::Scalar;
use crate::mir::Promoted;
use crate::ty::layout::VariantIdx;
use crate::ty::subst::{GenericArgKind, InternalSubsts, Subst, SubstsRef};
use crate::ty::subst::{GenericArg, GenericArgKind, InternalSubsts, Subst, SubstsRef};
use crate::ty::{
self, AdtDef, DefIdTree, Discr, Ty, TyCtxt, TypeFlags, TypeFoldable, WithConstness,
};
Expand Down Expand Up @@ -276,6 +276,7 @@ static_assert_size!(TyKind<'_>, 24);
/// - U is a type parameter representing the types of its upvars, tupled up
/// (borrowed, if appropriate; that is, if an U field represents a by-ref upvar,
/// and the up-var has the type `Foo`, then that field of U will be `&Foo`).
/// FIXME(eddyb) update this with the new setup which tuples all synthetics.
///
/// So, for example, given this function:
///
Expand Down Expand Up @@ -364,21 +365,26 @@ pub struct ClosureSubsts<'tcx> {
/// Struct returned by `split()`. Note that these are subslices of the
/// parent slice and not canonical substs themselves.
struct SplitClosureSubsts<'tcx> {
// FIXME(eddyb) maybe replace these with `GenericArg` to avoid having
// `GenericArg::expect_ty` called on all of them when only one is used.
closure_kind_ty: Ty<'tcx>,
closure_sig_ty: Ty<'tcx>,
tupled_upvars_ty: Ty<'tcx>,
upvars: &'tcx [GenericArg<'tcx>],
}

impl<'tcx> ClosureSubsts<'tcx> {
/// Divides the closure substs into their respective
/// components. Single source of truth with respect to the
/// ordering.
fn split(self) -> SplitClosureSubsts<'tcx> {
let parent_len = self.substs.len() - 3;
let synthetics = match self.substs[self.substs.len() - 1].expect_ty().kind {
Tuple(synthetics) => synthetics,
_ => bug!("synthetics should be tupled"),
};
SplitClosureSubsts {
closure_kind_ty: self.substs.type_at(parent_len),
closure_sig_ty: self.substs.type_at(parent_len + 1),
tupled_upvars_ty: self.substs.type_at(parent_len + 2),
closure_kind_ty: synthetics.type_at(0),
closure_sig_ty: synthetics.type_at(1),
upvars: &synthetics[2..],
}
}

Expand All @@ -388,22 +394,15 @@ impl<'tcx> ClosureSubsts<'tcx> {
/// Used primarily by `ty::print::pretty` to be able to handle closure
/// types that haven't had their synthetic types substituted in.
pub fn is_valid(self) -> bool {
self.substs.len() >= 3 && matches!(self.split().tupled_upvars_ty.kind, Tuple(_))
match self.substs[self.substs.len() - 1].expect_ty().kind {
Tuple(synthetics) => synthetics.len() >= 2,
_ => false,
}
}

#[inline]
pub fn upvar_tys(self) -> impl Iterator<Item = Ty<'tcx>> + 'tcx {
let upvars = match self.split().tupled_upvars_ty.kind {
Tuple(upvars) => upvars,
_ => bug!("upvars should be tupled"),
};
upvars.iter().map(|t| {
if let GenericArgKind::Type(ty) = t.unpack() {
ty
} else {
bug!("upvar should be type")
}
})
self.split().upvars.iter().map(|t| t.expect_ty())
}

/// Returns the closure kind for this closure; may return a type
Expand Down Expand Up @@ -454,22 +453,27 @@ pub struct GeneratorSubsts<'tcx> {
}

struct SplitGeneratorSubsts<'tcx> {
// FIXME(eddyb) maybe replace these with `GenericArg` to avoid having
// `GenericArg::expect_ty` called on all of them when only one is used.
resume_ty: Ty<'tcx>,
yield_ty: Ty<'tcx>,
return_ty: Ty<'tcx>,
witness: Ty<'tcx>,
tupled_upvars_ty: Ty<'tcx>,
upvars: &'tcx [GenericArg<'tcx>],
}

impl<'tcx> GeneratorSubsts<'tcx> {
fn split(self) -> SplitGeneratorSubsts<'tcx> {
let parent_len = self.substs.len() - 5;
let synthetics = match self.substs[self.substs.len() - 1].expect_ty().kind {
Tuple(synthetics) => synthetics,
_ => bug!("synthetics should be tupled"),
};
SplitGeneratorSubsts {
resume_ty: self.substs.type_at(parent_len),
yield_ty: self.substs.type_at(parent_len + 1),
return_ty: self.substs.type_at(parent_len + 2),
witness: self.substs.type_at(parent_len + 3),
tupled_upvars_ty: self.substs.type_at(parent_len + 4),
resume_ty: synthetics.type_at(0),
yield_ty: synthetics.type_at(1),
return_ty: synthetics.type_at(2),
witness: synthetics.type_at(3),
upvars: &synthetics[4..],
}
}

Expand All @@ -479,7 +483,10 @@ impl<'tcx> GeneratorSubsts<'tcx> {
/// Used primarily by `ty::print::pretty` to be able to handle generator
/// types that haven't had their synthetic types substituted in.
pub fn is_valid(self) -> bool {
self.substs.len() >= 5 && matches!(self.split().tupled_upvars_ty.kind, Tuple(_))
match self.substs[self.substs.len() - 1].expect_ty().kind {
Tuple(synthetics) => synthetics.len() >= 4,
_ => false,
}
}

/// This describes the types that can be contained in a generator.
Expand All @@ -493,17 +500,7 @@ impl<'tcx> GeneratorSubsts<'tcx> {

#[inline]
pub fn upvar_tys(self) -> impl Iterator<Item = Ty<'tcx>> + 'tcx {
let upvars = match self.split().tupled_upvars_ty.kind {
Tuple(upvars) => upvars,
_ => bug!("upvars should be tupled"),
};
upvars.iter().map(|t| {
if let GenericArgKind::Type(ty) = t.unpack() {
ty
} else {
bug!("upvar should be type")
}
})
self.split().upvars.iter().map(|t| t.expect_ty())
}

/// Returns the type representing the resume type of the generator.
Expand Down Expand Up @@ -643,13 +640,9 @@ pub enum UpvarSubsts<'tcx> {
impl<'tcx> UpvarSubsts<'tcx> {
#[inline]
pub fn upvar_tys(self) -> impl Iterator<Item = Ty<'tcx>> + 'tcx {
let tupled_upvars_ty = match self {
UpvarSubsts::Closure(substs) => substs.as_closure().split().tupled_upvars_ty,
UpvarSubsts::Generator(substs) => substs.as_generator().split().tupled_upvars_ty,
};
let upvars = match tupled_upvars_ty.kind {
Tuple(upvars) => upvars,
_ => bug!("upvars should be tupled"),
let upvars = match self {
UpvarSubsts::Closure(substs) => substs.as_closure().split().upvars,
UpvarSubsts::Generator(substs) => substs.as_generator().split().upvars,
};
upvars.iter().map(|t| {
if let GenericArgKind::Type(ty) = t.unpack() {
Expand Down
14 changes: 10 additions & 4 deletions src/librustc_mir/borrow_check/universal_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,9 +329,12 @@ impl<'tcx> UniversalRegions<'tcx> {
match self.defining_ty {
DefiningTy::Closure(def_id, substs) => {
err.note(&format!(
"defining type: {} with closure substs {:#?}",
"defining type: {} with closure synthetics {:#?}",
tcx.def_path_str_with_substs(def_id, substs),
&substs[tcx.generics_of(def_id).parent_count..],
substs[tcx.generics_of(def_id).parent_count]
.expect_ty()
.tuple_fields()
.collect::<Vec<_>>(),
));

// FIXME: It'd be nice to print the late-bound regions
Expand All @@ -346,9 +349,12 @@ impl<'tcx> UniversalRegions<'tcx> {
}
DefiningTy::Generator(def_id, substs, _) => {
err.note(&format!(
"defining type: {} with generator substs {:#?}",
"defining type: {} with generator synthetics {:#?}",
tcx.def_path_str_with_substs(def_id, substs),
&substs[tcx.generics_of(def_id).parent_count..],
substs[tcx.generics_of(def_id).parent_count]
.expect_ty()
.tuple_fields()
.collect::<Vec<_>>(),
));

// FIXME: As above, we'd like to print out the region
Expand Down
47 changes: 25 additions & 22 deletions src/librustc_typeck/check/closure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::astconv::AstConv;
use crate::middle::{lang_items, region};
use rustc::ty::fold::TypeFoldable;
use rustc::ty::subst::InternalSubsts;
use rustc::ty::{self, GenericParamDefKind, Ty};
use rustc::ty::{self, Ty};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_infer::infer::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};
Expand Down Expand Up @@ -82,32 +82,35 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// inference phase (`upvar.rs`).
let base_substs =
InternalSubsts::identity_for_item(self.tcx, self.tcx.closure_base_def_id(expr_def_id));
// HACK(eddyb) this hardcodes indices into substs but it should rely on
// HACK(eddyb) this hardcodes the number of synthetics but it should rely on
// `ClosureSubsts` and `GeneratorSubsts` providing constructors, instead.
// That would also remove the need for most of the inference variables,
// as they immediately unified with the actual type below, including
// the `InferCtxt::closure_sig` and `ClosureSubsts::sig_ty` methods.
let tupled_upvars_idx = base_substs.len() + if generator_types.is_some() { 4 } else { 2 };
let substs = base_substs.extend_to(self.tcx, expr_def_id, |param, _| match param.kind {
GenericParamDefKind::Lifetime => span_bug!(expr.span, "closure has lifetime param"),
GenericParamDefKind::Type { .. } => if param.index as usize == tupled_upvars_idx {
self.tcx.mk_tup(self.tcx.upvars(expr_def_id).iter().flat_map(|upvars| {
upvars.iter().map(|(&var_hir_id, _)| {
self.infcx.next_ty_var(TypeVariableOrigin {
// FIXME(eddyb) distinguish upvar inference variables from the rest.
kind: TypeVariableOriginKind::ClosureSynthetic,
span: self.tcx.hir().span(var_hir_id),
let non_upvar_synthetics = if generator_types.is_some() { 4 } else { 2 };
let substs = base_substs.extend_to(self.tcx, expr_def_id, |param, _| {
assert_eq!(param.index as usize, base_substs.len());

self.tcx
.mk_tup(
(0..non_upvar_synthetics)
.map(|_| {
self.infcx.next_ty_var(TypeVariableOrigin {
kind: TypeVariableOriginKind::ClosureSynthetic,
span: expr.span,
})
})
})
}))
} else {
self.infcx.next_ty_var(TypeVariableOrigin {
kind: TypeVariableOriginKind::ClosureSynthetic,
span: expr.span,
})
}
.into(),
GenericParamDefKind::Const => span_bug!(expr.span, "closure has const param"),
.chain(self.tcx.upvars(expr_def_id).iter().flat_map(|upvars| {
upvars.iter().map(|(&var_hir_id, _)| {
self.infcx.next_ty_var(TypeVariableOrigin {
// FIXME(eddyb) distinguish upvar inference variables from the rest.
kind: TypeVariableOriginKind::ClosureSynthetic,
span: self.tcx.hir().span(var_hir_id),
})
})
})),
)
.into()
});
if let Some(GeneratorTypes { resume_ty, yield_ty, interior, movability }) = generator_types
{
Expand Down
18 changes: 6 additions & 12 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1334,27 +1334,21 @@ fn generics_of(tcx: TyCtxt<'_>, def_id: DefId) -> &ty::Generics {
Some(param_def)
}));

// provide junk type parameter defs - the only place that
// cares about anything but the length is instantiation,
// and we don't do that for closures.
// Add a type parameter to hold the tupled closure/generator synthetics.
if let Node::Expr(&hir::Expr { kind: hir::ExprKind::Closure(.., gen), .. }) = node {
let dummy_args = if gen.is_some() {
&["<resume_ty>", "<yield_ty>", "<return_ty>", "<witness>", "<upvars>"][..]
} else {
&["<closure_kind>", "<closure_signature>", "<upvars>"][..]
};
let name = if gen.is_some() { "<generator_synthetics>" } else { "<closure_synthetics>" };

params.extend(dummy_args.iter().enumerate().map(|(i, &arg)| ty::GenericParamDef {
index: type_start + i as u32,
name: Symbol::intern(arg),
params.push(ty::GenericParamDef {
index: type_start,
name: Symbol::intern(name),
def_id,
pure_wrt_drop: false,
kind: ty::GenericParamDefKind::Type {
has_default: false,
object_lifetime_default: rl::Set1::Empty,
synthetic: None,
},
}));
});
}

let param_def_id_to_index = params.iter().map(|param| (param.def_id, param.index)).collect();
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/generator/sized-yield.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,6 @@ fn main() {
};
Pin::new(&mut gen).resume(());
//~^ ERROR the size for values of type
//~| ERROR the size for values of type
//~| ERROR the size for values of type
}
22 changes: 21 additions & 1 deletion src/test/ui/generator/sized-yield.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,26 @@ LL | | };
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
= note: the yield type of a generator must have a statically known size

error[E0277]: the size for values of type `str` cannot be known at compilation time
--> $DIR/sized-yield.rs:12:13
|
LL | Pin::new(&mut gen).resume(());
| ^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `str`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
= note: only the last element of a tuple may have a dynamically sized type

error[E0277]: the size for values of type `str` cannot be known at compilation time
--> $DIR/sized-yield.rs:12:4
|
LL | Pin::new(&mut gen).resume(());
| ^^^^^^^^^^^^^^^^^^ doesn't have a size known at compile-time
|
= help: the trait `std::marker::Sized` is not implemented for `str`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>
= note: only the last element of a tuple may have a dynamically sized type

error[E0277]: the size for values of type `str` cannot be known at compilation time
--> $DIR/sized-yield.rs:12:23
|
Expand All @@ -21,6 +41,6 @@ LL | Pin::new(&mut gen).resume(());
= help: the trait `std::marker::Sized` is not implemented for `str`
= note: to learn more, visit <https://doc.rust-lang.org/book/ch19-04-advanced-types.html#dynamically-sized-types-and-the-sized-trait>

error: aborting due to 2 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0277`.
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ note: no external requirements
LL | let mut closure = expect_sig(|p, y| *p = y);
| ^^^^^^^^^^^^^
|
= note: defining type: test::{{closure}}#0 with closure substs [
= note: defining type: test::{{closure}}#0 with closure synthetics [
i16,
for<'r, 's, 't0> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed('r)) mut &ReLateBound(DebruijnIndex(0), BrNamed('s)) i32, &ReLateBound(DebruijnIndex(0), BrNamed('t0)) i32)),
(),
]

error: lifetime may not live long enough
Expand Down
3 changes: 1 addition & 2 deletions src/test/ui/nll/closure-requirements/escape-argument.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ note: no external requirements
LL | let mut closure = expect_sig(|p, y| *p = y);
| ^^^^^^^^^^^^^
|
= note: defining type: test::{{closure}}#0 with closure substs [
= note: defining type: test::{{closure}}#0 with closure synthetics [
i16,
for<'r, 's> extern "rust-call" fn((&ReLateBound(DebruijnIndex(0), BrNamed('r)) mut &ReLateBound(DebruijnIndex(0), BrNamed('s)) i32, &ReLateBound(DebruijnIndex(0), BrNamed('s)) i32)),
(),
]

note: no external requirements
Expand Down
10 changes: 6 additions & 4 deletions src/test/ui/nll/closure-requirements/escape-upvar-nested.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ note: external requirements
LL | let mut closure1 = || p = &y;
| ^^^^^^^^^
|
= note: defining type: test::{{closure}}#0::{{closure}}#0 with closure substs [
= note: defining type: test::{{closure}}#0::{{closure}}#0 with closure synthetics [
i16,
extern "rust-call" fn(()),
(&'_#1r i32, &'_#2r mut &'_#3r i32),
&'_#1r i32,
&'_#2r mut &'_#3r i32,
]
= note: number of external vids: 4
= note: where '_#1r: '_#3r
Expand All @@ -22,10 +23,11 @@ LL | | closure1();
LL | | };
| |_________^
|
= note: defining type: test::{{closure}}#0 with closure substs [
= note: defining type: test::{{closure}}#0 with closure synthetics [
i16,
extern "rust-call" fn(()),
(&'_#1r i32, &'_#2r mut &'_#3r i32),
&'_#1r i32,
&'_#2r mut &'_#3r i32,
]
= note: number of external vids: 4
= note: where '_#1r: '_#3r
Expand Down
5 changes: 3 additions & 2 deletions src/test/ui/nll/closure-requirements/escape-upvar-ref.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,11 @@ note: external requirements
LL | let mut closure = || p = &y;
| ^^^^^^^^^
|
= note: defining type: test::{{closure}}#0 with closure substs [
= note: defining type: test::{{closure}}#0 with closure synthetics [
i16,
extern "rust-call" fn(()),
(&'_#1r i32, &'_#2r mut &'_#3r i32),
&'_#1r i32,
&'_#2r mut &'_#3r i32,
]
= note: number of external vids: 4
= note: where '_#1r: '_#3r
Expand Down
Loading

0 comments on commit a43fead

Please sign in to comment.