Skip to content

Commit 64a6fde

Browse files
committed
Constrain impl Trait regions in regionck
1 parent ccceab9 commit 64a6fde

File tree

3 files changed

+187
-10
lines changed

3 files changed

+187
-10
lines changed

src/librustc_typeck/check/mod.rs

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,7 @@ pub struct Inherited<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
212212
// associated fresh inference variable. Writeback resolves these
213213
// variables to get the concrete type, which can be used to
214214
// deanonymize TyAnon, after typeck is done with all functions.
215-
anon_types: RefCell<DefIdMap<(&'tcx Substs<'tcx>, Ty<'tcx>)>>,
215+
anon_types: RefCell<DefIdMap<AnonTypeDecl<'tcx>>>,
216216

217217
/// Each type parameter has an implicit region bound that
218218
/// indicates it must outlive at least the function body (the user
@@ -225,6 +225,38 @@ pub struct Inherited<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
225225
body_id: Option<hir::BodyId>,
226226
}
227227

228+
/// Information about the anonymous, abstract types whose values we
229+
/// are inferring in this function (these are the `impl Trait` that
230+
/// appear in the return type).
231+
struct AnonTypeDecl<'tcx> {
232+
/// The substitutions that we apply to the abstract that that this
233+
/// `impl Trait` desugars to. e.g., if:
234+
///
235+
/// fn foo<'a, 'b, T>() -> impl Trait<'a>
236+
///
237+
/// winds up desugared to:
238+
///
239+
/// abstract type Foo<'x, T>: Trait<'x>
240+
/// fn foo<'a, 'b, T>() -> Foo<'a, T>
241+
///
242+
/// then `substs` would be `['a, T]`.
243+
substs: &'tcx Substs<'tcx>,
244+
245+
/// The type variable that represents the value of the abstract type
246+
/// that we require. In other words, after we compile this function,
247+
/// we will be created a constraint like:
248+
///
249+
/// Foo<'a, T> = ?C
250+
///
251+
/// where `?C` is the value of this type variable. =) It may
252+
/// naturally refer to the type and lifetime parameters in scope
253+
/// in this function, though ultimately it should only reference
254+
/// those that are arguments to `Foo` in the constraint above. (In
255+
/// other words, `?C` should not include `'b`, even though it's a
256+
/// lifetime parameter on `foo`.)
257+
concrete_ty: Ty<'tcx>,
258+
}
259+
228260
impl<'a, 'gcx, 'tcx> Deref for Inherited<'a, 'gcx, 'tcx> {
229261
type Target = InferCtxt<'a, 'gcx, 'tcx>;
230262
fn deref(&self) -> &Self::Target {
@@ -888,7 +920,10 @@ fn typeck_tables_of<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
888920
param_env,
889921
&fn_sig);
890922

891-
check_fn(&inh, param_env, fn_sig, decl, id, body, false).0
923+
let fcx = check_fn(&inh, param_env, fn_sig, decl, id, body, false).0;
924+
// Ensure anon_types have been instantiated prior to entering regionck
925+
fcx.instantiate_anon_types(&fn_sig.output());
926+
fcx
892927
} else {
893928
let fcx = FnCtxt::new(&inh, param_env, body.value.id);
894929
let expected_type = tcx.type_of(def_id);
@@ -1943,12 +1978,15 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
19431978

19441979
// Use the same type variable if the exact same TyAnon appears more
19451980
// than once in the return type (e.g. if it's passed to a type alias).
1946-
if let Some(&(_substs, ty_var)) = self.anon_types.borrow().get(&def_id) {
1947-
return ty_var;
1981+
if let Some(anon_defn) = self.anon_types.borrow().get(&def_id) {
1982+
return anon_defn.concrete_ty;
19481983
}
19491984
let span = self.tcx.def_span(def_id);
19501985
let ty_var = self.next_ty_var(TypeVariableOrigin::TypeInference(span));
1951-
self.anon_types.borrow_mut().insert(def_id, (substs, ty_var));
1986+
self.anon_types.borrow_mut().insert(def_id, AnonTypeDecl {
1987+
substs: substs,
1988+
concrete_ty: ty_var
1989+
});
19521990
debug!("instantiate_anon_types: ty_var={:?}", ty_var);
19531991

19541992
let predicates_of = self.tcx.predicates_of(def_id);

src/librustc_typeck/check/regionck.rs

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,11 +338,14 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> {
338338
debug!("visit_fn_body body.id {:?} call_site_scope: {:?}",
339339
body.id(), call_site_scope);
340340
let call_site_region = self.tcx.mk_region(ty::ReScope(call_site_scope));
341+
341342
let body_hir_id = self.tcx.hir.node_to_hir_id(body_id.node_id);
342343
self.type_of_node_must_outlive(infer::CallReturn(span),
343344
body_hir_id,
344345
call_site_region);
345346

347+
self.constrain_anon_types();
348+
346349
self.region_bound_pairs.truncate(old_region_bounds_pairs_len);
347350

348351
self.set_body_id(old_body_id);
@@ -387,6 +390,140 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> {
387390
|| infer::RelateParamBound(cause.span, sup_type))
388391
}
389392

393+
/// Go through each of the existential `impl Trait` types that
394+
/// appear in the function signature. For example, if the current
395+
/// function is as follows:
396+
///
397+
/// fn foo<'a, 'b>(..) -> (impl Bar<'a>, impl Bar<'b>)
398+
///
399+
/// we would iterate through the `impl Bar<'a>` and the
400+
/// `impl Bar<'b>` here. Remember that each of them has
401+
/// their own "abstract type" definition created for them. As
402+
/// we iterate, we have a `def_id` that corresponds to this
403+
/// definition, and a set of substitutions `substs` that are
404+
/// being supplied to this abstract typed definition in the
405+
/// signature:
406+
///
407+
/// abstract type Foo1<'x>: Bar<'x>;
408+
/// abstract type Foo2<'x>: Bar<'x>;
409+
/// fn foo<'a, 'b>(..) -> (Foo1<'a>, Foo2<'b>) { .. }
410+
/// ^^^^ ^^ substs
411+
/// def_id
412+
///
413+
/// In addition, for each of the types we will have a type
414+
/// variable `concrete_ty` containing the concrete type that
415+
/// this function uses for `Foo1` and `Foo2`. That is,
416+
/// conceptually, there is a constraint like:
417+
///
418+
/// for<'a> (Foo1<'a> = C)
419+
///
420+
/// where `C` is `concrete_ty`. For this equation to be satisfiable,
421+
/// the type `C` can only refer to two regions: `'static` and `'a`.
422+
///
423+
/// The problem is that this type `C` may contain arbitrary
424+
/// region variables. In fact, it is fairly likely that it
425+
/// does! Consider this possible definition of `foo`:
426+
///
427+
/// fn foo<'a, 'b>(x: &'a i32, y: &'b i32) -> (impl Bar<'a>, impl Bar<'b>) {
428+
/// (&*x, &*y)
429+
/// }
430+
///
431+
/// Here, the values for the concrete types of the two impl
432+
/// traits will include inference variables:
433+
///
434+
/// &'0 i32
435+
/// &'1 i32
436+
///
437+
/// Ordinarily, the subtyping rules would ensure that these are
438+
/// sufficiently large. But since `impl Bar<'a>` isn't a specific
439+
/// type per se, we don't get such constraints by default. This
440+
/// is where this function comes into play. It adds extra
441+
/// constraints to ensure that all the regions which appear in the
442+
/// inferred type are regions that could validly appear.
443+
///
444+
/// This is actually a bit of a tricky constraint in general. We
445+
/// want to say that each variable (e.g., `'0``) can only take on
446+
/// values that were supplied as arguments to the abstract type
447+
/// (e.g., `'a` for `Foo1<'a>`) or `'static`, which is always in
448+
/// scope. We don't have a constraint quite of this kind in the current
449+
/// region checker.
450+
///
451+
/// What we *do* have is the `<=` relation. So what we do is to
452+
/// find the LUB of all the arguments that appear in the substs:
453+
/// in this case, that would be `LUB('a) = 'a`, and then we apply
454+
/// that as a least bound to the variables (e.g., `'a <= '0`).
455+
///
456+
/// In some cases this is pretty suboptimal. Consider this example:
457+
///
458+
/// fn baz<'a, 'b>() -> impl Trait<'a, 'b> { ... }
459+
///
460+
/// Here, the regions `'a` and `'b` appear in the substitutions,
461+
/// so we would generate `LUB('a, 'b)` as a kind of "minimal upper
462+
/// bound", but that turns out be `'static` -- which is clearly
463+
/// too strict!
464+
fn constrain_anon_types(&mut self) {
465+
debug!("constrain_anon_types()");
466+
467+
for (&def_id, anon_defn) in self.fcx.anon_types.borrow().iter() {
468+
let concrete_ty = self.resolve_type(anon_defn.concrete_ty);
469+
470+
debug!("constrain_anon_types: def_id={:?}", def_id);
471+
debug!("constrain_anon_types: substs={:?}", anon_defn.substs);
472+
debug!("constrain_anon_types: concrete_ty={:?}", concrete_ty);
473+
474+
let abstract_type_generics = self.tcx.generics_of(def_id);
475+
476+
// Go through all the regions used as arguments to the
477+
// abstract type. These are the parameters to the abstract
478+
// type; so in our example above, `substs` would contain
479+
// `['a]` for the first impl trait and `'b` for the
480+
// second.
481+
let mut bound_region = None;
482+
for region_def in &abstract_type_generics.regions {
483+
// Find the index of this region in the list of substitutions.
484+
let index = region_def.index as usize;
485+
486+
// Get the value supplied for this region from the substs.
487+
let subst_arg = anon_defn.substs[index].as_region().unwrap();
488+
489+
// Compute the least upper bound of it with the other regions.
490+
debug!("constrain_anon_types: bound_region={:?}", bound_region);
491+
debug!("constrain_anon_types: subst_arg={:?}", subst_arg);
492+
if let Some(r) = bound_region {
493+
let lub = self.free_region_map.lub_free_regions(self.tcx,
494+
r,
495+
subst_arg);
496+
bound_region = Some(lub);
497+
498+
if let ty::ReStatic = *lub {
499+
// If LUB results in `'static`, we might as well
500+
// stop iterating here.
501+
//
502+
// FIXME: We might consider issuing an error
503+
// here. `'static` is too strict to be useful,
504+
// most likely, and the resulting errors may
505+
// well be rather confusing.
506+
break;
507+
}
508+
} else {
509+
bound_region = Some(subst_arg);
510+
}
511+
}
512+
debug!("constrain_anon_types: bound_region={:?}", bound_region);
513+
514+
// If we don't find any arguments in the interface, then
515+
// the only valid region that can appear in the resulting
516+
// type is `'static`.
517+
let bound_region = bound_region.unwrap_or(self.tcx.types.re_static);
518+
519+
// Require that all regions outlive `bound_region`
520+
let span = self.tcx.def_span(def_id);
521+
self.tcx.for_each_free_region(&concrete_ty, |region| {
522+
self.sub_regions(infer::CallReturn(span), bound_region, region);
523+
});
524+
}
525+
}
526+
390527
/// This method populates the region map's `free_region_map`. It walks over the transformed
391528
/// argument and return types for each function just before we check the body of that function,
392529
/// looking for types where you have a borrowed pointer to other borrowed data (e.g., `&'a &'b

src/librustc_typeck/check/writeback.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -286,9 +286,9 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
286286

287287
fn visit_anon_types(&mut self) {
288288
let gcx = self.tcx().global_tcx();
289-
for (&def_id, &(substs, concrete_ty)) in self.fcx.anon_types.borrow().iter() {
289+
for (&def_id, anon_defn) in self.fcx.anon_types.borrow().iter() {
290290
let node_id = gcx.hir.as_local_node_id(def_id).unwrap();
291-
let inside_ty = self.resolve(&concrete_ty, &node_id);
291+
let inside_ty = self.resolve(&anon_defn.concrete_ty, &node_id);
292292

293293
// Use substs to build up a reverse map from regions
294294
// to their identity mappings.
@@ -298,9 +298,11 @@ impl<'cx, 'gcx, 'tcx> WritebackCx<'cx, 'gcx, 'tcx> {
298298
// resulting in the parameters shifting.
299299
let id_substs = Substs::identity_for_item(gcx, def_id);
300300
let map: FxHashMap<Kind, Kind> =
301-
substs.iter().enumerate()
302-
.map(|(index, subst)| (*subst, id_substs[index]))
303-
.collect();
301+
anon_defn.substs
302+
.iter()
303+
.enumerate()
304+
.map(|(index, subst)| (*subst, id_substs[index]))
305+
.collect();
304306

305307
// Convert the type from the function into a type valid outside
306308
// the function, by replacing invalid regions with 'static,

0 commit comments

Comments
 (0)