From ea873f57c9e2bd8d184d45737a4c581ee28523d1 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Dec 2018 14:11:53 +0100 Subject: [PATCH 1/2] Introduce constraint category for yields. This commit adds a new `ConstraintCategory` for yield points - this allows for differentiation between a normal return and a yield in the diagnostics. --- src/librustc/ich/impls_mir.rs | 1 + src/librustc/mir/mod.rs | 1 + .../nll/region_infer/error_reporting/mod.rs | 10 +++++----- src/librustc_mir/borrow_check/nll/type_check/mod.rs | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/librustc/ich/impls_mir.rs b/src/librustc/ich/impls_mir.rs index c72887124aa0b..a46e12be1aeac 100644 --- a/src/librustc/ich/impls_mir.rs +++ b/src/librustc/ich/impls_mir.rs @@ -471,6 +471,7 @@ impl_stable_hash_for!(struct mir::ClosureOutlivesRequirement<'tcx> { impl_stable_hash_for!(enum mir::ConstraintCategory { Return, + Yield, UseAsConst, UseAsStatic, TypeAnnotation, diff --git a/src/librustc/mir/mod.rs b/src/librustc/mir/mod.rs index 7259bbfb780bf..6ff60b39bd341 100644 --- a/src/librustc/mir/mod.rs +++ b/src/librustc/mir/mod.rs @@ -2905,6 +2905,7 @@ pub struct ClosureOutlivesRequirement<'tcx> { #[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)] pub enum ConstraintCategory { Return, + Yield, UseAsConst, UseAsStatic, TypeAnnotation, diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs index 3358e5851f939..940f9d6040cf5 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs @@ -38,6 +38,7 @@ impl ConstraintDescription for ConstraintCategory { match self { ConstraintCategory::Assignment => "assignment ", ConstraintCategory::Return => "returning this value ", + ConstraintCategory::Yield => "yielding this value ", ConstraintCategory::UseAsConst => "using this value as a constant ", ConstraintCategory::UseAsStatic => "using this value as a static ", ConstraintCategory::Cast => "cast ", @@ -133,11 +134,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { let constraint_sup_scc = self.constraint_sccs.scc(constraint.sup); match categorized_path[i].0 { - ConstraintCategory::OpaqueType - | ConstraintCategory::Boring - | ConstraintCategory::BoringNoLocation - | ConstraintCategory::Internal => false, - ConstraintCategory::TypeAnnotation | ConstraintCategory::Return => true, + ConstraintCategory::OpaqueType | ConstraintCategory::Boring | + ConstraintCategory::BoringNoLocation | ConstraintCategory::Internal => false, + ConstraintCategory::TypeAnnotation | ConstraintCategory::Return | + ConstraintCategory::Yield => true, _ => constraint_sup_scc != target_scc, } }); diff --git a/src/librustc_mir/borrow_check/nll/type_check/mod.rs b/src/librustc_mir/borrow_check/nll/type_check/mod.rs index 5f64dfd931c89..33346a584e5a4 100644 --- a/src/librustc_mir/borrow_check/nll/type_check/mod.rs +++ b/src/librustc_mir/borrow_check/nll/type_check/mod.rs @@ -1467,7 +1467,7 @@ impl<'a, 'gcx, 'tcx> TypeChecker<'a, 'gcx, 'tcx> { value_ty, ty, term_location.to_locations(), - ConstraintCategory::Return, + ConstraintCategory::Yield, ) { span_mirbug!( self, From 4a286d33f9833d001d1b6ee7272b2afc9b7ff425 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 3 Dec 2018 14:13:28 +0100 Subject: [PATCH 2/2] Fix ICE in region naming. This commit puts a fix in place for the ICE in region naming code so that it doesn't break the compiler. However, this results in the diagnostic being poorer as the borrow explanation that was causing the ICE is not being added - this should be fixed as a follow-up. --- .../borrow_check/nll/explain_borrow/mod.rs | 18 +++++--- .../nll/region_infer/error_reporting/mod.rs | 10 ++--- .../error_reporting/region_name.rs | 5 +-- src/test/ui/nll/issue-55850.nll.stderr | 18 ++++++++ src/test/ui/nll/issue-55850.rs | 44 +++++++++++++++++++ src/test/ui/nll/issue-55850.stderr | 17 +++++++ 6 files changed, 96 insertions(+), 16 deletions(-) create mode 100644 src/test/ui/nll/issue-55850.nll.stderr create mode 100644 src/test/ui/nll/issue-55850.rs create mode 100644 src/test/ui/nll/issue-55850.stderr diff --git a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs index 477b78926084e..7fb3f02e0e3f3 100644 --- a/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs +++ b/src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs @@ -277,13 +277,17 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> { borrow_region_vid, region, ); - let opt_place_desc = self.describe_place(&borrow.borrowed_place); - BorrowExplanation::MustBeValidFor { - category, - from_closure, - span, - region_name, - opt_place_desc, + if let Some(region_name) = region_name { + let opt_place_desc = self.describe_place(&borrow.borrowed_place); + BorrowExplanation::MustBeValidFor { + category, + from_closure, + span, + region_name, + opt_place_desc, + } + } else { + BorrowExplanation::Unexplained } } else { BorrowExplanation::Unexplained diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs index 940f9d6040cf5..32aaa0590d2f9 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/mod.rs @@ -376,9 +376,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { diag.span_label(span, message); - match self.give_region_a_name(infcx, mir, mir_def_id, outlived_fr, &mut 1) - .source - { + match self.give_region_a_name(infcx, mir, mir_def_id, outlived_fr, &mut 1).unwrap().source { RegionNameSource::NamedEarlyBoundRegion(fr_span) | RegionNameSource::NamedFreeRegion(fr_span) | RegionNameSource::SynthesizedFreeEnvRegion(fr_span, _) @@ -521,10 +519,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { ); let counter = &mut 1; - let fr_name = self.give_region_a_name(infcx, mir, mir_def_id, fr, counter); + let fr_name = self.give_region_a_name(infcx, mir, mir_def_id, fr, counter).unwrap(); fr_name.highlight_region_name(&mut diag); let outlived_fr_name = - self.give_region_a_name(infcx, mir, mir_def_id, outlived_fr, counter); + self.give_region_a_name(infcx, mir, mir_def_id, outlived_fr, counter).unwrap(); outlived_fr_name.highlight_region_name(&mut diag); let mir_def_name = if infcx.tcx.is_closure(mir_def_id) { @@ -661,7 +659,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { infcx: &InferCtxt<'_, '_, 'tcx>, borrow_region: RegionVid, outlived_region: RegionVid, - ) -> (ConstraintCategory, bool, Span, RegionName) { + ) -> (ConstraintCategory, bool, Span, Option) { let (category, from_closure, span) = self.best_blame_constraint(mir, borrow_region, |r| r == outlived_region); let outlived_fr_name = diff --git a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs index a32fb0503a814..b01e257ae2eff 100644 --- a/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs +++ b/src/librustc_mir/borrow_check/nll/region_infer/error_reporting/region_name.rs @@ -157,7 +157,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { mir_def_id: DefId, fr: RegionVid, counter: &mut usize, - ) -> RegionName { + ) -> Option { debug!("give_region_a_name(fr={:?}, counter={})", fr, counter); assert!(self.universal_regions.is_universal_region(fr)); @@ -177,8 +177,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { self.give_name_if_anonymous_region_appears_in_output( infcx, mir, mir_def_id, fr, counter, ) - }) - .unwrap_or_else(|| span_bug!(mir.span, "can't make a name for free region {:?}", fr)); + }); debug!("give_region_a_name: gave name {:?}", value); value diff --git a/src/test/ui/nll/issue-55850.nll.stderr b/src/test/ui/nll/issue-55850.nll.stderr new file mode 100644 index 0000000000000..08e5217670e84 --- /dev/null +++ b/src/test/ui/nll/issue-55850.nll.stderr @@ -0,0 +1,18 @@ +error[E0597]: `s` does not live long enough + --> $DIR/issue-55850.rs:38:16 + | +LL | yield &s[..] //~ ERROR `s` does not live long enough [E0597] + | ^ borrowed value does not live long enough +LL | }) + | - `s` dropped here while still borrowed + +error[E0626]: borrow may still be in use when generator yields + --> $DIR/issue-55850.rs:38:16 + | +LL | yield &s[..] //~ ERROR `s` does not live long enough [E0597] + | -------^---- possible yield occurs here + +error: aborting due to 2 previous errors + +Some errors occurred: E0597, E0626. +For more information about an error, try `rustc --explain E0597`. diff --git a/src/test/ui/nll/issue-55850.rs b/src/test/ui/nll/issue-55850.rs new file mode 100644 index 0000000000000..4140815c91bb6 --- /dev/null +++ b/src/test/ui/nll/issue-55850.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. + +#![allow(unused_mut)] +#![feature(generators, generator_trait)] + +use std::ops::Generator; +use std::ops::GeneratorState::Yielded; + +pub struct GenIter(G); + +impl Iterator for GenIter +where + G: Generator, +{ + type Item = G::Yield; + + fn next(&mut self) -> Option { + unsafe { + match self.0.resume() { + Yielded(y) => Some(y), + _ => None + } + } + } +} + +fn bug<'a>() -> impl Iterator { + GenIter(move || { + let mut s = String::new(); + yield &s[..] //~ ERROR `s` does not live long enough [E0597] + }) +} + +fn main() { + bug(); +} diff --git a/src/test/ui/nll/issue-55850.stderr b/src/test/ui/nll/issue-55850.stderr new file mode 100644 index 0000000000000..26b4c82076c23 --- /dev/null +++ b/src/test/ui/nll/issue-55850.stderr @@ -0,0 +1,17 @@ +error[E0597]: `s` does not live long enough + --> $DIR/issue-55850.rs:38:16 + | +LL | yield &s[..] //~ ERROR `s` does not live long enough [E0597] + | ^ borrowed value does not live long enough +LL | }) + | - borrowed value only lives until here + | +note: borrowed value must be valid for the lifetime 'a as defined on the function body at 35:8... + --> $DIR/issue-55850.rs:35:8 + | +LL | fn bug<'a>() -> impl Iterator { + | ^^ + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`.