Allow hidden lifetimes in impl Trait, take 2

src/librustc/hir/lowering.rs

@@ -37,8 +37,9 @@ use crate::hir::map::{DefKey, DefPathData, Definitions};
 use crate::hir::def_id::{DefId, DefIndex, DefIndexAddressSpace, CRATE_DEF_INDEX};
 use crate::hir::def::{Def, PathResolution, PerNS};
 use crate::hir::{GenericArg, ConstArg};
-use crate::lint::builtin::{self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
-                    ELIDED_LIFETIMES_IN_PATHS};
+use crate::lint::builtin::{
+    self, PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES, ELIDED_LIFETIMES_IN_PATHS
+};
 use crate::middle::cstore::CrateStore;
 use crate::session::Session;
 use crate::session::config::nightly_options;
@@ -5145,7 +5146,7 @@ impl<'a> LoweringContext<'a> {
     /// `Box<dyn Debug + '_>`. In those cases, `lower_lifetime` is invoked.
     fn elided_dyn_bound(&mut self, span: Span) -> hir::Lifetime {
         match self.anonymous_lifetime_mode {
-            // NB. We intentionally ignore the create-parameter mode here.
+            // N.B., We intentionally ignore the create-parameter mode here.
             // and instead "pass through" to resolve-lifetimes, which will apply
             // the object-lifetime-defaulting rules. Elided object lifetime defaults
             // do not act like other elided lifetimes. In other words, given this:

src/librustc/infer/outlives/env.rs

@@ -197,8 +197,8 @@ impl<'a, 'gcx: 'tcx, 'tcx: 'a> OutlivesEnvironment<'tcx> {
     ) where
         I: IntoIterator<Item = OutlivesBound<'tcx>>,
     {
-        // Record relationships such as `T:'x` that don't go into the
-        // free-region-map but which we use here.
+        // Record relationships such as `T: 'x`, which don't go into the
+        // free-region-map, but which we use here.
         for outlives_bound in outlives_bounds {
             debug!("add_outlives_bounds: outlives_bound={:?}", outlives_bound);
             match outlives_bound {

src/librustc/infer/outlives/free_region_map.rs

@@ -1,4 +1,5 @@
 use crate::ty::{self, Lift, TyCtxt, Region};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::transitive_relation::TransitiveRelation;
 
 #[derive(Clone, RustcEncodable, RustcDecodable, Debug, Default)]
@@ -15,7 +16,7 @@ impl<'tcx> FreeRegionMap<'tcx> {
         self.relation.is_empty()
     }
 
-    // Record that `'sup:'sub`. Or, put another way, `'sub <= 'sup`.
+    // Record that `'sup: 'sub`. Or, put another way, `'sub <= 'sup`.
     // (with the exception that `'static: 'x` is not notable)
     pub fn relate_regions(&mut self, sub: Region<'tcx>, sup: Region<'tcx>) {
         debug!("relate_regions(sub={:?}, sup={:?})", sub, sup);
@@ -24,7 +25,7 @@ impl<'tcx> FreeRegionMap<'tcx> {
         }
     }
 
-    /// Computes the least-upper-bound of two free regions. In some
+    /// Computes the least upper bound of two free regions. In some
     /// cases, this is more conservative than necessary, in order to
     /// avoid making arbitrary choices. See
     /// `TransitiveRelation::postdom_upper_bound` for more details.
@@ -51,12 +52,18 @@ impl<'tcx> FreeRegionMap<'tcx> {
 /// slightly different way; this trait allows functions to be abstract
 /// over which version is in use.
 pub trait FreeRegionRelations<'tcx> {
-    /// Tests whether `r_a <= r_b`. Both must be free regions or
-    /// `'static`.
+    /// Gets all the free regions in the set of relations.
+    fn all_regions(&self) -> FxHashSet<ty::Region<'tcx>>;
+
+    /// Tests whether `r_a <= r_b`. Both must be free regions or `'static`.
     fn sub_free_regions(&self, shorter: ty::Region<'tcx>, longer: ty::Region<'tcx>) -> bool;
 }
 
 impl<'tcx> FreeRegionRelations<'tcx> for FreeRegionMap<'tcx> {
+    fn all_regions(&self) -> FxHashSet<ty::Region<'tcx>> {
+        self.relation.elements().map(|r| *r).collect()
+    }
+
     fn sub_free_regions(&self,
                         r_a: Region<'tcx>,
                         r_b: Region<'tcx>)
@@ -90,6 +97,7 @@ impl_stable_hash_for!(struct FreeRegionMap<'tcx> {
 
 impl<'a, 'tcx> Lift<'tcx> for FreeRegionMap<'a> {
     type Lifted = FreeRegionMap<'tcx>;
+
     fn lift_to_tcx<'b, 'gcx>(&self, tcx: TyCtxt<'b, 'gcx, 'tcx>) -> Option<FreeRegionMap<'tcx>> {
         self.relation.maybe_map(|&fr| tcx.lift(&fr))
                      .map(|relation| FreeRegionMap { relation })

src/librustc/traits/auto_trait.rs

@@ -141,9 +141,9 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
             // selection and projection:
             //
             // * We can always cache the result of a particular trait selection for the lifetime of
-            // an InfCtxt
+            // an `InferCtxt`.
             // * Given a projection bound such as '<T as SomeTrait>::SomeItem = K', if 'T:
-            // SomeTrait' doesn't hold, then we don't need to care about the 'SomeItem = K'
+            // SomeTrait' doesn't hold, then we don't need to care about the 'SomeItem = K'.
             //
             // We fix the first assumption by manually clearing out all of the InferCtxt's caches
             // in between calls to SelectionContext.select. This allows us to keep all of the

src/librustc/traits/query/outlives_bounds.rs

@@ -72,7 +72,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
     /// argument types are well-formed. This may imply certain relationships
     /// between generic parameters. For example:
     ///
-    ///     fn foo<'a,T>(x: &'a T)
+    ///     fn foo<'a, T>(x: &'a T)
     ///
     /// can only be called with a `'a` and `T` such that `&'a T` is WF.
     /// For `&'a T` to be WF, `T: 'a` must hold. So we can assume `T: 'a`.
@@ -102,7 +102,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
             Err(NoSolution) => {
                 self.tcx.sess.delay_span_bug(
                     span,
-                    "implied_outlives_bounds failed to solve all obligations"
+                    "implied_outlives_bounds: failed to solve all obligations",
                 );
                 return vec![];
             }
@@ -117,7 +117,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
             Err(_) => {
                 self.tcx.sess.delay_span_bug(
                     span,
-                    "implied_outlives_bounds failed to instantiate"
+                    "implied_outlives_bounds: failed to instantiate",
                 );
                 return vec![];
             }
@@ -130,7 +130,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
         if fulfill_cx.select_all_or_error(self).is_err() {
             self.tcx.sess.delay_span_bug(
                 span,
-                "implied_outlives_bounds failed to solve obligations from instantiation"
+                "implied_outlives_bounds: failed to solve obligations from instantiation",
             );
         }
 

src/librustc/ty/mod.rs

@@ -1643,7 +1643,7 @@ pub struct ParamEnv<'tcx> {
     pub reveal: traits::Reveal,
 
     /// If this `ParamEnv` comes from a call to `tcx.param_env(def_id)`,
-    /// register that `def_id` (useful for transitioning to the chalk trait
+    /// register that `def_id` (useful for transitioning to the Chalk trait
     /// solver).
     pub def_id: Option<DefId>,
 }

src/librustc/ty/outlives.rs

@@ -1,4 +1,4 @@
-// The outlines relation `T: 'a` or `'a: 'b`. This code frequently
+// The outlives relation; `T: 'a` or `'a: 'b`. This code frequently
 // refers to rules defined in RFC 1214 (`OutlivesFooBar`), so see that
 // RFC for reference.
 

src/librustc/ty/query/mod.rs

@@ -158,7 +158,7 @@ rustc_query_append! { [define_queries!][ <'tcx>
         [] fn is_const_fn_raw: IsConstFn(DefId) -> bool,
 
 
-        /// Returns true if calls to the function may be promoted
+        /// Returns `true` if calls to the function may be promoted.
         ///
         /// This is either because the function is e.g., a tuple-struct or tuple-variant
         /// constructor, or because it has the `#[rustc_promotable]` attribute. The attribute should
@@ -174,22 +174,22 @@ rustc_query_append! { [define_queries!][ <'tcx>
         /// instead.
         [] fn crate_variances: crate_variances(CrateNum) -> Lrc<ty::CrateVariancesMap>,
 
-        /// Maps from def-id of a type or region parameter to its
+        /// Maps from def-ID of a type or region parameter to its
         /// (inferred) variance.
         [] fn variances_of: ItemVariances(DefId) -> Lrc<Vec<ty::Variance>>,
     },
 
     TypeChecking {
-        /// Maps from def-id of a type to its (inferred) outlives.
+        /// Maps from def-ID of a type to its (inferred) outlives.
         [] fn inferred_outlives_crate: InferredOutlivesCrate(CrateNum)
             -> Lrc<ty::CratePredicatesMap<'tcx>>,
     },
 
     Other {
-        /// Maps from an impl/trait def-id to a list of the def-ids of its items
+        /// Maps from an impl/trait def-ID to a list of the def-IDs of its items.
         [] fn associated_item_def_ids: AssociatedItemDefIds(DefId) -> Lrc<Vec<DefId>>,
 
-        /// Maps from a trait item to the trait item "descriptor"
+        /// Maps from a trait item to the trait item "descriptor".
         [] fn associated_item: AssociatedItems(DefId) -> ty::AssociatedItem,
 
         [] fn impl_trait_ref: ImplTraitRef(DefId) -> Option<ty::TraitRef<'tcx>>,

src/librustc_data_structures/transitive_relation.rs

@@ -7,10 +7,9 @@ use std::fmt::Debug;
 use std::hash::Hash;
 use std::mem;
 
-
 #[derive(Clone, Debug)]
 pub struct TransitiveRelation<T: Clone + Debug + Eq + Hash> {
-    // List of elements. This is used to map from a T to a usize.
+    // List of elements. This is used to map from a `T` to a `usize`.
     elements: Vec<T>,
 
     // Maps each element to an index.
@@ -22,7 +21,7 @@ pub struct TransitiveRelation<T: Clone + Debug + Eq + Hash> {
 
     // This is a cached transitive closure derived from the edges.
     // Currently, we build it lazilly and just throw out any existing
-    // copy whenever a new edge is added. (The Lock is to permit
+    // copy whenever a new edge is added. (The `Lock` is to permit
     // the lazy computation.) This is kind of silly, except for the
     // fact its size is tied to `self.elements.len()`, so I wanted to
     // wait before building it up to avoid reallocating as new edges
@@ -54,6 +53,10 @@ struct Edge {
 }
 
 impl<T: Clone + Debug + Eq + Hash> TransitiveRelation<T> {
+    pub fn elements(&self) -> impl Iterator<Item = &T> {
+        self.elements.iter()
+    }
+
     pub fn is_empty(&self) -> bool {
         self.edges.is_empty()
     }
@@ -209,10 +212,10 @@ impl<T: Clone + Debug + Eq + Hash> TransitiveRelation<T> {
             }
         };
 
-        // in some cases, there are some arbitrary choices to be made;
+        // In some cases, there are some arbitrary choices to be made;
         // it doesn't really matter what we pick, as long as we pick
         // the same thing consistently when queried, so ensure that
-        // (a, b) are in a consistent relative order
+        // (a, b) are in a consistent relative order.
         if a > b {
             mem::swap(&mut a, &mut b);
         }

src/librustc_mir/borrow_check/nll/type_check/free_region_relations.rs

@@ -10,6 +10,7 @@ use rustc::mir::ConstraintCategory;
 use rustc::traits::query::outlives_bounds::{self, OutlivesBound};
 use rustc::traits::query::type_op::{self, TypeOp};
 use rustc::ty::{self, RegionVid, Ty};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_data_structures::transitive_relation::TransitiveRelation;
 use std::rc::Rc;
 use syntax_pos::DUMMY_SP;
@@ -358,6 +359,10 @@ impl UniversalRegionRelationsBuilder<'cx, 'gcx, 'tcx> {
 /// over the `FreeRegionMap` from lexical regions and
 /// `UniversalRegions` (from NLL)`.
 impl<'tcx> FreeRegionRelations<'tcx> for UniversalRegionRelations<'tcx> {
+    fn all_regions(&self) -> FxHashSet<ty::Region<'tcx>> {
+        self.universal_regions.regions().map(|r| *r).collect()
+    }
+
     fn sub_free_regions(&self, shorter: ty::Region<'tcx>, longer: ty::Region<'tcx>) -> bool {
         let shorter = shorter.to_region_vid();
         assert!(self.universal_regions.is_universal_region(shorter));

src/librustc_mir/borrow_check/nll/universal_regions.rs

@@ -208,6 +208,10 @@ impl<'tcx> UniversalRegions<'tcx> {
         }.build()
     }
 
+    pub fn regions(&self) -> impl Iterator<Item = &ty::Region<'tcx>> {
+        self.indices.indices.keys()
+    }
+
     /// Given a reference to a closure type, extracts all the values
     /// from its free regions and returns a vector with them. This is
     /// used when the closure's creator checks that the

src/librustc_typeck/check/regionck.rs

@@ -374,7 +374,7 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> {
     fn visit_region_obligations(&mut self, hir_id: hir::HirId) {
         debug!("visit_region_obligations: hir_id={:?}", hir_id);
 
-        // region checking can introduce new pending obligations
+        // Region checking can introduce new pending obligations,
         // which, when processed, might generate new region
         // obligations. So make sure we process those.
         self.select_all_obligations_or_error();
@@ -402,7 +402,7 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> {
             // data will be accessible from anywhere that the variable is
             // accessed. We must be wary of loops like this:
             //
-            //     // from src/test/compile-fail/borrowck-lend-flow.rs
+            //     // from `src/test/compile-fail/borrowck-lend-flow.rs`
             //     let mut v = box 3, w = box 4;
             //     let mut x = &mut w;
             //     loop {

src/test/ui/impl-trait/issue-55608-captures-empty-region.rs

@@ -1,9 +1,9 @@
+// run-pass
+
 // This used to ICE because it creates an `impl Trait` that captures a
 // hidden empty region.
 
-#![feature(conservative_impl_trait)]
-
-fn server() -> impl FilterBase2 { //~ ERROR [E0700]
+fn server() -> impl FilterBase2 {
     segment2(|| { loop { } }).map2(|| "")
 }
 

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

@@ -1,4 +1,4 @@
-// In contrast to `region-escape-via-bound-invariant`, in this case we
+// In contrast to `region-escape-via-bound.rs`, 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`.

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

@@ -1,5 +1,7 @@
-// Test that we do not allow the region `'x` to escape in the impl
-// trait **even though** `'y` escapes, which outlives `'x`.
+// run-pass
+
+// Test that we allow the region `'x` to escape in the impl
+// because 'y` escapes, which outlives `'x`.
 //
 // See https://github.com/rust-lang/rust/issues/46541 for more details.
 
@@ -14,10 +16,14 @@ 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 [E0700]
+    // ^ hidden type for `impl Trait` captures lifetime that does not appear in bounds
+    //   because it outlives the lifetime that *does* appear in the bounds, `'y`
 where 'x: 'y
 {
     x
 }
 
-fn main() { }
+fn main() {
+    let x = 123;
+    let _ = foo(Cell::new(&x));
+}

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

@@ -0,0 +1,29 @@
+// compile-fail
+
+// See https://github.com/rust-lang/rust/pull/57870#issuecomment-457333709 for more details.
+
+trait Swap: Sized {
+    fn swap(self, other: Self);
+}
+
+impl<T> Swap for &mut T {
+    fn swap(self, other: Self) {
+        std::mem::swap(self, other);
+    }
+}
+
+// The hidden relifetimegion `'b` should not be allowed to escape this function, since it may not
+// outlive '`a`, in which case we have a dangling reference.
+fn hide<'a, 'b, T: 'static>(x: &'a mut &'b T) -> impl Swap + 'a {
+//~^ lifetime mismatch [E0623]
+    x
+}
+
+fn dangle() -> &'static [i32; 3] {
+    let mut res = &[4, 5, 6];
+    let x = [1, 2, 3];
+    hide(&mut res).swap(hide(&mut &x));
+    res
+}
+
+fn main() {}

src/test/ui/impl-trait/region-escape-via-swap.stderr

@@ -0,0 +1,12 @@
+error[E0623]: lifetime mismatch
+  --> $DIR/region-escape-via-swap.rs:17:50
+   |
+LL | fn hide<'a, 'b, T: 'static>(x: &'a mut &'b T) -> impl Swap + 'a {
+   |                                        -----     ^^^^^^^^^^^^^^
+   |                                        |         |
+   |                                        |         ...but data from `x` is returned here
+   |                                        this parameter and the return type are declared with different lifetimes...
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0623`.