Skip to content

Commit

Permalink
Auto merge of #56460 - davidtwco:issue-55850, r=pnkfelix
Browse files Browse the repository at this point in the history
Fix ICE with generators and NLL

Fix #55850.

This PR stops an ICE in #55850 by not panicking when a region cannot be named. However, this PR does not (yet) fix the underlying issue that the correct name for the test case provided for the issue (in this instance, `'a`) was not found.

This PR also lays a little bit of groundwork by categorizing yields separately from returns so that region naming can be specialized for this case.

r? @pnkfelix
  • Loading branch information
bors committed Dec 7, 2018
2 parents 1c3236a + 4a286d3 commit a40fded
Show file tree
Hide file tree
Showing 9 changed files with 104 additions and 22 deletions.
1 change: 1 addition & 0 deletions src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,7 @@ impl_stable_hash_for!(struct mir::ClosureOutlivesRequirement<'tcx> {

impl_stable_hash_for!(enum mir::ConstraintCategory {
Return,
Yield,
UseAsConst,
UseAsStatic,
TypeAnnotation,
Expand Down
1 change: 1 addition & 0 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 11 additions & 7 deletions src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 ",
Expand Down Expand Up @@ -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,
}
});
Expand Down Expand Up @@ -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, _)
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<RegionName>) {
let (category, from_closure, span) =
self.best_blame_constraint(mir, borrow_region, |r| r == outlived_region);
let outlived_fr_name =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
mir_def_id: DefId,
fr: RegionVid,
counter: &mut usize,
) -> RegionName {
) -> Option<RegionName> {
debug!("give_region_a_name(fr={:?}, counter={})", fr, counter);

assert!(self.universal_regions.is_universal_region(fr));
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_mir/borrow_check/nll/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
18 changes: 18 additions & 0 deletions src/test/ui/nll/issue-55850.nll.stderr
Original file line number Diff line number Diff line change
@@ -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`.
44 changes: 44 additions & 0 deletions src/test/ui/nll/issue-55850.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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>(G);

impl <G> Iterator for GenIter<G>
where
G: Generator,
{
type Item = G::Yield;

fn next(&mut self) -> Option<Self::Item> {
unsafe {
match self.0.resume() {
Yielded(y) => Some(y),
_ => None
}
}
}
}

fn bug<'a>() -> impl Iterator<Item = &'a str> {
GenIter(move || {
let mut s = String::new();
yield &s[..] //~ ERROR `s` does not live long enough [E0597]
})
}

fn main() {
bug();
}
17 changes: 17 additions & 0 deletions src/test/ui/nll/issue-55850.stderr
Original file line number Diff line number Diff line change
@@ -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<Item = &'a str> {
| ^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0597`.

0 comments on commit a40fded

Please sign in to comment.