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.

Sign up for GitHub

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

Ensure no type errors when calling Closure/Generator upvars_ty #78392

Closed
wants to merge 1 commit into from
Closed

Ensure no type errors when calling Closure/Generator upvars_ty #78392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 6 additions & 2 deletions compiler/rustc_middle/src/ty/flags.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,9 @@ impl FlagComputation {
self.add_ty(substs.return_ty());
self.add_ty(substs.witness());
self.add_ty(substs.yield_ty());
self.add_ty(substs.tupled_upvars_ty());
if let Ok(tupled_ty) = substs.tupled_upvars_ty() {
self.add_ty(tupled_ty);
}
}

&ty::GeneratorWitness(ts) => {
Expand All @@ -122,7 +124,9 @@ impl FlagComputation {

self.add_ty(substs.sig_as_fn_ptr_ty());
self.add_ty(substs.kind_ty());
self.add_ty(substs.tupled_upvars_ty());
if let Ok(tupled_ty) = substs.tupled_upvars_ty() {
self.add_ty(tupled_ty);
}
}

&ty::Bound(debruijn, _) => {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/instance.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -506,9 +506,9 @@ fn polymorphize<'tcx>(
// 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())
substs.as_closure().tupled_upvars_ty().ok()
} else if tcx.type_of(def_id).is_generator() {
Some(substs.as_generator().tupled_upvars_ty())
substs.as_generator().tupled_upvars_ty().ok()
} else {
None
};
Expand Down
29 changes: 21 additions & 8 deletions compiler/rustc_middle/src/ty/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,12 +609,23 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
ty::Generator(def_id, substs, _) => self.generator_layout(ty, def_id, substs)?,

ty::Closure(_, ref substs) => {
let tys = substs.as_closure().upvar_tys();
univariant(
&tys.map(|ty| self.layout_of(ty)).collect::<Result<Vec<_>, _>>()?,
&ReprOptions::default(),
StructKind::AlwaysSized,
)?
let substs = substs.as_closure();
if substs.tupled_upvars_ty().is_ok() {
let tys = substs.upvar_tys();
univariant(
&tys.map(|ty| self.layout_of(ty)).collect::<Result<Vec<_>, _>>()?,
&ReprOptions::default(),
StructKind::AlwaysSized,
)?
} else {
univariant(
&std::iter::empty()
.map(|ty| self.layout_of(ty))
.collect::<Result<Vec<_>, _>>()?,
&ReprOptions::default(),
StructKind::AlwaysSized,
)?
}
}

ty::Tuple(tys) => {
Expand Down Expand Up @@ -1396,10 +1407,13 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let info = tcx.generator_layout(def_id);
let (ineligible_locals, assignments) = self.generator_saved_local_eligibility(&info);

let substs = substs.as_generator();

// Build a prefix layout, including "promoting" all ineligible
// locals as part of the prefix. We compute the layout of all of
// these fields at once to get optimal packing.
let tag_index = substs.as_generator().prefix_tys().count();
let tag_index =
if substs.tupled_upvars_ty().is_ok() { substs.prefix_tys().count() } else { 0 };

// `info.variant_fields` already accounts for the reserved variants, so no need to add them.
let max_discr = (info.variant_fields.len() - 1) as u128;
Expand All @@ -1415,7 +1429,6 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
.map(|ty| tcx.mk_maybe_uninit(ty))
.map(|ty| self.layout_of(ty));
let prefix_layouts = substs
.as_generator()
.prefix_tys()
.map(|ty| self.layout_of(ty))
.chain(iter::once(Ok(tag_layout)))
Expand Down
10 changes: 6 additions & 4 deletions compiler/rustc_middle/src/ty/outlives.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,14 +96,16 @@ fn compute_components(
}

ty::Closure(_, ref substs) => {
let tupled_ty = substs.as_closure().tupled_upvars_ty();
compute_components(tcx, tupled_ty, out, visited);
if let Ok(tupled_ty) = substs.as_closure().tupled_upvars_ty() {
compute_components(tcx, tupled_ty, out, visited);
}
}

ty::Generator(_, ref substs, _) => {
// Same as the closure case
let tupled_ty = substs.as_generator().tupled_upvars_ty();
compute_components(tcx, tupled_ty, out, visited);
if let Ok(tupled_ty) = substs.as_generator().tupled_upvars_ty() {
compute_components(tcx, tupled_ty, out, visited);
}

// We ignore regions in the generator interior as we don't
// want these to affect region inference
Expand Down
14 changes: 12 additions & 2 deletions compiler/rustc_middle/src/ty/print/pretty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,11 @@ pub trait PrettyPrinter<'tcx>:
p!(print_def_path(did, substs));
p!(" upvar_tys=(");
if !substs.as_generator().is_valid() {
p!("unavailable");
if substs.as_generator().tupled_upvars_ty().is_err() {
p!("err");
} else {
p!("unavailable");
}
} else {
self = self.comma_sep(substs.as_generator().upvar_tys())?;
}
Expand Down Expand Up @@ -699,7 +703,13 @@ pub trait PrettyPrinter<'tcx>:
} else {
p!(print_def_path(did, substs));
if !substs.as_closure().is_valid() {
p!(" closure_substs=(unavailable)");
p!(" closure_substs=(");
if substs.as_closure().tupled_upvars_ty().is_err() {
p!("err");
} else {
p!("unavailable");
}
p!(")");
} else {
p!(" closure_kind_ty=", print(substs.as_closure().kind_ty()));
p!(
Expand Down
43 changes: 34 additions & 9 deletions compiler/rustc_middle/src/ty/sty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::ty::{DelaySpanBugEmitted, List, ParamEnv, TyS};
use polonius_engine::Atom;
use rustc_ast as ast;
use rustc_data_structures::captures::Captures;
use rustc_errors::ErrorReported;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_index::vec::Idx;
Expand Down Expand Up @@ -388,15 +389,23 @@ impl<'tcx> ClosureSubsts<'tcx> {
self.split().parent_substs
}

/// Returns an iterator that iterates the types of paths captured by a closure.
/// Note it's possible that there was a type error that prevented us from figuring out
/// the types of the upvars captured by the closure.
///
/// This function can be safely called if `self.tupled_upvars_ty().is_ok()` is true.
#[inline]
pub fn upvar_tys(self) -> impl Iterator<Item = Ty<'tcx>> + 'tcx {
self.tupled_upvars_ty().tuple_fields()
self.split().tupled_upvars_ty.expect_ty().tuple_fields()
}

/// Returns the tuple type representing the upvars for this closure.
/// Returns a tuple type containing the types of paths captured by the closure.
/// Returns Err(ErrorReported) if a type error prevented us from figuring out
/// the types of the upvars for this closure.
#[inline]
pub fn tupled_upvars_ty(self) -> Ty<'tcx> {
self.split().tupled_upvars_ty.expect_ty()
pub fn tupled_upvars_ty(self) -> Result<Ty<'tcx>, ErrorReported> {
let tupled_ty = self.split().tupled_upvars_ty.expect_ty();
if let TyKind::Error(_) = tupled_ty.kind() { Err(ErrorReported) } else { Ok(tupled_ty) }
}

/// Returns the closure kind for this closure; may return a type
Expand Down Expand Up @@ -515,15 +524,23 @@ impl<'tcx> GeneratorSubsts<'tcx> {
self.split().witness.expect_ty()
}

/// Returns an iterator that iterates the types of paths captured by a generator.
/// Note it's possible that there was a type error that prevented us from figuring out
/// the types of the upvars captured by the generator.
///
/// This function can be safely called if `self.tupled_upvars_ty().is_ok()` is true.
#[inline]
pub fn upvar_tys(self) -> impl Iterator<Item = Ty<'tcx>> + 'tcx {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I think this function merits a comment too. Something like

Returns an iterator over the upvar types for this closure/generator. This should only be invoked after type check is complete; before that, the upvar types may still be being inferred or may be an inference variable. To access the upvar types before type check is known to have completed, use tupled_upvar_tys.

That said, this will still ICE in the event of a Ty::Err, right? I'm not sure if that's a great idea, as it means that every use of this function has to be guarded by a call to tupled_upvars_ty. Why not make this function return a Result as well (or perhaps an empty vector)?

self.tupled_upvars_ty().tuple_fields()
self.split().tupled_upvars_ty.expect_ty().tuple_fields()
}

/// Returns the tuple type representing the upvars for this generator.
/// Returns a tuple type containing the types of paths captured by the generator.
/// Returns Err(ErrorReported) if a type error prevented us from figuring out
/// the types of the upvars for this generator.
#[inline]
pub fn tupled_upvars_ty(self) -> Ty<'tcx> {
self.split().tupled_upvars_ty.expect_ty()
pub fn tupled_upvars_ty(self) -> Result<Ty<'tcx>, ErrorReported> {
let tupled_ty = self.split().tupled_upvars_ty.expect_ty();
if let TyKind::Error(_) = tupled_ty.kind() { Err(ErrorReported) } else { Ok(tupled_ty) }
}

/// Returns the type representing the resume type of the generator.
Expand Down Expand Up @@ -660,6 +677,11 @@ pub enum UpvarSubsts<'tcx> {
}

impl<'tcx> UpvarSubsts<'tcx> {
/// Returns an iterator that iterates the types of paths captured by a closure/generator.
/// Note it's possible that there was a type error that prevented us from figuring out
/// the types of the upvars captured by the closure/generator.
///
/// This function can be safely called if `self.tupled_upvars_ty().is_ok()` is true.
#[inline]
pub fn upvar_tys(self) -> impl Iterator<Item = Ty<'tcx>> + 'tcx {
let tupled_upvars_ty = match self {
Expand All @@ -669,8 +691,11 @@ impl<'tcx> UpvarSubsts<'tcx> {
tupled_upvars_ty.expect_ty().tuple_fields()
}

/// Returns a tuple type containing the types of paths captured by a closure/generator.
/// Returns Err(ErrorReported) if a type error prevented us from figuring out
/// the types of the upvars for this closure/generator.
#[inline]
pub fn tupled_upvars_ty(self) -> Ty<'tcx> {
pub fn tupled_upvars_ty(self) -> Result<Ty<'tcx>, ErrorReported> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Pre-existing to some extent, but I think this function merits a comment. Something like:

Returns a tuple type whose elements are the types of the upvars captured by this closure/generator. Returns Err(ErrorReported) if a type error prevented us from figuring out the types of the upvars for this closure.

match self {
UpvarSubsts::Closure(substs) => substs.as_closure().tupled_upvars_ty(),
UpvarSubsts::Generator(substs) => substs.as_generator().tupled_upvars_ty(),
Expand Down
14 changes: 12 additions & 2 deletions compiler/rustc_mir/src/util/elaborate_drops.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -850,7 +850,12 @@ where
let ty = self.place_ty(self.place);
match ty.kind() {
ty::Closure(_, substs) => {
let tys: Vec<_> = substs.as_closure().upvar_tys().collect();
let substs = substs.as_closure();
let tys: Vec<_> = if substs.tupled_upvars_ty().is_ok() {
substs.upvar_tys().collect()
} else {
vec![]
};
self.open_drop_for_tuple(&tys)
}
// Note that `elaborate_drops` only drops the upvars of a generator,
Expand All @@ -860,7 +865,12 @@ where
// It effetively only contains upvars until the generator transformation runs.
// See librustc_body/transform/generator.rs for more details.
ty::Generator(_, substs, _) => {
let tys: Vec<_> = substs.as_generator().upvar_tys().collect();
let substs = substs.as_generator();
let tys: Vec<_> = if substs.tupled_upvars_ty().is_ok() {
substs.upvar_tys().collect()
} else {
vec![]
};
self.open_drop_for_tuple(&tys)
}
ty::Tuple(..) => {
Expand Down
16 changes: 10 additions & 6 deletions compiler/rustc_trait_selection/src/opaque_types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -717,10 +717,12 @@ where
ty::Closure(_, ref substs) => {
// Skip lifetime parameters of the enclosing item(s)

substs.as_closure().tupled_upvars_ty().visit_with(self);
if let Ok(tupled_ty) = substs.as_closure().tupled_upvars_ty() {
tupled_ty.visit_with(self);

for upvar_ty in substs.as_closure().upvar_tys() {
upvar_ty.visit_with(self);
for upvar_ty in substs.as_closure().upvar_tys() {
upvar_ty.visit_with(self);
}
}

substs.as_closure().sig_as_fn_ptr_ty().visit_with(self);
Expand All @@ -730,10 +732,12 @@ where
// Skip lifetime parameters of the enclosing item(s)
// Also skip the witness type, because that has no free regions.

substs.as_generator().tupled_upvars_ty().visit_with(self);
if let Ok(tupled_ty) = substs.as_generator().tupled_upvars_ty() {
tupled_ty.visit_with(self);

for upvar_ty in substs.as_generator().upvar_tys() {
upvar_ty.visit_with(self);
for upvar_ty in substs.as_closure().upvar_tys() {
upvar_ty.visit_with(self);
}
}

substs.as_generator().return_ty().visit_with(self);
Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_trait_selection/src/traits/query/dropck_outlives.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,12 @@ pub fn trivial_dropck_outlives<'tcx>(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>) -> bool {
// check if *any* of those are trivial.
ty::Tuple(ref tys) => tys.iter().all(|t| trivial_dropck_outlives(tcx, t.expect_ty())),
ty::Closure(_, ref substs) => {
trivial_dropck_outlives(tcx, substs.as_closure().tupled_upvars_ty())
if let Ok(tupled_tys) = substs.as_closure().tupled_upvars_ty() {
trivial_dropck_outlives(tcx, tupled_tys)
} else {
// Same as the error case above
true
}
}

ty::Adt(def, _) => {
Expand Down
34 changes: 25 additions & 9 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1601,12 +1601,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

ty::Closure(_, substs) => {
// (*) binder moved here
let ty = self.infcx.shallow_resolve(substs.as_closure().tupled_upvars_ty());
if let ty::Infer(ty::TyVar(_)) = ty.kind() {
// Not yet resolved.
Ambiguous

if let Ok(tupled_tys) = substs.as_closure().tupled_upvars_ty() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: it feels like we may want a helper for this pattern of shallow resolving the tupled_upvars_ty, but maybe it doesn't occur that often actually.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't see too much benefit here, especially since each case Tuple/Infer/Error will need to be handled at each call site. Also, we only do this in 3 spots.

let ty = self.infcx.shallow_resolve(tupled_tys);
if let ty::Infer(ty::TyVar(_)) = ty.kind() {
// Not yet resolved.
Ambiguous
} else {
Where(
obligation.predicate.rebind(substs.as_closure().upvar_tys().collect()),
)
}
} else {
Where(obligation.predicate.rebind(substs.as_closure().upvar_tys().collect()))
Where(ty::Binder::dummy(Vec::new()))
}
}

Expand Down Expand Up @@ -1677,14 +1684,23 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
}

ty::Closure(_, ref substs) => {
let ty = self.infcx.shallow_resolve(substs.as_closure().tupled_upvars_ty());
vec![ty]
if let Ok(tupled_tys) = substs.as_closure().tupled_upvars_ty() {
let ty = self.infcx.shallow_resolve(tupled_tys);
vec![ty]
} else {
vec![]
}
}

ty::Generator(_, ref substs, _) => {
let ty = self.infcx.shallow_resolve(substs.as_generator().tupled_upvars_ty());
let tys_vec = if let Ok(tupled_tys) = substs.as_closure().tupled_upvars_ty() {
let ty = self.infcx.shallow_resolve(tupled_tys);
vec![ty]
} else {
vec![]
};
let witness = substs.as_generator().witness();
vec![ty].into_iter().chain(iter::once(witness)).collect()
tys_vec.into_iter().chain(iter::once(witness)).collect()
}

ty::GeneratorWitness(types) => {
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_trait_selection/src/traits/wf.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,9 @@ impl<'a, 'tcx> WfPredicates<'a, 'tcx> {
// only inspects the upvar types).
walker.skip_current_subtree(); // subtree handled below
// FIXME(eddyb) add the type to `walker` instead of recursing.
self.compute(substs.as_closure().tupled_upvars_ty().into());
if let Ok(tupled_tys) = substs.as_closure().tupled_upvars_ty() {
self.compute(tupled_tys.into());
}
}

ty::FnPtr(_) => {
Expand Down
13 changes: 9 additions & 4 deletions compiler/rustc_ty/src/needs_drop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,15 +94,20 @@ where
_ if component.is_copy_modulo_regions(tcx.at(DUMMY_SP), self.param_env) => (),

ty::Closure(_, substs) => {
for upvar_ty in substs.as_closure().upvar_tys() {
queue_type(self, upvar_ty);
let substs = substs.as_closure();
if substs.tupled_upvars_ty().is_ok() {
for upvar_ty in substs.upvar_tys() {
queue_type(self, upvar_ty);
}
}
}

ty::Generator(def_id, substs, _) => {
let substs = substs.as_generator();
for upvar_ty in substs.upvar_tys() {
queue_type(self, upvar_ty);
if substs.tupled_upvars_ty().is_ok() {
for upvar_ty in substs.upvar_tys() {
queue_type(self, upvar_ty);
}
}

let witness = substs.witness();
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_typeck/src/check/upvar.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Build a tuple (U0..Un) of the final upvar types U0..Un
// and unify the upvar tupe type in the closure with it:
let final_tupled_upvars_type = self.tcx.mk_tup(final_upvar_tys.iter());
self.demand_suptype(span, substs.tupled_upvars_ty(), final_tupled_upvars_type);
if let Ok(tupled_upvars) = substs.tupled_upvars_ty() {
self.demand_suptype(span, tupled_upvars, final_tupled_upvars_type);
}

// If we are also inferred the closure kind here,
// process any deferred resolutions.
Expand Down
Loading