Skip to content

Commit

Permalink
Auto merge of #45879 - nikomatsakis:nll-kill-cyclic-closures, r=<try>
Browse files Browse the repository at this point in the history
[WIP] move closure kind, signature into `ClosureSubsts`

Instead of using side-tables, store the closure-kind and signature in the substitutions themselves. This has two key effects:

- It means that the closure's type changes as inference finds out more things, which is very nice.
    - As a result, it avoids the need for the `freshen_closure_like` code (though we still use it for generators).
- It avoids cyclic closures calls.
    - These were never meant to be supported, precisely because they make a lot of the fancy inference that we do much more complicated. However, due to an oversight, it was previously possible -- if challenging -- to create a setup where a closure *directly* called itself (see e.g. #21410).

We have to see what the effect of this change is, though. Needs a crater run. Marking as [WIP] until that has been assessed.

r? @arielb1
  • Loading branch information
bors committed Nov 13, 2017
2 parents b7ccb0a + 0f6bd17 commit e3645b5
Show file tree
Hide file tree
Showing 44 changed files with 729 additions and 647 deletions.
2 changes: 0 additions & 2 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -498,9 +498,7 @@ define_dep_nodes!( <'tcx>
[] IsAutoImpl(DefId),
[] ImplTraitRef(DefId),
[] ImplPolarity(DefId),
[] ClosureKind(DefId),
[] FnSignature(DefId),
[] GenSignature(DefId),
[] CoerceUnsizedInfo(DefId),

[] ItemVarianceConstraints(DefId),
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/ich/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,9 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for ty::Predicate<'gcx> {
ty::Predicate::ObjectSafe(def_id) => {
def_id.hash_stable(hcx, hasher);
}
ty::Predicate::ClosureKind(def_id, closure_kind) => {
ty::Predicate::ClosureKind(def_id, closure_substs, closure_kind) => {
def_id.hash_stable(hcx, hasher);
closure_substs.hash_stable(hcx, hasher);
closure_kind.hash_stable(hcx, hasher);
}
ty::Predicate::ConstEvaluatable(def_id, substs) => {
Expand Down
4 changes: 1 addition & 3 deletions src/librustc/infer/error_reporting/need_type_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
// ```
labels.clear();
labels.push((pattern.span, format!("consider giving this closure parameter a type")));
}

if let Some(pattern) = local_visitor.found_local_pattern {
} else if let Some(pattern) = local_visitor.found_local_pattern {
if let Some(simple_name) = pattern.simple_name() {
labels.push((pattern.span, format!("consider giving `{}` a type", simple_name)));
} else {
Expand Down
133 changes: 2 additions & 131 deletions src/librustc/infer/freshen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@
use ty::{self, Ty, TyCtxt, TypeFoldable};
use ty::fold::TypeFolder;
use ty::subst::Substs;
use util::nodemap::FxHashMap;
use hir::def_id::DefId;

use std::collections::hash_map::Entry;

Expand All @@ -56,7 +54,6 @@ pub struct TypeFreshener<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
infcx: &'a InferCtxt<'a, 'gcx, 'tcx>,
freshen_count: u32,
freshen_map: FxHashMap<ty::InferTy, Ty<'tcx>>,
closure_set: Vec<DefId>,
}

impl<'a, 'gcx, 'tcx> TypeFreshener<'a, 'gcx, 'tcx> {
Expand All @@ -66,7 +63,6 @@ impl<'a, 'gcx, 'tcx> TypeFreshener<'a, 'gcx, 'tcx> {
infcx,
freshen_count: 0,
freshen_map: FxHashMap(),
closure_set: vec![],
}
}

Expand All @@ -92,88 +88,6 @@ impl<'a, 'gcx, 'tcx> TypeFreshener<'a, 'gcx, 'tcx> {
}
}
}

fn next_fresh<F>(&mut self,
freshener: F)
-> Ty<'tcx>
where F: FnOnce(u32) -> ty::InferTy,
{
let index = self.freshen_count;
self.freshen_count += 1;
self.infcx.tcx.mk_infer(freshener(index))
}

fn freshen_closure_like<M, C>(&mut self,
def_id: DefId,
substs: ty::ClosureSubsts<'tcx>,
t: Ty<'tcx>,
markers: M,
combine: C)
-> Ty<'tcx>
where M: FnOnce(&mut Self) -> (Ty<'tcx>, Ty<'tcx>),
C: FnOnce(&'tcx Substs<'tcx>) -> Ty<'tcx>
{
let tcx = self.infcx.tcx;

let closure_in_progress = self.infcx.in_progress_tables.map_or(false, |tables| {
tcx.hir.as_local_node_id(def_id).map_or(false, |closure_id| {
tables.borrow().local_id_root ==
Some(DefId::local(tcx.hir.node_to_hir_id(closure_id).owner))
})
});

if !closure_in_progress {
// If this closure belongs to another infcx, its kind etc. were
// fully inferred and its signature/kind are exactly what's listed
// in its infcx. So we don't need to add the markers for them.
return t.super_fold_with(self);
}

// We are encoding a closure in progress. Because we want our freshening
// key to contain all inference information needed to make sense of our
// value, we need to encode the closure signature and kind. The way
// we do that is to add them as 2 variables to the closure substs,
// basically because it's there (and nobody cares about adding extra stuff
// to substs).
//
// This means the "freshened" closure substs ends up looking like
// fresh_substs = [PARENT_SUBSTS* ; UPVARS* ; SIG_MARKER ; KIND_MARKER]
let (marker_1, marker_2) = if self.closure_set.contains(&def_id) {
// We found the closure def-id within its own signature. Just
// leave a new freshened type - any matching operations would
// have found and compared the exterior closure already to
// get here.
//
// In that case, we already know what the signature would
// be - the parent closure on the stack already contains a
// "copy" of the signature, so there is no reason to encode
// it again for injectivity. Just use a fresh type variable
// to make everything comparable.
//
// For example (closure kinds omitted for clarity)
// t=[closure FOO sig=[closure BAR sig=[closure FOO ..]]]
// Would get encoded to
// t=[closure FOO sig=[closure BAR sig=[closure FOO sig=$0]]]
//
// and we can decode by having
// $0=[closure BAR {sig doesn't exist in decode}]
// and get
// t=[closure FOO]
// sig[FOO] = [closure BAR]
// sig[BAR] = [closure FOO]
(self.next_fresh(ty::FreshTy), self.next_fresh(ty::FreshTy))
} else {
self.closure_set.push(def_id);
let markers = markers(self);
self.closure_set.pop();
markers
};

combine(tcx.mk_substs(
substs.substs.iter().map(|k| k.fold_with(self)).chain(
[marker_1, marker_2].iter().cloned().map(From::from)
)))
}
}

impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for TypeFreshener<'a, 'gcx, 'tcx> {
Expand Down Expand Up @@ -249,51 +163,7 @@ impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for TypeFreshener<'a, 'gcx, 'tcx> {
t
}

ty::TyClosure(def_id, substs) => {
self.freshen_closure_like(
def_id, substs, t,
|this| {
// HACK: use a "random" integer type to mark the kind. Because
// different closure kinds shouldn't get unified during
// selection, the "subtyping" relationship (where any kind is
// better than no kind) shouldn't matter here, just that the
// types are different.
let closure_kind = this.infcx.closure_kind(def_id);
let closure_kind_marker = match closure_kind {
None => tcx.types.i8,
Some(ty::ClosureKind::Fn) => tcx.types.i16,
Some(ty::ClosureKind::FnMut) => tcx.types.i32,
Some(ty::ClosureKind::FnOnce) => tcx.types.i64,
};

let closure_sig = this.infcx.fn_sig(def_id);
(tcx.mk_fn_ptr(closure_sig.fold_with(this)),
closure_kind_marker)
},
|substs| tcx.mk_closure(def_id, substs)
)
}

ty::TyGenerator(def_id, substs, interior) => {
self.freshen_closure_like(
def_id, substs, t,
|this| {
let gen_sig = this.infcx.generator_sig(def_id).unwrap();
// FIXME: want to revise this strategy when generator
// signatures can actually contain LBRs.
let sig = this.tcx().no_late_bound_regions(&gen_sig)
.unwrap_or_else(|| {
bug!("late-bound regions in signature of {:?}",
def_id)
});
(sig.yield_ty, sig.return_ty).fold_with(this)
},
|substs| {
tcx.mk_generator(def_id, ty::ClosureSubsts { substs }, interior)
}
)
}

ty::TyGenerator(..) |
ty::TyBool |
ty::TyChar |
ty::TyInt(..) |
Expand All @@ -314,6 +184,7 @@ impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for TypeFreshener<'a, 'gcx, 'tcx> {
ty::TyProjection(..) |
ty::TyForeign(..) |
ty::TyParam(..) |
ty::TyClosure(..) |
ty::TyAnon(..) => {
t.super_fold_with(self)
}
Expand Down
52 changes: 19 additions & 33 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1338,26 +1338,17 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
!traits::type_known_to_meet_bound(self, param_env, ty, copy_def_id, span)
}

/// Obtains the latest type of the given closure; this may be a
/// closure in the current function, in which case its
/// `ClosureKind` may not yet be known.
pub fn closure_kind(&self,
def_id: DefId)
closure_def_id: DefId,
closure_substs: ty::ClosureSubsts<'tcx>)
-> Option<ty::ClosureKind>
{
if let Some(tables) = self.in_progress_tables {
if let Some(id) = self.tcx.hir.as_local_node_id(def_id) {
let hir_id = self.tcx.hir.node_to_hir_id(id);
return tables.borrow()
.closure_kinds()
.get(hir_id)
.cloned()
.map(|(kind, _)| kind);
}
}

// During typeck, ALL closures are local. But afterwards,
// during trans, we see closure ids from other traits.
// That may require loading the closure data out of the
// cstore.
Some(self.tcx.closure_kind(def_id))
let closure_kind_ty = closure_substs.closure_kind_ty(closure_def_id, self.tcx);
let closure_kind_ty = self.shallow_resolve(&closure_kind_ty);
closure_kind_ty.to_opt_closure_kind()
}

/// Obtain the signature of a function or closure.
Expand All @@ -1368,27 +1359,22 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
if let Some(tables) = self.in_progress_tables {
if let Some(id) = self.tcx.hir.as_local_node_id(def_id) {
let hir_id = self.tcx.hir.node_to_hir_id(id);
if let Some(&ty) = tables.borrow().closure_tys().get(hir_id) {
return ty;
}
let closure_ty = tables.borrow().node_id_to_type(hir_id);
let (closure_def_id, closure_substs) = match closure_ty.sty {
ty::TyClosure(closure_def_id, closure_substs) =>
(closure_def_id, closure_substs),
_ =>
bug!("closure with non-closure type: {:?}", closure_ty),
};
assert_eq!(def_id, closure_def_id);
let closure_sig_ty = closure_substs.closure_sig_ty(def_id, self.tcx);
let closure_sig_ty = self.shallow_resolve(&closure_sig_ty);
return closure_sig_ty.fn_sig(self.tcx);
}
}

self.tcx.fn_sig(def_id)
}

pub fn generator_sig(&self, def_id: DefId) -> Option<ty::PolyGenSig<'tcx>> {
if let Some(tables) = self.in_progress_tables {
if let Some(id) = self.tcx.hir.as_local_node_id(def_id) {
let hir_id = self.tcx.hir.node_to_hir_id(id);
if let Some(&ty) = tables.borrow().generator_sigs().get(hir_id) {
return ty.map(|t| ty::Binder(t));
}
}
}

self.tcx.generator_sig(def_id)
}
}

impl<'a, 'gcx, 'tcx> TypeTrace<'tcx> {
Expand Down
5 changes: 4 additions & 1 deletion src/librustc/infer/type_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@ pub enum TypeVariableOrigin {
NormalizeProjectionType(Span),
TypeInference(Span),
TypeParameterDefinition(Span, ast::Name),
TransformedUpvar(Span),

/// one of the upvars or closure kind parameters in a `ClosureSubsts`
/// (before it has been determined)
ClosureSynthetic(Span),
SubstitutionPlaceholder(Span),
AutoDeref(Span),
AdjustmentType(Span),
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,15 @@
#![feature(inclusive_range_syntax)]
#![cfg_attr(windows, feature(libc))]
#![feature(macro_vis_matcher)]
#![feature(match_default_bindings)]
#![feature(never_type)]
#![feature(nonzero)]
#![feature(quote)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_patterns)]
#![feature(specialization)]
#![feature(unboxed_closures)]
#![feature(underscore_lifetimes)]
#![feature(trace_macros)]
#![feature(test)]
#![feature(const_atomic_bool_new)]
Expand Down
17 changes: 13 additions & 4 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -750,10 +750,19 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {

let kind = match self.node_ty(fn_hir_id)?.sty {
ty::TyGenerator(..) => ty::ClosureKind::FnOnce,
ty::TyClosure(..) => {
match self.tables.closure_kinds().get(fn_hir_id) {
Some(&(kind, _)) => kind,
None => span_bug!(span, "missing closure kind"),
ty::TyClosure(closure_def_id, closure_substs) => {
match self.infcx {
// During upvar inference we may not know the
// closure kind, just use the LATTICE_BOTTOM value.
Some(infcx) =>
infcx.closure_kind(closure_def_id, closure_substs)
.unwrap_or(ty::ClosureKind::LATTICE_BOTTOM),

None =>
self.tcx.global_tcx()
.lift(&closure_substs)
.expect("no inference cx, but inference variables in closure ty")
.closure_kind(closure_def_id, self.tcx.global_tcx()),
}
}
ref t => span_bug!(span, "unexpected type for fn in mem_categorization: {:?}", t),
Expand Down
14 changes: 7 additions & 7 deletions src/librustc/traits/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -661,8 +661,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
violations)
}

ty::Predicate::ClosureKind(closure_def_id, kind) => {
let found_kind = self.closure_kind(closure_def_id).unwrap();
ty::Predicate::ClosureKind(closure_def_id, closure_substs, kind) => {
let found_kind = self.closure_kind(closure_def_id, closure_substs).unwrap();
let closure_span = self.tcx.hir.span_if_local(closure_def_id).unwrap();
let node_id = self.tcx.hir.as_local_node_id(closure_def_id).unwrap();
let mut err = struct_span_err!(
Expand All @@ -681,14 +681,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
if let Some(tables) = self.in_progress_tables {
let tables = tables.borrow();
let closure_hir_id = self.tcx.hir.node_to_hir_id(node_id);
match tables.closure_kinds().get(closure_hir_id) {
Some(&(ty::ClosureKind::FnOnce, Some((span, name)))) => {
err.span_note(span, &format!(
match (found_kind, tables.closure_kind_origins().get(closure_hir_id)) {
(ty::ClosureKind::FnOnce, Some((span, name))) => {
err.span_note(*span, &format!(
"closure is `FnOnce` because it moves the \
variable `{}` out of its environment", name));
},
Some(&(ty::ClosureKind::FnMut, Some((span, name)))) => {
err.span_note(span, &format!(
(ty::ClosureKind::FnMut, Some((span, name))) => {
err.span_note(*span, &format!(
"closure is `FnMut` because it mutates the \
variable `{}` here", name));
},
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -491,8 +491,8 @@ fn process_predicate<'a, 'gcx, 'tcx>(
}
}

ty::Predicate::ClosureKind(closure_def_id, kind) => {
match selcx.infcx().closure_kind(closure_def_id) {
ty::Predicate::ClosureKind(closure_def_id, closure_substs, kind) => {
match selcx.infcx().closure_kind(closure_def_id, closure_substs) {
Some(closure_kind) => {
if closure_kind.extends(kind) {
Ok(Some(vec![]))
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1264,8 +1264,7 @@ fn confirm_generator_candidate<'cx, 'gcx, 'tcx>(
vtable: VtableGeneratorData<'tcx, PredicateObligation<'tcx>>)
-> Progress<'tcx>
{
let gen_sig = selcx.infcx().generator_sig(vtable.closure_def_id).unwrap()
.subst(selcx.tcx(), vtable.substs.substs);
let gen_sig = vtable.substs.generator_poly_sig(vtable.closure_def_id, selcx.tcx());
let Normalized {
value: gen_sig,
obligations
Expand Down
Loading

0 comments on commit e3645b5

Please sign in to comment.