From cd564d20ff521d56ad3f01fe0e0588d9487c0c13 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Sun, 3 Dec 2017 06:16:52 -0500 Subject: [PATCH] only propagate ClosureRegionRequirements if non-trivial Before, we would always have a `Some` ClosureRegionRequirements if we were inferring values for a closure. Now we only do is it has a non-empty set of outlives requirements. --- .../borrow_check/nll/region_infer/mod.rs | 173 ++++++++++-------- .../escape-argument-callee.stderr | 3 +- .../escape-argument.stderr | 3 +- ...pagate-approximated-fail-no-postdom.stderr | 3 +- ...er-to-static-comparing-against-free.stderr | 3 +- .../propagate-approximated-to-empty.stderr | 45 +++++ ...ail-to-approximate-longer-no-bounds.stderr | 7 +- ...-to-approximate-longer-wrong-bounds.stderr | 7 +- .../return-wrong-bound-region.stderr | 3 +- 9 files changed, 148 insertions(+), 99 deletions(-) create mode 100644 src/test/ui/nll/closure-requirements/propagate-approximated-to-empty.stderr diff --git a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs index 6d76398b941e..af88edc22cee 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/mod.rs @@ -251,24 +251,88 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir_def_id: DefId, ) -> Option { assert!(self.inferred_values.is_none(), "values already inferred"); - let tcx = infcx.tcx; - // Find the minimal regions that can solve the constraints. This is infallible. self.propagate_constraints(mir); - // Now, see whether any of the constraints were too strong. In - // particular, we want to check for a case where a universally - // quantified region exceeded its bounds. Consider: - // - // fn foo<'a, 'b>(x: &'a u32) -> &'b u32 { x } - // - // In this case, returning `x` requires `&'a u32 <: &'b u32` - // and hence we establish (transitively) a constraint that - // `'a: 'b`. The `propagate_constraints` code above will - // therefore add `end('a)` into the region for `'b` -- but we - // have no evidence that `'a` outlives `'b`, so we want to report - // an error. + let outlives_requirements = self.check_universal_regions(infcx, mir_def_id); + if outlives_requirements.is_empty() { + None + } else { + let num_external_vids = self.universal_regions.num_global_and_external_regions(); + Some(ClosureRegionRequirements { + num_external_vids, + outlives_requirements, + }) + } + } + + /// Propagate the region constraints: this will grow the values + /// for each region variable until all the constraints are + /// satisfied. Note that some values may grow **too** large to be + /// feasible, but we check this later. + fn propagate_constraints(&mut self, mir: &Mir<'tcx>) { + let mut changed = true; + + debug!("propagate_constraints()"); + debug!("propagate_constraints: constraints={:#?}", { + let mut constraints: Vec<_> = self.constraints.iter().collect(); + constraints.sort(); + constraints + }); + + // The initial values for each region are derived from the liveness + // constraints we have accumulated. + let mut inferred_values = self.liveness_constraints.clone(); + + while changed { + changed = false; + debug!("propagate_constraints: --------------------"); + for constraint in &self.constraints { + debug!("propagate_constraints: constraint={:?}", constraint); + + // Grow the value as needed to accommodate the + // outlives constraint. + let Ok(made_changes) = self.dfs( + mir, + CopyFromSourceToTarget { + source_region: constraint.sub, + target_region: constraint.sup, + inferred_values: &mut inferred_values, + constraint_point: constraint.point, + }, + ); + + if made_changes { + debug!("propagate_constraints: sub={:?}", constraint.sub); + debug!("propagate_constraints: sup={:?}", constraint.sup); + changed = true; + } + } + debug!("\n"); + } + + self.inferred_values = Some(inferred_values); + } + + /// Once regions have been propagated, this method is used to see + /// whether any of the constraints were too strong. In particular, + /// we want to check for a case where a universally quantified + /// region exceeded its bounds. Consider: + /// + /// fn foo<'a, 'b>(x: &'a u32) -> &'b u32 { x } + /// + /// In this case, returning `x` requires `&'a u32 <: &'b u32` + /// and hence we establish (transitively) a constraint that + /// `'a: 'b`. The `propagate_constraints` code above will + /// therefore add `end('a)` into the region for `'b` -- but we + /// have no evidence that `'b` outlives `'a`, so we want to report + /// an error. + fn check_universal_regions( + &self, + infcx: &InferCtxt<'_, '_, 'tcx>, + mir_def_id: DefId, + ) -> Vec { // The universal regions are always found in a prefix of the // full list. let universal_definitions = self.definitions @@ -283,27 +347,23 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.check_universal_region(infcx, fr, &mut outlives_requirements); } - // If this is not a closure, then there is no caller to which we can - // "pass the buck". So if there are any outlives-requirements that were - // not satisfied, we just have to report a hard error here. - if !tcx.is_closure(mir_def_id) { - for outlives_requirement in outlives_requirements { - self.report_error( - infcx, - outlives_requirement.free_region, - outlives_requirement.outlived_free_region, - outlives_requirement.blame_span, - ); - } - return None; + // If this is a closure, we can propagate unsatisfied + // `outlives_requirements` to our creator. Otherwise, we have + // to report a hard error here. + if infcx.tcx.is_closure(mir_def_id) { + return outlives_requirements; } - let num_external_vids = self.universal_regions.num_global_and_external_regions(); + for outlives_requirement in outlives_requirements { + self.report_error( + infcx, + outlives_requirement.free_region, + outlives_requirement.outlived_free_region, + outlives_requirement.blame_span, + ); + } - Some(ClosureRegionRequirements { - num_external_vids, - outlives_requirements, - }) + vec![] } /// Check the final value for the free region `fr` to see if it @@ -396,54 +456,6 @@ impl<'tcx> RegionInferenceContext<'tcx> { ); } - /// Propagate the region constraints: this will grow the values - /// for each region variable until all the constraints are - /// satisfied. Note that some values may grow **too** large to be - /// feasible, but we check this later. - fn propagate_constraints(&mut self, mir: &Mir<'tcx>) { - let mut changed = true; - - debug!("propagate_constraints()"); - debug!("propagate_constraints: constraints={:#?}", { - let mut constraints: Vec<_> = self.constraints.iter().collect(); - constraints.sort(); - constraints - }); - - // The initial values for each region are derived from the liveness - // constraints we have accumulated. - let mut inferred_values = self.liveness_constraints.clone(); - - while changed { - changed = false; - debug!("propagate_constraints: --------------------"); - for constraint in &self.constraints { - debug!("propagate_constraints: constraint={:?}", constraint); - - // Grow the value as needed to accommodate the - // outlives constraint. - let Ok(made_changes) = self.dfs( - mir, - CopyFromSourceToTarget { - source_region: constraint.sub, - target_region: constraint.sup, - inferred_values: &mut inferred_values, - constraint_point: constraint.point, - }, - ); - - if made_changes { - debug!("propagate_constraints: sub={:?}", constraint.sub); - debug!("propagate_constraints: sup={:?}", constraint.sup); - changed = true; - } - } - debug!("\n"); - } - - self.inferred_values = Some(inferred_values); - } - /// Tries to finds a good span to blame for the fact that `fr1` /// contains `fr2`. fn blame_span(&self, fr1: RegionVid, fr2: RegionVid) -> Span { @@ -589,4 +601,3 @@ impl ClosureRegionRequirementsExt for ClosureRegionRequirements { } } } - diff --git a/src/test/ui/nll/closure-requirements/escape-argument-callee.stderr b/src/test/ui/nll/closure-requirements/escape-argument-callee.stderr index c842d51a2ad7..2dfafd8f1725 100644 --- a/src/test/ui/nll/closure-requirements/escape-argument-callee.stderr +++ b/src/test/ui/nll/closure-requirements/escape-argument-callee.stderr @@ -10,7 +10,7 @@ error: free region `'_#4r` does not outlive free region `'_#3r` 36 | let mut closure = expect_sig(|p, y| *p = y); | ^^^^^^ -note: External requirements +note: No external requirements --> $DIR/escape-argument-callee.rs:36:38 | 36 | let mut closure = expect_sig(|p, y| *p = y); @@ -20,7 +20,6 @@ note: External requirements i16, for<'r, 's, 't0> extern "rust-call" fn((&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) mut &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) i32, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't0)) i32)) ] - = note: number of external vids: 1 note: No external requirements --> $DIR/escape-argument-callee.rs:30:1 diff --git a/src/test/ui/nll/closure-requirements/escape-argument.stderr b/src/test/ui/nll/closure-requirements/escape-argument.stderr index 0ec671997e7a..567ed299ac15 100644 --- a/src/test/ui/nll/closure-requirements/escape-argument.stderr +++ b/src/test/ui/nll/closure-requirements/escape-argument.stderr @@ -1,4 +1,4 @@ -note: External requirements +note: No external requirements --> $DIR/escape-argument.rs:36:38 | 36 | let mut closure = expect_sig(|p, y| *p = y); @@ -8,7 +8,6 @@ note: External requirements i16, for<'r, 's> extern "rust-call" fn((&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) mut &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) i32, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) i32)) ] - = note: number of external vids: 1 note: No external requirements --> $DIR/escape-argument.rs:30:1 diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-fail-no-postdom.stderr b/src/test/ui/nll/closure-requirements/propagate-approximated-fail-no-postdom.stderr index d581622c4c63..cdda8ab5392b 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-fail-no-postdom.stderr +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-fail-no-postdom.stderr @@ -10,7 +10,7 @@ error: free region `'_#5r` does not outlive free region `'_#6r` 57 | demand_y(x, y, p) | ^ -note: External requirements +note: No external requirements --> $DIR/propagate-approximated-fail-no-postdom.rs:53:9 | 53 | / |_outlives1, _outlives2, _outlives3, x, y| { @@ -26,7 +26,6 @@ note: External requirements i16, for<'r, 's> extern "rust-call" fn((std::cell::Cell<&'_#1r &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&'_#2r &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) &'_#3r u32>, std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) u32>, std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) u32>)) ] - = note: number of external vids: 4 note: No external requirements --> $DIR/propagate-approximated-fail-no-postdom.rs:48:1 diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.stderr b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.stderr index b93c69dc13f4..3e54e62d0110 100644 --- a/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.stderr +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-shorter-to-static-comparing-against-free.stderr @@ -10,7 +10,7 @@ error: free region `'_#2r` does not outlive free region `'_#1r` 33 | cell_a.set(cell_x.get()); // forces 'x: 'a, error in closure | ^^^^^^ -note: External requirements +note: No external requirements --> $DIR/propagate-approximated-shorter-to-static-comparing-against-free.rs:31:15 | 31 | foo(cell, |cell_a, cell_x| { @@ -25,7 +25,6 @@ note: External requirements i32, for<'r> extern "rust-call" fn((std::cell::Cell<&'_#1r u32>, std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) u32>)) ] - = note: number of external vids: 2 note: No external requirements --> $DIR/propagate-approximated-shorter-to-static-comparing-against-free.rs:28:1 diff --git a/src/test/ui/nll/closure-requirements/propagate-approximated-to-empty.stderr b/src/test/ui/nll/closure-requirements/propagate-approximated-to-empty.stderr new file mode 100644 index 000000000000..e8dc8a13f876 --- /dev/null +++ b/src/test/ui/nll/closure-requirements/propagate-approximated-to-empty.stderr @@ -0,0 +1,45 @@ +warning: not reporting region error due to -Znll + --> $DIR/propagate-approximated-to-empty.rs:41:9 + | +41 | demand_y(x, y, x.get()) + | ^^^^^^^^^^^^^^^^^^^^^^^ + +error: free region `'_#6r` does not outlive free region `'_#4r` + --> $DIR/propagate-approximated-to-empty.rs:41:21 + | +41 | demand_y(x, y, x.get()) + | ^ + +note: No external requirements + --> $DIR/propagate-approximated-to-empty.rs:39:47 + | +39 | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| { + | _______________________________________________^ +40 | | // Only works if 'x: 'y: +41 | | demand_y(x, y, x.get()) +42 | | //~^ WARN not reporting region error due to -Znll +43 | | //~| ERROR free region `'_#6r` does not outlive free region `'_#4r` +44 | | }); + | |_____^ + | + = note: defining type: DefId(0/1:18 ~ propagate_approximated_to_empty[317d]::supply[0]::{{closure}}[0]) with closure substs [ + i16, + for<'r, 's, 't0, 't1, 't2> extern "rust-call" fn((&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) &'_#1r u32>, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't1)) u32>, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't2)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) u32>)) + ] + +note: No external requirements + --> $DIR/propagate-approximated-to-empty.rs:38:1 + | +38 | / fn supply<'a, 'b>(cell_a: Cell<&'a u32>, cell_b: Cell<&'b u32>) { +39 | | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| { +40 | | // Only works if 'x: 'y: +41 | | demand_y(x, y, x.get()) +... | +44 | | }); +45 | | } + | |_^ + | + = note: defining type: DefId(0/0:6 ~ propagate_approximated_to_empty[317d]::supply[0]) with substs [] + +error: aborting due to previous error + diff --git a/src/test/ui/nll/closure-requirements/propagate-fail-to-approximate-longer-no-bounds.stderr b/src/test/ui/nll/closure-requirements/propagate-fail-to-approximate-longer-no-bounds.stderr index 6094f9aad81d..f3c40c838fb3 100644 --- a/src/test/ui/nll/closure-requirements/propagate-fail-to-approximate-longer-no-bounds.stderr +++ b/src/test/ui/nll/closure-requirements/propagate-fail-to-approximate-longer-no-bounds.stderr @@ -5,12 +5,12 @@ warning: not reporting region error due to -Znll | ^^^^^^^^^^^^^^^^^^^^^^^ error: free region `'_#6r` does not outlive free region `'_#4r` - --> $DIR/propagate-fail-to-approximate-longer-no-bounds.rs:47:21 + --> $DIR/propagate-fail-to-approximate-longer-no-bounds.rs:47:18 | 47 | demand_y(x, y, x.get()) - | ^ + | ^ -note: External requirements +note: No external requirements --> $DIR/propagate-fail-to-approximate-longer-no-bounds.rs:45:47 | 45 | establish_relationships(&cell_a, &cell_b, |_outlives, x, y| { @@ -26,7 +26,6 @@ note: External requirements i16, for<'r, 's, 't0, 't1, 't2> extern "rust-call" fn((&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) &'_#1r u32>, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't1)) u32>, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't2)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) u32>)) ] - = note: number of external vids: 2 note: No external requirements --> $DIR/propagate-fail-to-approximate-longer-no-bounds.rs:44:1 diff --git a/src/test/ui/nll/closure-requirements/propagate-fail-to-approximate-longer-wrong-bounds.stderr b/src/test/ui/nll/closure-requirements/propagate-fail-to-approximate-longer-wrong-bounds.stderr index 6658ee63abd9..a66c2a789702 100644 --- a/src/test/ui/nll/closure-requirements/propagate-fail-to-approximate-longer-wrong-bounds.stderr +++ b/src/test/ui/nll/closure-requirements/propagate-fail-to-approximate-longer-wrong-bounds.stderr @@ -5,12 +5,12 @@ warning: not reporting region error due to -Znll | ^^^^^^^^^^^^^^^^^^^^^^^ error: free region `'_#5r` does not outlive free region `'_#7r` - --> $DIR/propagate-fail-to-approximate-longer-wrong-bounds.rs:51:21 + --> $DIR/propagate-fail-to-approximate-longer-wrong-bounds.rs:51:18 | 51 | demand_y(x, y, x.get()) - | ^ + | ^ -note: External requirements +note: No external requirements --> $DIR/propagate-fail-to-approximate-longer-wrong-bounds.rs:49:47 | 49 | establish_relationships(&cell_a, &cell_b, |_outlives1, _outlives2, x, y| { @@ -26,7 +26,6 @@ note: External requirements i16, for<'r, 's, 't0, 't1, 't2, 't3> extern "rust-call" fn((&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) &'_#1r u32>, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't0)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't1)) &'_#2r u32>, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't2)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) u32>, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't3)) std::cell::Cell<&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 't1)) u32>)) ] - = note: number of external vids: 3 note: No external requirements --> $DIR/propagate-fail-to-approximate-longer-wrong-bounds.rs:48:1 diff --git a/src/test/ui/nll/closure-requirements/return-wrong-bound-region.stderr b/src/test/ui/nll/closure-requirements/return-wrong-bound-region.stderr index 8999f69e8ded..cb2b2e2f1186 100644 --- a/src/test/ui/nll/closure-requirements/return-wrong-bound-region.stderr +++ b/src/test/ui/nll/closure-requirements/return-wrong-bound-region.stderr @@ -10,7 +10,7 @@ error: free region `'_#3r` does not outlive free region `'_#2r` 21 | expect_sig(|a, b| b); // ought to return `a` | ^ -note: External requirements +note: No external requirements --> $DIR/return-wrong-bound-region.rs:21:16 | 21 | expect_sig(|a, b| b); // ought to return `a` @@ -20,7 +20,6 @@ note: External requirements i16, for<'r, 's> extern "rust-call" fn((&ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) i32, &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 's)) i32)) -> &ReLateBound(DebruijnIndex { depth: 1 }, BrNamed(crate0:DefIndex(0:0), 'r)) i32 ] - = note: number of external vids: 1 note: No external requirements --> $DIR/return-wrong-bound-region.rs:20:1