report an error if we see an unexpected lifetime in impl Trait

But leave closure substs alone.

Author: nikomatsakis

src/librustc/diagnostics.rs

@@ -2074,6 +2074,54 @@ a (non-transparent) struct containing a single float, while `Grams` is a
 transparent wrapper around a float. This can make a difference for the ABI.
 "##,
 
+E0909: r##"
+The `impl Trait` return type captures lifetime parameters that do not
+appear within the `impl Trait` itself.
+
+Erroneous code example:
+
+```compile-fail,E0909
+use std::cell::Cell;
+
+trait Trait<'a> { }
+
+impl Trait<'b> for Cell<&'a u32> { }
+
+fn foo<'x, 'y>(x: Cell<&'x u32>) -> impl Trait<'y>
+where 'x: 'y
+{
+    x
+}
+```
+
+Here, the function `foo` returns a value of type `Cell<&'x u32>`,
+which references the lifetime `'x`. However, the return type is
+declared as `impl Trait<'y>` -- this indicates that `foo` returns
+"some type that implements `Trait<'y>`", but it also indicates that
+the return type **only captures data referencing the lifetime `'y`**.
+In this case, though, we are referencing data with lifetime `'x`, so
+this function is in error.
+
+To fix this, you must reference the lifetime `'x` from the return
+type. For example, changing the return type to `impl Trait<'y> + 'x`
+would work:
+
+```
+use std::cell::Cell;
+
+trait Trait<'a> { }
+
+impl Trait<'b> for Cell<&'a u32> { }
+
+fn foo<'x, 'y>(x: Cell<&'x u32>) -> impl Trait<'y> + 'x
+where 'x: 'y
+{
+    x
+}
+```
+"##,
+
+
 }
 
 

src/librustc/infer/anon_types/mod.rs

@@ -17,7 +17,7 @@ use traits::{self, PredicateObligation};
 use ty::{self, Ty, TyCtxt};
 use ty::fold::{BottomUpFolder, TypeFoldable, TypeFolder};
 use ty::outlives::Component;
-use ty::subst::{Kind, UnpackedKind, Substs};
+use ty::subst::{Kind, Substs, UnpackedKind};
 use util::nodemap::DefIdMap;
 
 pub type AnonTypeMap<'tcx> = DefIdMap<AnonTypeDecl<'tcx>>;
@@ -113,10 +113,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
     ) -> InferOk<'tcx, (T, AnonTypeMap<'tcx>)> {
         debug!(
             "instantiate_anon_types(value={:?}, parent_def_id={:?}, body_id={:?}, param_env={:?})",
-            value,
-            parent_def_id,
-            body_id,
-            param_env,
+            value, parent_def_id, body_id, param_env,
         );
         let mut instantiator = Instantiator {
             infcx: self,
@@ -458,7 +455,14 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         // Convert the type from the function into a type valid outside
         // the function, by replacing invalid regions with 'static,
         // after producing an error for each of them.
-        let definition_ty = instantiated_ty.fold_with(&mut ReverseMapper { tcx: self.tcx, map });
+        let definition_ty =
+            instantiated_ty.fold_with(&mut ReverseMapper::new(
+                self.tcx,
+                self.is_tainted_by_errors(),
+                def_id,
+                map,
+                instantiated_ty,
+            ));
         debug!(
             "infer_anon_definition_from_instantiation: definition_ty={:?}",
             definition_ty
@@ -475,7 +479,49 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
 
 struct ReverseMapper<'cx, 'gcx: 'tcx, 'tcx: 'cx> {
     tcx: TyCtxt<'cx, 'gcx, 'tcx>,
-    map: FxHashMap<Kind<'tcx>, Kind<'gcx>>
+
+    /// If errors have already been reported in this fn, we suppress
+    /// our own errors because they are sometimes derivative.
+    tainted_by_errors: bool,
+
+    anon_type_def_id: DefId,
+    map: FxHashMap<Kind<'tcx>, Kind<'gcx>>,
+    map_missing_regions_to_empty: bool,
+
+    /// initially `Some`, set to `None` once error has been reported
+    hidden_ty: Option<Ty<'tcx>>,
+}
+
+impl<'cx, 'gcx, 'tcx> ReverseMapper<'cx, 'gcx, 'tcx> {
+    fn new(
+        tcx: TyCtxt<'cx, 'gcx, 'tcx>,
+        tainted_by_errors: bool,
+        anon_type_def_id: DefId,
+        map: FxHashMap<Kind<'tcx>, Kind<'gcx>>,
+        hidden_ty: Ty<'tcx>,
+    ) -> Self {
+        Self {
+            tcx,
+            tainted_by_errors,
+            anon_type_def_id,
+            map,
+            map_missing_regions_to_empty: false,
+            hidden_ty: Some(hidden_ty),
+        }
+    }
+
+    fn fold_kind_mapping_missing_regions_to_empty(&mut self, kind: Kind<'tcx>) -> Kind<'tcx> {
+        assert!(!self.map_missing_regions_to_empty);
+        self.map_missing_regions_to_empty = true;
+        let kind = kind.fold_with(self);
+        self.map_missing_regions_to_empty = false;
+        kind
+    }
+
+    fn fold_kind_normally(&mut self, kind: Kind<'tcx>) -> Kind<'tcx> {
+        assert!(!self.map_missing_regions_to_empty);
+        kind.fold_with(self)
+    }
 }
 
 impl<'cx, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for ReverseMapper<'cx, 'gcx, 'tcx> {
@@ -484,33 +530,105 @@ impl<'cx, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for ReverseMapper<'cx, 'gcx, 'tcx>
     }
 
     fn fold_region(&mut self, r: ty::Region<'tcx>) -> ty::Region<'tcx> {
-        // ignore bound regions that appear in the type (e.g., this
-        // would ignore `'r` in a type like `for<'r> fn(&'r u32)`.
-        if let ty::ReLateBound(..) = *r {
-            return r;
+        match r {
+            // ignore bound regions that appear in the type (e.g., this
+            // would ignore `'r` in a type like `for<'r> fn(&'r u32)`.
+            ty::ReLateBound(..) => return r,
+
+            // ignore `'static`, as that can appear anywhere
+            ty::ReStatic => return r,
+
+            _ => { }
         }
 
         match self.map.get(&r.into()).map(|k| k.unpack()) {
             Some(UnpackedKind::Lifetime(r1)) => r1,
             Some(u) => panic!("region mapped to unexpected kind: {:?}", u),
             None => {
-                // No mapping was found. This means that it is either a
-                // disallowed lifetime, which will be caught by regionck,
-                // or it is a region in a non-upvar closure generic, which
-                // is explicitly allowed. If that surprises you, read on.
+                if !self.map_missing_regions_to_empty && !self.tainted_by_errors {
+                    if let Some(hidden_ty) = self.hidden_ty.take() {
+                        let span = self.tcx.def_span(self.anon_type_def_id);
+                        let mut err = struct_span_err!(
+                            self.tcx.sess,
+                            span,
+                            E0909,
+                            "hidden type for `impl Trait` captures lifetime that \
+                             does not appear in bounds",
+                        );
+
+                        // Assuming regionck succeeded, then we must
+                        // be capturing *some* region from the fn
+                        // header, and hence it must be free, so it's
+                        // ok to invoke this fn (which doesn't accept
+                        // all regions, and would ICE if an
+                        // inappropriate region is given). We check
+                        // `is_tainted_by_errors` by errors above, so
+                        // we don't get in here unless regionck
+                        // succeeded. (Note also that if regionck
+                        // failed, then the regions we are attempting
+                        // to map here may well be giving errors
+                        // *because* the constraints were not
+                        // satisfiable.)
+                        self.tcx.note_and_explain_free_region(
+                            &mut err,
+                            &format!("hidden type `{}` captures ", hidden_ty),
+                            r,
+                            ""
+                        );
+
+                        err.emit();
+                    }
+                }
+                self.tcx.types.re_empty
+            },
+        }
+    }
+
+    fn fold_ty(&mut self, ty: Ty<'tcx>) -> Ty<'tcx> {
+        match ty.sty {
+            ty::TyClosure(def_id, substs) => {
+                // I am a horrible monster and I pray for death. When
+                // we encounter a closure here, it is always a closure
+                // from within the function that we are currently
+                // type-checking -- one that is now being encapsulated
+                // in an existential abstract type. Ideally, we would
+                // go through the types/lifetimes that it references
+                // and treat them just like we would any other type,
+                // which means we would error out if we find any
+                // reference to a type/region that is not in the
+                // "reverse map".
                 //
-                // The case of closure is a somewhat subtle (read: hacky)
-                // consideration. The problem is that our closure types
-                // currently include all the lifetime parameters declared
-                // on the enclosing function, even if they are unused by
-                // the closure itself. We can't readily filter them out,
+                // **However,** in the case of closures, there is a
+                // somewhat subtle (read: hacky) consideration. The
+                // problem is that our closure types currently include
+                // all the lifetime parameters declared on the
+                // enclosing function, even if they are unused by the
+                // closure itself. We can't readily filter them out,
                 // so here we replace those values with `'empty`. This
                 // can't really make a difference to the rest of the
-                // compiler; those regions are ignored for the outlives
-                // relation, and hence don't affect trait selection or
-                // auto traits, and they are erased during trans.
-                self.tcx.types.re_empty
+                // compiler; those regions are ignored for the
+                // outlives relation, and hence don't affect trait
+                // selection or auto traits, and they are erased
+                // during trans.
+
+                let generics = self.tcx.generics_of(def_id);
+                let parent_len = generics.parent_count();
+                let substs = self.tcx.mk_substs(substs.substs.iter().enumerate().map(
+                    |(index, &kind)| {
+                        if index < parent_len {
+                            // Accommodate missing regions in the parent kinds...
+                            self.fold_kind_mapping_missing_regions_to_empty(kind)
+                        } else {
+                            // ...but not elsewhere.
+                            self.fold_kind_normally(kind)
+                        }
+                    },
+                ));
+
+                self.tcx.mk_closure(def_id, ty::ClosureSubsts { substs })
             }
+
+            _ => ty.super_fold_with(self),
         }
     }
 }
@@ -573,12 +691,13 @@ impl<'a, 'gcx, 'tcx> Instantiator<'a, 'gcx, 'tcx> {
                             return self.fold_anon_ty(ty, def_id, substs);
                         }
 
-                        debug!("instantiate_anon_types_in_map: \
-                                encountered anon with wrong parent \
-                                def_id={:?} \
-                                anon_parent_def_id={:?}",
-                               def_id,
-                               anon_parent_def_id);
+                        debug!(
+                            "instantiate_anon_types_in_map: \
+                             encountered anon with wrong parent \
+                             def_id={:?} \
+                             anon_parent_def_id={:?}",
+                            def_id, anon_parent_def_id
+                        );
                     }
                 }
 
@@ -598,8 +717,7 @@ impl<'a, 'gcx, 'tcx> Instantiator<'a, 'gcx, 'tcx> {
 
         debug!(
             "instantiate_anon_types: TyAnon(def_id={:?}, substs={:?})",
-            def_id,
-            substs
+            def_id, substs
         );
 
         // Use the same type variable if the exact same TyAnon appears more
@@ -608,8 +726,10 @@ impl<'a, 'gcx, 'tcx> Instantiator<'a, 'gcx, 'tcx> {
             return anon_defn.concrete_ty;
         }
         let span = tcx.def_span(def_id);
-        let ty_var = infcx.next_ty_var(ty::UniverseIndex::ROOT,
-                                       TypeVariableOrigin::TypeInference(span));
+        let ty_var = infcx.next_ty_var(
+            ty::UniverseIndex::ROOT,
+            TypeVariableOrigin::TypeInference(span),
+        );
 
         let predicates_of = tcx.predicates_of(def_id);
         let bounds = predicates_of.instantiate(tcx, substs);

src/test/ui/impl-trait/region-escape-via-bound-contravariant.rs

@@ -0,0 +1,35 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// In contrast to `region-escape-via-bound-invariant`, in this case we
+// *can* return a value of type `&'x u32`, even though `'x` does not
+// appear in the bounds. This is because `&` is contravariant, and so
+// we are *actually* returning a `&'y u32`.
+//
+// See https://github.com/rust-lang/rust/issues/46541 for more details.
+
+// run-pass
+
+#![allow(dead_code)]
+#![feature(conservative_impl_trait)]
+#![feature(in_band_lifetimes)]
+#![feature(nll)]
+
+trait Trait<'a> { }
+
+impl Trait<'b> for &'a u32 { }
+
+fn foo(x: &'x u32) -> impl Trait<'y>
+where 'x: 'y
+{
+    x
+}
+
+fn main() { }

src/test/ui/impl-trait/region-escape-via-bound.rs

@@ -0,0 +1,34 @@
+// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// Test that we do not allow the region `'x` to escape in the impl
+// trait **even though** `'y` escapes, which outlives `'x`.
+//
+// See https://github.com/rust-lang/rust/issues/46541 for more details.
+
+#![allow(dead_code)]
+#![feature(conservative_impl_trait)]
+#![feature(in_band_lifetimes)]
+#![feature(nll)]
+
+use std::cell::Cell;
+
+trait Trait<'a> { }
+
+impl Trait<'b> for Cell<&'a u32> { }
+
+fn foo(x: Cell<&'x u32>) -> impl Trait<'y>
+    //~^ ERROR hidden type for `impl Trait` captures lifetime that does not appear in bounds [E0909]
+where 'x: 'y
+{
+    x
+}
+
+fn main() { }