Auto merge of #46509 - nikomatsakis:nll-master-to-rust-master-3, r=arielb1

closure handling for NLL

This branch overhauls how we handle closures and universally quantified regions in the NLL code. The goal is to lay the groundwork for "region erasure" by the HIR-based type checker, as well as to avoid "crazy hacks" when it comes to closures. This means that when we type-check a closure, we cannot make use of *any* of the precise values of free regions in its signature, since those are inferred by the HIR type-checker. Therefore, the new code handles closures by:

- Creating fresh regions for every free region that appears in the closure's type, which now includes both its signature and the types of all upvars.
    - This means that the closure is type-checked without knowing about the connections.
- When we encounter some region relationship that we can't locally verify, we propagate it to the closure's creator.
- During MIR typeck, the closure creators then validates those relationships.

For a good example and explanation, see e.g. the test `src/test/nll/closure-requirements/propagate-despite-same-free-region.rs`.

Upcoming changes in the next NLL PR (not included in this PR in order to keep it manageable):

- Improvements to the MIR type checker so that it handles a lot of stuff presently overlooked
- Refactor how we store region values to make it more efficient, better encapsulated
- Propagate "type-outlives" relationships like `T: 'a` in a similar fashion to the one covered in this PR (still in the works, but close)
- Improvements to error reporting (still in the works)

r? @arielb1 or @pnkfelix

Author: bors

src/librustc/ich/impls_mir.rs

@@ -572,3 +572,15 @@ impl<'gcx> HashStable<StableHashingContext<'gcx>> for mir::Literal<'gcx> {
 }
 
 impl_stable_hash_for!(struct mir::Location { block, statement_index });
+
+impl_stable_hash_for!(struct mir::ClosureRegionRequirements {
+    num_external_vids,
+    outlives_requirements
+});
+
+impl_stable_hash_for!(struct mir::ClosureOutlivesRequirement {
+    free_region,
+    outlived_free_region,
+    blame_span
+});
+

src/librustc/ich/impls_ty.rs

@@ -84,6 +84,16 @@ for ty::RegionKind {
     }
 }
 
+impl<'gcx> HashStable<StableHashingContext<'gcx>> for ty::RegionVid {
+    #[inline]
+    fn hash_stable<W: StableHasherResult>(&self,
+                                          hcx: &mut StableHashingContext<'gcx>,
+                                          hasher: &mut StableHasher<W>) {
+        use rustc_data_structures::indexed_vec::Idx;
+        self.index().hash_stable(hcx, hasher);
+    }
+}
+
 impl<'gcx> HashStable<StableHashingContext<'gcx>>
 for ty::adjustment::AutoBorrow<'gcx> {
     fn hash_stable<W: StableHasherResult>(&self,

src/librustc/infer/mod.rs

@@ -1062,6 +1062,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         self.tcx.mk_region(ty::ReVar(self.borrow_region_constraints().new_region_var(origin)))
     }
 
+    /// Number of region variables created so far.
+    pub fn num_region_vars(&self) -> usize {
+        self.borrow_region_constraints().var_origins().len()
+    }
+
     /// Just a convenient wrapper of `next_region_var` for using during NLL.
     pub fn next_nll_region_var(&self, origin: NLLRegionVariableOrigin)
                                -> ty::Region<'tcx> {
@@ -1475,38 +1480,18 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         closure_kind_ty.to_opt_closure_kind()
     }
 
-    /// Obtain the signature of a function or closure.
-    /// For closures, unlike `tcx.fn_sig(def_id)`, this method will
-    /// work during the type-checking of the enclosing function and
-    /// return the closure signature in its partially inferred state.
-    pub fn fn_sig(&self, def_id: DefId) -> ty::PolyFnSig<'tcx> {
-        // Do we have an in-progress set of tables we are inferring?
-        if let Some(tables) = self.in_progress_tables {
-            // Is this a local item?
-            if let Some(id) = self.tcx.hir.as_local_node_id(def_id) {
-                // Is it a local *closure*?
-                if self.tcx.is_closure(def_id) {
-                    let hir_id = self.tcx.hir.node_to_hir_id(id);
-                    // Is this local closure contained within the tables we are inferring?
-                    if tables.borrow().local_id_root == Some(DefId::local(hir_id.owner)) {
-                        // if so, extract signature from there.
-                        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)
+    /// Obtain the signature of a closure.  For closures, unlike
+    /// `tcx.fn_sig(def_id)`, this method will work during the
+    /// type-checking of the enclosing function and return the closure
+    /// signature in its partially inferred state.
+    pub fn closure_sig(
+        &self,
+        def_id: DefId,
+        substs: ty::ClosureSubsts<'tcx>
+    ) -> ty::PolyFnSig<'tcx> {
+        let closure_sig_ty = substs.closure_sig_ty(def_id, self.tcx);
+        let closure_sig_ty = self.shallow_resolve(&closure_sig_ty);
+        closure_sig_ty.fn_sig(self.tcx)
     }
 
     /// Normalizes associated types in `value`, potentially returning

src/librustc/mir/mod.rs

@@ -1789,6 +1789,75 @@ pub struct GeneratorLayout<'tcx> {
     pub fields: Vec<LocalDecl<'tcx>>,
 }
 
+/// After we borrow check a closure, we are left with various
+/// requirements that we have inferred between the free regions that
+/// appear in the closure's signature or on its field types.  These
+/// requirements are then verified and proved by the closure's
+/// creating function. This struct encodes those requirements.
+///
+/// The requirements are listed as being between various
+/// `RegionVid`. The 0th region refers to `'static`; subsequent region
+/// vids refer to the free regions that appear in the closure (or
+/// generator's) type, in order of appearance. (This numbering is
+/// actually defined by the `UniversalRegions` struct in the NLL
+/// region checker. See for example
+/// `UniversalRegions::closure_mapping`.) Note that we treat the free
+/// regions in the closure's type "as if" they were erased, so their
+/// precise identity is not important, only their position.
+///
+/// Example: If type check produces a closure with the closure substs:
+///
+/// ```
+/// ClosureSubsts = [
+///     i8,                                  // the "closure kind"
+///     for<'x> fn(&'a &'x u32) -> &'x u32,  // the "closure signature"
+///     &'a String,                          // some upvar
+/// ]
+/// ```
+///
+/// here, there is one unique free region (`'a`) but it appears
+/// twice. We would "renumber" each occurence to a unique vid, as follows:
+///
+/// ```
+/// ClosureSubsts = [
+///     i8,                                  // the "closure kind"
+///     for<'x> fn(&'1 &'x u32) -> &'x u32,  // the "closure signature"
+///     &'2 String,                          // some upvar
+/// ]
+/// ```
+///
+/// Now the code might impose a requirement like `'1: '2`. When an
+/// instance of the closure is created, the corresponding free regions
+/// can be extracted from its type and constrained to have the given
+/// outlives relationship.
+#[derive(Clone, Debug)]
+pub struct ClosureRegionRequirements {
+    /// The number of external regions defined on the closure.  In our
+    /// example above, it would be 3 -- one for `'static`, then `'1`
+    /// and `'2`. This is just used for a sanity check later on, to
+    /// make sure that the number of regions we see at the callsite
+    /// matches.
+    pub num_external_vids: usize,
+
+    /// Requirements between the various free regions defined in
+    /// indices.
+    pub outlives_requirements: Vec<ClosureOutlivesRequirement>,
+}
+
+/// Indicates an outlives constraint between two free-regions declared
+/// on the closure.
+#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct ClosureOutlivesRequirement {
+    // This region ...
+    pub free_region: ty::RegionVid,
+
+    // .. must outlive this one.
+    pub outlived_free_region: ty::RegionVid,
+
+    // If not, report an error here.
+    pub blame_span: Span,
+}
+
 /*
  * TypeFoldable implementations for MIR types
  */

src/librustc/traits/project.rs

@@ -1339,26 +1339,27 @@ fn confirm_closure_candidate<'cx, 'gcx, 'tcx>(
     vtable: VtableClosureData<'tcx, PredicateObligation<'tcx>>)
     -> Progress<'tcx>
 {
-    let closure_typer = selcx.closure_typer();
-    let closure_type = closure_typer.fn_sig(vtable.closure_def_id)
-        .subst(selcx.tcx(), vtable.substs.substs);
+    let tcx = selcx.tcx();
+    let infcx = selcx.infcx();
+    let closure_sig_ty = vtable.substs.closure_sig_ty(vtable.closure_def_id, tcx);
+    let closure_sig = infcx.shallow_resolve(&closure_sig_ty).fn_sig(tcx);
     let Normalized {
-        value: closure_type,
+        value: closure_sig,
         obligations
     } = normalize_with_depth(selcx,
                              obligation.param_env,
                              obligation.cause.clone(),
                              obligation.recursion_depth+1,
-                             &closure_type);
+                             &closure_sig);
 
-    debug!("confirm_closure_candidate: obligation={:?},closure_type={:?},obligations={:?}",
+    debug!("confirm_closure_candidate: obligation={:?},closure_sig={:?},obligations={:?}",
            obligation,
-           closure_type,
+           closure_sig,
            obligations);
 
     confirm_callable_candidate(selcx,
                                obligation,
-                               closure_type,
+                               closure_sig,
                                util::TupleArgumentsFlag::No)
         .with_addl_obligations(vtable.nested)
         .with_addl_obligations(obligations)

src/librustc/traits/select.rs

@@ -3183,8 +3183,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                                       substs: ty::ClosureSubsts<'tcx>)
                                       -> ty::PolyTraitRef<'tcx>
     {
-        let closure_type = self.infcx.fn_sig(closure_def_id)
-            .subst(self.tcx(), substs.substs);
+        let closure_type = self.infcx.closure_sig(closure_def_id, substs);
         let ty::Binder((trait_ref, _)) =
             self.tcx().closure_trait_ref_and_return_type(obligation.predicate.def_id(),
                                                          obligation.predicate.0.self_ty(), // (1)

src/librustc/ty/instance.rs

@@ -10,7 +10,7 @@
 
 use hir::def_id::DefId;
 use ty::{self, Ty, TypeFoldable, Substs, TyCtxt};
-use ty::subst::{Kind, Subst};
+use ty::subst::Kind;
 use traits;
 use syntax::abi::Abi;
 use util::ppaux;
@@ -311,7 +311,7 @@ fn fn_once_adapter_instance<'a, 'tcx>(
     let self_ty = tcx.mk_closure_from_closure_substs(
         closure_did, substs);
 
-    let sig = tcx.fn_sig(closure_did).subst(tcx, substs.substs);
+    let sig = substs.closure_sig(closure_did, tcx);
     let sig = tcx.erase_late_bound_regions_and_normalize(&sig);
     assert_eq!(sig.inputs().len(), 1);
     let substs = tcx.mk_substs([

src/librustc/ty/maps/mod.rs

@@ -190,8 +190,10 @@ define_maps! { <'tcx>
     [] fn coherent_trait: coherent_trait_dep_node((CrateNum, DefId)) -> (),
 
     [] fn borrowck: BorrowCheck(DefId) -> Rc<BorrowCheckResult>,
-    // FIXME: shouldn't this return a `Result<(), BorrowckErrors>` instead?
-    [] fn mir_borrowck: MirBorrowCheck(DefId) -> (),
+
+    /// Borrow checks the function body. If this is a closure, returns
+    /// additional requirements that the closure's creator must verify.
+    [] fn mir_borrowck: MirBorrowCheck(DefId) -> Option<mir::ClosureRegionRequirements>,
 
     /// Gets a complete map from all types to their inherent impls.
     /// Not meant to be used directly outside of coherence.

src/librustc/ty/sty.rs

@@ -356,13 +356,17 @@ impl<'tcx> ClosureSubsts<'tcx> {
     /// Returns the closure kind for this closure; only usable outside
     /// of an inference context, because in that context we know that
     /// there are no type variables.
+    ///
+    /// If you have an inference context, use `infcx.closure_kind()`.
     pub fn closure_kind(self, def_id: DefId, tcx: TyCtxt<'_, 'tcx, 'tcx>) -> ty::ClosureKind {
         self.split(def_id, tcx).closure_kind_ty.to_opt_closure_kind().unwrap()
     }
 
     /// Extracts the signature from the closure; only usable outside
     /// of an inference context, because in that context we know that
     /// there are no type variables.
+    ///
+    /// If you have an inference context, use `infcx.closure_sig()`.
     pub fn closure_sig(self, def_id: DefId, tcx: TyCtxt<'_, 'tcx, 'tcx>) -> ty::PolyFnSig<'tcx> {
         match self.closure_sig_ty(def_id, tcx).sty {
             ty::TyFnPtr(sig) => sig,
@@ -646,6 +650,17 @@ impl<'tcx> PolyExistentialTraitRef<'tcx> {
 pub struct Binder<T>(pub T);
 
 impl<T> Binder<T> {
+    /// Wraps `value` in a binder, asserting that `value` does not
+    /// contain any bound regions that would be bound by the
+    /// binder. This is commonly used to 'inject' a value T into a
+    /// different binding level.
+    pub fn dummy<'tcx>(value: T) -> Binder<T>
+        where T: TypeFoldable<'tcx>
+    {
+        assert!(!value.has_escaping_regions());
+        Binder(value)
+    }
+
     /// Skips the binder and returns the "bound" value. This is a
     /// risky thing to do because it's easy to get confused about
     /// debruijn indices and the like. It is usually better to
@@ -700,6 +715,32 @@ impl<T> Binder<T> {
             Some(self.skip_binder().clone())
         }
     }
+
+    /// Given two things that have the same binder level,
+    /// and an operation that wraps on their contents, execute the operation
+    /// and then wrap its result.
+    ///
+    /// `f` should consider bound regions at depth 1 to be free, and
+    /// anything it produces with bound regions at depth 1 will be
+    /// bound in the resulting return value.
+    pub fn fuse<U,F,R>(self, u: Binder<U>, f: F) -> Binder<R>
+        where F: FnOnce(T, U) -> R
+    {
+        ty::Binder(f(self.0, u.0))
+    }
+
+    /// Split the contents into two things that share the same binder
+    /// level as the original, returning two distinct binders.
+    ///
+    /// `f` should consider bound regions at depth 1 to be free, and
+    /// anything it produces with bound regions at depth 1 will be
+    /// bound in the resulting return values.
+    pub fn split<U,V,F>(self, f: F) -> (Binder<U>, Binder<V>)
+        where F: FnOnce(T) -> (U, V)
+    {
+        let (u, v) = f(self.0);
+        (ty::Binder(u), ty::Binder(v))
+    }
 }
 
 /// Represents the projection of an associated type. In explicit UFCS
@@ -799,6 +840,9 @@ impl<'tcx> PolyFnSig<'tcx> {
     pub fn input(&self, index: usize) -> ty::Binder<Ty<'tcx>> {
         self.map_bound_ref(|fn_sig| fn_sig.inputs()[index])
     }
+    pub fn inputs_and_output(&self) -> ty::Binder<&'tcx Slice<Ty<'tcx>>> {
+        self.map_bound_ref(|fn_sig| fn_sig.inputs_and_output)
+    }
     pub fn output(&self) -> ty::Binder<Ty<'tcx>> {
         self.map_bound_ref(|fn_sig| fn_sig.output().clone())
     }

src/librustc_driver/driver.rs

@@ -1069,7 +1069,7 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(control: &CompileController,
 
         time(time_passes,
              "MIR borrow checking",
-             || for def_id in tcx.body_owners() { tcx.mir_borrowck(def_id) });
+             || for def_id in tcx.body_owners() { tcx.mir_borrowck(def_id); });
 
         time(time_passes,
              "MIR effect checking",