Auto merge of rust-lang#45879 - nikomatsakis:nll-kill-cyclic-closures…

…, r=arielb1

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. rust-lang#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

src/librustc/dep_graph/dep_node.rs

@@ -498,9 +498,7 @@ define_dep_nodes!( <'tcx>
     [] IsAutoImpl(DefId),
     [] ImplTraitRef(DefId),
     [] ImplPolarity(DefId),
-    [] ClosureKind(DefId),
     [] FnSignature(DefId),
-    [] GenSignature(DefId),
     [] CoerceUnsizedInfo(DefId),
 
     [] ItemVarianceConstraints(DefId),

src/librustc/diagnostics.rs

@@ -1969,8 +1969,39 @@ fn foo<'a>(x: &'a i32, y: &i32) -> &'a i32 {
 ```
 "##,
 
+E0644: r##"
+A closure or generator was constructed that references its own type.
+
+Erroneous example:
+
+```compile-fail,E0644
+fn fix<F>(f: &F)
+  where F: Fn(&F)
+{
+  f(&f);
 }
 
+fn main() {
+  fix(&|y| {
+    // Here, when `x` is called, the parameter `y` is equal to `x`.
+  });
+}
+```
+
+Rust does not permit a closure to directly reference its own type,
+either through an argument (as in the example above) or by capturing
+itself through its environment. This restriction helps keep closure
+inference tractable.
+
+The easiest fix is to rewrite your closure into a top-level function,
+or into a method. In some cases, you may also be able to have your
+closure call itself by capturing a `&Fn()` object or `fn()` pointer
+that refers to itself. That is permitting, since the closure would be
+invoking itself via a virtual call, and hence does not directly
+reference its own *type*.
+
+"##, }
+
 
 register_diagnostics! {
 //  E0006 // merged with E0005

src/librustc/ich/impls_ty.rs

@@ -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) => {

src/librustc/infer/combine.rs

@@ -270,6 +270,7 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> {
             for_vid_sub_root: self.infcx.type_variables.borrow_mut().sub_root_var(for_vid),
             ambient_variance,
             needs_wf: false,
+            root_ty: ty,
         };
 
         let ty = generalize.relate(&ty, &ty)?;
@@ -280,10 +281,23 @@ impl<'infcx, 'gcx, 'tcx> CombineFields<'infcx, 'gcx, 'tcx> {
 
 struct Generalizer<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> {
     infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>,
+
+    /// Span, used when creating new type variables and things.
     span: Span,
+
+    /// The vid of the type variable that is in the process of being
+    /// instantiated; if we find this within the type we are folding,
+    /// that means we would have created a cyclic type.
     for_vid_sub_root: ty::TyVid,
+
+    /// Track the variance as we descend into the type.
     ambient_variance: ty::Variance,
-    needs_wf: bool, // see the field `needs_wf` in `Generalization`
+
+    /// See the field `needs_wf` in `Generalization`.
+    needs_wf: bool,
+
+    /// The root type that we are generalizing. Used when reporting cycles.
+    root_ty: Ty<'tcx>,
 }
 
 /// Result from a generalization operation. This includes
@@ -386,7 +400,7 @@ impl<'cx, 'gcx, 'tcx> TypeRelation<'cx, 'gcx, 'tcx> for Generalizer<'cx, 'gcx, '
                 if sub_vid == self.for_vid_sub_root {
                     // If sub-roots are equal, then `for_vid` and
                     // `vid` are related via subtyping.
-                    return Err(TypeError::CyclicTy);
+                    return Err(TypeError::CyclicTy(self.root_ty));
                 } else {
                     match variables.probe_root(vid) {
                         Some(u) => {

src/librustc/infer/error_reporting/mod.rs

@@ -689,9 +689,16 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                          diag: &mut DiagnosticBuilder<'tcx>,
                          cause: &ObligationCause<'tcx>,
                          secondary_span: Option<(Span, String)>,
-                         values: Option<ValuePairs<'tcx>>,
+                         mut values: Option<ValuePairs<'tcx>>,
                          terr: &TypeError<'tcx>)
     {
+        // For some types of errors, expected-found does not make
+        // sense, so just ignore the values we were given.
+        match terr {
+            TypeError::CyclicTy(_) => { values = None; }
+            _ => { }
+        }
+
         let (expected_found, exp_found, is_simple_error) = match values {
             None => (None, None, false),
             Some(values) => {
@@ -780,17 +787,20 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                terr);
 
         let span = trace.cause.span;
-        let failure_str = trace.cause.as_failure_str();
-        let mut diag = match trace.cause.code {
-            ObligationCauseCode::IfExpressionWithNoElse => {
+        let failure_code = trace.cause.as_failure_code(terr);
+        let mut diag = match failure_code {
+            FailureCode::Error0317(failure_str) => {
                 struct_span_err!(self.tcx.sess, span, E0317, "{}", failure_str)
             }
-            ObligationCauseCode::MainFunctionType => {
+            FailureCode::Error0580(failure_str) => {
                 struct_span_err!(self.tcx.sess, span, E0580, "{}", failure_str)
             }
-            _ => {
+            FailureCode::Error0308(failure_str) => {
                 struct_span_err!(self.tcx.sess, span, E0308, "{}", failure_str)
             }
+            FailureCode::Error0644(failure_str) => {
+                struct_span_err!(self.tcx.sess, span, E0644, "{}", failure_str)
+            }
         };
         self.note_type_err(&mut diag, &trace.cause, None, Some(trace.values), terr);
         diag
@@ -1040,23 +1050,40 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     }
 }
 
+enum FailureCode {
+    Error0317(&'static str),
+    Error0580(&'static str),
+    Error0308(&'static str),
+    Error0644(&'static str),
+}
+
 impl<'tcx> ObligationCause<'tcx> {
-    fn as_failure_str(&self) -> &'static str {
+    fn as_failure_code(&self, terr: &TypeError<'tcx>) -> FailureCode {
+        use self::FailureCode::*;
         use traits::ObligationCauseCode::*;
         match self.code {
-            CompareImplMethodObligation { .. } => "method not compatible with trait",
-            MatchExpressionArm { source, .. } => match source {
+            CompareImplMethodObligation { .. } => Error0308("method not compatible with trait"),
+            MatchExpressionArm { source, .. } => Error0308(match source {
                 hir::MatchSource::IfLetDesugar{..} => "`if let` arms have incompatible types",
                 _ => "match arms have incompatible types",
-            },
-            IfExpression => "if and else have incompatible types",
-            IfExpressionWithNoElse => "if may be missing an else clause",
-            EquatePredicate => "equality predicate not satisfied",
-            MainFunctionType => "main function has wrong type",
-            StartFunctionType => "start function has wrong type",
-            IntrinsicType => "intrinsic has wrong type",
-            MethodReceiver => "mismatched method receiver",
-            _ => "mismatched types",
+            }),
+            IfExpression => Error0308("if and else have incompatible types"),
+            IfExpressionWithNoElse => Error0317("if may be missing an else clause"),
+            EquatePredicate => Error0308("equality predicate not satisfied"),
+            MainFunctionType => Error0580("main function has wrong type"),
+            StartFunctionType => Error0308("start function has wrong type"),
+            IntrinsicType => Error0308("intrinsic has wrong type"),
+            MethodReceiver => Error0308("mismatched method receiver"),
+
+            // In the case where we have no more specific thing to
+            // say, also take a look at the error code, maybe we can
+            // tailor to that.
+            _ => match terr {
+                TypeError::CyclicTy(ty) if ty.is_closure() || ty.is_generator() =>
+                    Error0644("closure/generator type that references itself"),
+                _ =>
+                    Error0308("mismatched types"),
+            }
         }
     }
 

src/librustc/infer/error_reporting/need_type_info.rs

@@ -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 {

src/librustc/infer/freshen.rs

@@ -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;
 
@@ -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> {
@@ -66,7 +63,6 @@ impl<'a, 'gcx, 'tcx> TypeFreshener<'a, 'gcx, 'tcx> {
             infcx,
             freshen_count: 0,
             freshen_map: FxHashMap(),
-            closure_set: vec![],
         }
     }
 
@@ -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> {
@@ -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(..) |
@@ -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)
             }