From 7f2c82f8f36acfccab7f37535ba1549e81409968 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 7 Nov 2017 13:18:42 -0500 Subject: [PATCH] leak the affects of closures on the free-region-map, like we used to This restores the behavior of regionck with respect to the free-region-map: that is, it collects all the relations from the fn and its closures. This feels a bit fishy but it's the behavior we've had for some time, and it will go away with NLL, so seems best to just keep it. --- src/librustc/infer/outlives/env.rs | 46 +++++++++++++++++++ src/librustc_typeck/check/regionck.rs | 4 +- .../implied-bounds-closure-arg-outlives.rs | 44 ++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 src/test/run-pass/implied-bounds-closure-arg-outlives.rs diff --git a/src/librustc/infer/outlives/env.rs b/src/librustc/infer/outlives/env.rs index ef09479f75131..2099e923e0959 100644 --- a/src/librustc/infer/outlives/env.rs +++ b/src/librustc/infer/outlives/env.rs @@ -89,6 +89,52 @@ impl<'a, 'gcx: 'tcx, 'tcx: 'a> OutlivesEnvironment<'tcx> { self.free_region_map } + /// This is a hack to support the old-skool regionck, which + /// processes region constraints from the main function and the + /// closure together. In that context, when we enter a closure, we + /// want to be able to "save" the state of the surrounding a + /// function. We can then add implied bounds and the like from the + /// closure arguments into the environment -- these should only + /// apply in the closure body, so once we exit, we invoke + /// `pop_snapshot_post_closure` to remove them. + /// + /// Example: + /// + /// ``` + /// fn foo() { + /// callback(for<'a> |x: &'a T| { + /// // ^^^^^^^ not legal syntax, but probably should be + /// // within this closure body, `T: 'a` holds + /// }) + /// } + /// ``` + /// + /// This "containment" of closure's effects only works so well. In + /// particular, we (intentionally) leak relationships between free + /// regions that are created by the closure's bounds. The case + /// where this is useful is when you have (e.g.) a closure with a + /// signature like `for<'a, 'b> fn(x: &'a &'b u32)` -- in this + /// case, we want to keep the relationship `'b: 'a` in the + /// free-region-map, so that later if we have to take `LUB('b, + /// 'a)` we can get the result `'b`. + /// + /// I have opted to keep **all modifications** to the + /// free-region-map, however, and not just those that concern free + /// variables bound in the closure. The latter seems more correct, + /// but it is not the existing behavior, and I could not find a + /// case where the existing behavior went wrong. In any case, it + /// seems like it'd be readily fixed if we wanted. There are + /// similar leaks around givens that seem equally suspicious, to + /// be honest. --nmatsakis + pub fn push_snapshot_pre_closure(&self) -> usize { + self.region_bound_pairs.len() + } + + /// See `push_snapshot_pre_closure`. + pub fn pop_snapshot_post_closure(&mut self, len: usize) { + self.region_bound_pairs.truncate(len); + } + /// This method adds "implied bounds" into the outlives environment. /// Implied bounds are outlives relationships that we can deduce /// on the basis that certain types must be well-formed -- these are diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 932cb12e81dfb..a17133d412c7a 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -428,17 +428,17 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for RegionCtxt<'a, 'gcx, 'tcx> { // Save state of current function before invoking // `visit_fn_body`. We will restore afterwards. - let outlives_environment = self.outlives_environment.clone(); let old_body_id = self.body_id; let old_call_site_scope = self.call_site_scope; + let env_snapshot = self.outlives_environment.push_snapshot_pre_closure(); let body = self.tcx.hir.body(body_id); self.visit_fn_body(id, body, span); // Restore state from previous function. + self.outlives_environment.pop_snapshot_post_closure(env_snapshot); self.call_site_scope = old_call_site_scope; self.body_id = old_body_id; - self.outlives_environment = outlives_environment; } //visit_pat: visit_pat, // (..) see above diff --git a/src/test/run-pass/implied-bounds-closure-arg-outlives.rs b/src/test/run-pass/implied-bounds-closure-arg-outlives.rs new file mode 100644 index 0000000000000..0e5cc574f0022 --- /dev/null +++ b/src/test/run-pass/implied-bounds-closure-arg-outlives.rs @@ -0,0 +1,44 @@ +// 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that we are able to handle the relationships between free +// regions bound in a closure callback. + +#[derive(Copy, Clone)] +struct MyCx<'short, 'long: 'short> { + short: &'short u32, + long: &'long u32, +} + +impl<'short, 'long> MyCx<'short, 'long> { + fn short(self) -> &'short u32 { self.short } + fn long(self) -> &'long u32 { self.long } + fn set_short(&mut self, v: &'short u32) { self.short = v; } +} + +fn with(op: F) -> R +where + F: for<'short, 'long> FnOnce(MyCx<'short, 'long>) -> R, +{ + op(MyCx { + short: &22, + long: &22, + }) +} + +fn main() { + with(|mut cx| { + // For this to type-check, we need to be able to deduce that + // the lifetime of `l` can be `'short`, even though it has + // input from `'long`. + let l = if true { cx.long() } else { cx.short() }; + cx.set_short(l); + }); +}