From fdcd2256f00886b0b9088570040520f800aab2d7 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 31 Oct 2017 13:21:58 -0400 Subject: [PATCH] trace span info for constraints and report errors --- .../transform/nll/constraint_generation.rs | 11 +- .../transform/nll/region_infer.rs | 119 ++++++++++++++---- src/test/compile-fail/nll/region-error.rs | 5 - src/test/ui/nll/named-region-basic.rs | 22 ++++ src/test/ui/nll/named-region-basic.stderr | 29 +++++ 5 files changed, 155 insertions(+), 31 deletions(-) delete mode 100644 src/test/compile-fail/nll/region-error.rs create mode 100644 src/test/ui/nll/named-region-basic.rs create mode 100644 src/test/ui/nll/named-region-basic.stderr diff --git a/src/librustc_mir/transform/nll/constraint_generation.rs b/src/librustc_mir/transform/nll/constraint_generation.rs index f077dd4d1240..a2f9bbb174eb 100644 --- a/src/librustc_mir/transform/nll/constraint_generation.rs +++ b/src/librustc_mir/transform/nll/constraint_generation.rs @@ -191,6 +191,7 @@ impl<'cx, 'gcx, 'tcx> ConstraintGeneration<'cx, 'gcx, 'tcx> { _borrowed_lv: &Lvalue<'tcx>, ) { let tcx = self.infcx.tcx; + let span = self.mir.source_info(location).span; let destination_ty = destination_lv.ty(self.mir, tcx).to_ty(tcx); let destination_region = match destination_ty.sty { @@ -198,7 +199,8 @@ impl<'cx, 'gcx, 'tcx> ConstraintGeneration<'cx, 'gcx, 'tcx> { _ => bug!() }; - self.regioncx.add_outlives(borrow_region.to_region_index(), + self.regioncx.add_outlives(span, + borrow_region.to_region_index(), destination_region.to_region_index(), location.successor_within_block()); } @@ -226,7 +228,9 @@ impl<'cx, 'gcx, 'tcx> ConstraintGeneration<'cx, 'gcx, 'tcx> { }, } - self.regioncx.add_outlives(base_region.to_region_index(), + let span = self.mir.source_info(location).span; + self.regioncx.add_outlives(span, + base_region.to_region_index(), borrow_region.to_region_index(), location.successor_within_block()); } @@ -259,8 +263,9 @@ impl<'cx, 'gcx, 'tcx> Visitor<'tcx> for ConstraintGeneration<'cx, 'gcx, 'tcx> { let destination_ty = destination_lv.ty(self.mir, tcx).to_ty(tcx); let rv_ty = rv.ty(self.mir, tcx); + let span = self.mir.source_info(location).span; for (a, b) in subtype::outlives_pairs(tcx, rv_ty, destination_ty) { - self.regioncx.add_outlives(a, b, location.successor_within_block()); + self.regioncx.add_outlives(span, a, b, location.successor_within_block()); } } diff --git a/src/librustc_mir/transform/nll/region_infer.rs b/src/librustc_mir/transform/nll/region_infer.rs index f4a7682e9d62..553d5ad4a320 100644 --- a/src/librustc_mir/transform/nll/region_infer.rs +++ b/src/librustc_mir/transform/nll/region_infer.rs @@ -17,6 +17,7 @@ use rustc_data_structures::indexed_vec::{Idx, IndexVec}; use rustc_data_structures::fx::FxHashSet; use std::collections::BTreeSet; use std::fmt; +use syntax_pos::Span; pub struct RegionInferenceContext<'tcx> { /// Contains the definition for every region variable. Region @@ -70,10 +71,11 @@ struct Region { impl fmt::Debug for Region { fn fmt(&self, formatter: &mut fmt::Formatter) -> Result<(), fmt::Error> { - formatter.debug_set() - .entries(&self.points) - .entries(&self.free_regions) - .finish() + formatter + .debug_set() + .entries(&self.points) + .entries(&self.free_regions) + .finish() } } @@ -93,8 +95,16 @@ impl Region { #[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)] pub struct Constraint { - sub: RegionIndex, + /// Where did this constraint arise? + span: Span, + + /// The region SUP must outlive SUB... sup: RegionIndex, + + /// Region that must be outlived. + sub: RegionIndex, + + /// At this location. point: Location, } @@ -210,56 +220,116 @@ impl<'a, 'gcx, 'tcx> RegionInferenceContext<'tcx> { pub(super) fn add_live_point(&mut self, v: RegionIndex, point: Location) { debug!("add_live_point({:?}, {:?})", v, point); let definition = &mut self.definitions[v]; - definition.value.add_point(point); + if !definition.constant { + definition.value.add_point(point); + } else { + // Constants are used for free regions, which already + // contain all the points in the control-flow graph. + assert!(definition.value.contains_point(point)); + } } /// Indicates that the region variable `sup` must outlive `sub` is live at the point `point`. - pub(super) fn add_outlives(&mut self, sup: RegionIndex, sub: RegionIndex, point: Location) { + pub(super) fn add_outlives( + &mut self, + span: Span, + sup: RegionIndex, + sub: RegionIndex, + point: Location, + ) { debug!("add_outlives({:?}: {:?} @ {:?}", sup, sub, point); - self.constraints.push(Constraint { sup, sub, point }); + self.constraints.push(Constraint { + span, + sup, + sub, + point, + }); } /// Perform region inference. pub(super) fn solve(&mut self, infcx: &InferCtxt<'a, 'gcx, 'tcx>, mir: &Mir<'tcx>) { - self.propagate_constraints(infcx, mir); + let errors = self.propagate_constraints(mir); + + // worst error msg ever + for (fr1, span, fr2) in errors { + infcx.tcx.sess.span_err( + span, + &format!( + "free region `{}` does not outlive `{}`", + self.definitions[fr1].name.unwrap(), + self.definitions[fr2].name.unwrap() + ), + ); + } } /// 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, infcx: &InferCtxt<'a, 'gcx, 'tcx>, mir: &Mir<'tcx>) { + fn propagate_constraints( + &mut self, + mir: &Mir<'tcx>, + ) -> Vec<(RegionIndex, Span, RegionIndex)> { let mut changed = true; - let mut dfs = Dfs::new(infcx, mir); + let mut dfs = Dfs::new(mir); + let mut error_regions = FxHashSet(); + let mut errors = vec![]; while changed { changed = false; for constraint in &self.constraints { + debug!("constraint: {:?}", constraint); let sub = &self.definitions[constraint.sub].value.clone(); let sup_def = &mut self.definitions[constraint.sup]; - debug!("constraint: {:?}", constraint); + debug!(" sub (before): {:?}", sub); debug!(" sup (before): {:?}", sup_def.value); - if dfs.copy(sub, &mut sup_def.value, constraint.point) { - changed = true; - } + if !sup_def.constant { + // If this is not a constant, then grow the value as needed to + // accommodate the outlives constraint. - debug!(" sup (after) : {:?}", sup_def.value); - debug!(" changed : {:?}", changed); + if dfs.copy(sub, &mut sup_def.value, constraint.point) { + changed = true; + } + + debug!(" sup (after) : {:?}", sup_def.value); + debug!(" changed : {:?}", changed); + } else { + // If this is a constant, check whether it *would + // have* to grow in order for the constraint to be + // satisfied. If so, create an error. + + let mut sup_value = sup_def.value.clone(); + if dfs.copy(sub, &mut sup_value, constraint.point) { + // Constant values start out with the entire + // CFG, so it must be some new free region + // that was added. Find one. + let &new_region = sup_value + .free_regions + .difference(&sup_def.value.free_regions) + .next() + .unwrap(); + debug!(" new_region : {:?}", new_region); + if error_regions.insert(constraint.sup) { + errors.push((constraint.sup, constraint.span, new_region)); + } + } + } } debug!("\n"); } + errors } } -struct Dfs<'a, 'gcx: 'tcx + 'a, 'tcx: 'a> { - #[allow(dead_code)] infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, +struct Dfs<'a, 'tcx: 'a> { mir: &'a Mir<'tcx>, } -impl<'a, 'gcx: 'tcx, 'tcx: 'a> Dfs<'a, 'gcx, 'tcx> { - fn new(infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, mir: &'a Mir<'tcx>) -> Self { - Self { infcx, mir } +impl<'a, 'tcx> Dfs<'a, 'tcx> { + fn new(mir: &'a Mir<'tcx>) -> Self { + Self { mir } } fn copy( @@ -316,7 +386,10 @@ impl<'a, 'gcx: 'tcx, 'tcx: 'a> Dfs<'a, 'gcx, 'tcx> { // over any skolemized end points in the `from_region` // and make sure they are included in the `to_region`. - to_region.free_regions.extend(&from_region.free_regions); + debug!(" dfs: free_regions={:?}", from_region.free_regions); + for &fr in &from_region.free_regions { + changed |= to_region.free_regions.insert(fr); + } } else { stack.extend(successor_points); } diff --git a/src/test/compile-fail/nll/region-error.rs b/src/test/compile-fail/nll/region-error.rs deleted file mode 100644 index 4a3e838beb34..000000000000 --- a/src/test/compile-fail/nll/region-error.rs +++ /dev/null @@ -1,5 +0,0 @@ -fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> &'b u32 { - &*x -} - -fn main() { } diff --git a/src/test/ui/nll/named-region-basic.rs b/src/test/ui/nll/named-region-basic.rs new file mode 100644 index 000000000000..539c2017ea6c --- /dev/null +++ b/src/test/ui/nll/named-region-basic.rs @@ -0,0 +1,22 @@ +// 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. + +// Basic test for free regions in the NLL code. This test ought to +// report an error due to a reborrowing constraint. Right now, we get +// a variety of errors from the older, AST-based machinery (notably +// borrowck), and then we get the NLL error at the end. + +// compile-flags:-Znll + +fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> &'b u32 { + &*x +} + +fn main() { } diff --git a/src/test/ui/nll/named-region-basic.stderr b/src/test/ui/nll/named-region-basic.stderr new file mode 100644 index 000000000000..6b789465670b --- /dev/null +++ b/src/test/ui/nll/named-region-basic.stderr @@ -0,0 +1,29 @@ +warning: not reporting region error due to -Znll: SubSupConflict(AddrOfRegion($DIR/named-region-basic.rs:19:5: 19:8), Reborrow($DIR/named-region-basic.rs:19:5: 19:8), ReFree(DefId { krate: CrateNum(0), index: DefIndex(0:3) => named_region_basic[317d]::foo[0] }, BrNamed(CrateNum(0):DefIndex(1:10), 'b(88))), Reborrow($DIR/named-region-basic.rs:19:5: 19:8), ReFree(DefId { krate: CrateNum(0), index: DefIndex(0:3) => named_region_basic[317d]::foo[0] }, BrNamed(CrateNum(0):DefIndex(1:9), 'a(86)))) + --> $DIR/named-region-basic.rs:19:5 + | +19 | &*x + | ^^^ + +error[E0597]: `*x` does not live long enough + --> $DIR/named-region-basic.rs:19:6 + | +19 | &*x + | ^^ does not live long enough + | + = note: borrowed value must be valid for the static lifetime... +note: ...but borrowed value is only valid for the lifetime 'a as defined on the function body at 18:1 + --> $DIR/named-region-basic.rs:18:1 + | +18 | / fn foo<'a, 'b>(x: &'a u32, y: &'b u32) -> &'b u32 { +19 | | &*x +20 | | } + | |_^ + +error: free region `'a` does not outlive `'b` + --> $DIR/named-region-basic.rs:19:5 + | +19 | &*x + | ^^^ + +error: aborting due to 2 previous errors +