Skip to content

Commit

Permalink
Explain error when yielding a reference to a local variable
Browse files Browse the repository at this point in the history
  • Loading branch information
matthewjasper committed Apr 22, 2019
1 parent c21fbfe commit d9ea132
Show file tree
Hide file tree
Showing 6 changed files with 107 additions and 26 deletions.
34 changes: 24 additions & 10 deletions src/librustc_mir/borrow_check/error_reporting.rs
Expand Up @@ -826,18 +826,21 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

let borrow_span = borrow_spans.var_or_use();
if let BorrowExplanation::MustBeValidFor {
category: ConstraintCategory::Return,
category,
span,
ref opt_place_desc,
from_closure: false,
..
} = explanation {
return self.report_cannot_return_reference_to_local(
if let Some(diag) = self.try_report_cannot_return_reference_to_local(
borrow,
borrow_span,
span,
category,
opt_place_desc.as_ref(),
);
) {
return diag;
}
}

let mut err = self.infcx.tcx.path_does_not_live_long_enough(
Expand Down Expand Up @@ -1015,17 +1018,20 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
);

if let BorrowExplanation::MustBeValidFor {
category: ConstraintCategory::Return,
category,
span,
from_closure: false,
..
} = explanation {
return self.report_cannot_return_reference_to_local(
if let Some(diag) = self.try_report_cannot_return_reference_to_local(
borrow,
proper_span,
span,
category,
None,
);
) {
return diag;
}
}

let tcx = self.infcx.tcx;
Expand Down Expand Up @@ -1064,15 +1070,22 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
err
}

fn report_cannot_return_reference_to_local(
fn try_report_cannot_return_reference_to_local(
&self,
borrow: &BorrowData<'tcx>,
borrow_span: Span,
return_span: Span,
category: ConstraintCategory,
opt_place_desc: Option<&String>,
) -> DiagnosticBuilder<'cx> {
) -> Option<DiagnosticBuilder<'cx>> {
let tcx = self.infcx.tcx;

let return_kind = match category {
ConstraintCategory::Return => "return",
ConstraintCategory::Yield => "yield",
_ => return None,
};

// FIXME use a better heuristic than Spans
let reference_desc = if return_span == self.mir.source_info(borrow.reserve_location).span {
"reference to"
Expand Down Expand Up @@ -1110,7 +1123,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
let local = if let Place::Base(PlaceBase::Local(local)) = *root_place {
local
} else {
bug!("report_cannot_return_reference_to_local: not a local")
bug!("try_report_cannot_return_reference_to_local: not a local")
};
match self.mir.local_kind(local) {
LocalKind::ReturnPointer | LocalKind::Temp => {
Expand All @@ -1131,6 +1144,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {

let mut err = tcx.cannot_return_reference_to_local(
return_span,
return_kind,
reference_desc,
&place_desc,
Origin::Mir,
Expand All @@ -1140,7 +1154,7 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
err.span_label(borrow_span, note);
}

err
Some(err)
}

fn report_escaping_closure_capture(
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_mir/borrow_check/nll/explain_borrow/mod.rs
Expand Up @@ -313,9 +313,13 @@ impl<'cx, 'gcx, 'tcx> MirBorrowckCtxt<'cx, 'gcx, 'tcx> {
opt_place_desc,
}
} else {
debug!("explain_why_borrow_contains_point: \
Could not generate a region name");
BorrowExplanation::Unexplained
}
} else {
debug!("explain_why_borrow_contains_point: \
Could not generate an error region vid");
BorrowExplanation::Unexplained
}
}
Expand Down
Expand Up @@ -32,6 +32,7 @@ crate enum RegionNameSource {
MatchedAdtAndSegment(Span),
AnonRegionFromUpvar(Span, String),
AnonRegionFromOutput(Span, String, String),
AnonRegionFromYieldTy(Span, String),
}

impl RegionName {
Expand All @@ -46,7 +47,8 @@ impl RegionName {
RegionNameSource::MatchedHirTy(..) |
RegionNameSource::MatchedAdtAndSegment(..) |
RegionNameSource::AnonRegionFromUpvar(..) |
RegionNameSource::AnonRegionFromOutput(..) => false,
RegionNameSource::AnonRegionFromOutput(..) |
RegionNameSource::AnonRegionFromYieldTy(..) => false,
}
}

Expand Down Expand Up @@ -103,6 +105,12 @@ impl RegionName {
format!("return type{} is {}", mir_description, type_name),
);
},
RegionNameSource::AnonRegionFromYieldTy(span, type_name) => {
diag.span_label(
*span,
format!("yield type is {}", type_name),
);
}
RegionNameSource::Static => {},
}
}
Expand Down Expand Up @@ -167,6 +175,11 @@ impl<'tcx> RegionInferenceContext<'tcx> {
self.give_name_if_anonymous_region_appears_in_output(
infcx, mir, mir_def_id, fr, counter,
)
})
.or_else(|| {
self.give_name_if_anonymous_region_appears_in_yield_ty(
infcx, mir, mir_def_id, fr, counter,
)
});

debug!("give_region_a_name: gave name {:?}", value);
Expand Down Expand Up @@ -673,10 +686,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
"give_name_if_anonymous_region_appears_in_output: return_ty = {:?}",
return_ty
);
if !infcx
.tcx
.any_free_region_meets(&return_ty, |r| r.to_region_vid() == fr)
{
if !tcx.any_free_region_meets(&return_ty, |r| r.to_region_vid() == fr) {
return None;
}

Expand Down Expand Up @@ -721,6 +731,57 @@ impl<'tcx> RegionInferenceContext<'tcx> {
})
}

fn give_name_if_anonymous_region_appears_in_yield_ty(
&self,
infcx: &InferCtxt<'_, '_, 'tcx>,
mir: &Mir<'tcx>,
mir_def_id: DefId,
fr: RegionVid,
counter: &mut usize,
) -> Option<RegionName> {
// Note: generators from `async fn` yield `()`, so we don't have to
// worry about them here.
let yield_ty = self.universal_regions.yield_ty?;
debug!(
"give_name_if_anonymous_region_appears_in_yield_ty: yield_ty = {:?}",
yield_ty,
);

let tcx = infcx.tcx;

if !tcx.any_free_region_meets(&yield_ty, |r| r.to_region_vid() == fr) {
return None;
}

let mut highlight = RegionHighlightMode::default();
highlight.highlighting_region_vid(fr, *counter);
let type_name = infcx.extract_type_name(&yield_ty, Some(highlight));

let mir_node_id = tcx.hir().as_local_node_id(mir_def_id).expect("non-local mir");

let yield_span = match tcx.hir().get(mir_node_id) {
hir::Node::Expr(hir::Expr {
node: hir::ExprKind::Closure(_, _, _, span, _),
..
}) => (
tcx.sess.source_map().end_point(*span)
),
_ => mir.span,
};

debug!(
"give_name_if_anonymous_region_appears_in_yield_ty: \
type_name = {:?}, yield_span = {:?}",
yield_span,
type_name,
);

Some(RegionName {
name: self.synthesize_region_name(counter),
source: RegionNameSource::AnonRegionFromYieldTy(yield_span, type_name),
})
}

/// Creates a synthetic region named `'1`, incrementing the
/// counter.
fn synthesize_region_name(&self, counter: &mut usize) -> InternedString {
Expand Down
6 changes: 4 additions & 2 deletions src/librustc_mir/util/borrowck_errors.rs
Expand Up @@ -650,6 +650,7 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
fn cannot_return_reference_to_local(
self,
span: Span,
return_kind: &str,
reference_desc: &str,
path_desc: &str,
o: Origin,
Expand All @@ -658,15 +659,16 @@ pub trait BorrowckErrors<'cx>: Sized + Copy {
self,
span,
E0515,
"cannot return {REFERENCE} {LOCAL}{OGN}",
"cannot {RETURN} {REFERENCE} {LOCAL}{OGN}",
RETURN=return_kind,
REFERENCE=reference_desc,
LOCAL=path_desc,
OGN = o
);

err.span_label(
span,
format!("returns a {} data owned by the current function", reference_desc),
format!("{}s a {} data owned by the current function", return_kind, reference_desc),
);

self.cancel_if_wrong_origin(err, o)
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/nll/issue-55850.rs
Expand Up @@ -25,7 +25,7 @@ where
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]
yield &s[..] //~ ERROR cannot yield value referencing local variable `s` [E0515]
//~| ERROR borrow may still be in use when generator yields
})
}
Expand Down
16 changes: 8 additions & 8 deletions src/test/ui/nll/issue-55850.stderr
@@ -1,11 +1,11 @@
error[E0597]: `s` does not live long enough
--> $DIR/issue-55850.rs:28:16
error[E0515]: cannot yield value referencing local variable `s`
--> $DIR/issue-55850.rs:28:9
|
LL | yield &s[..]
| ^ borrowed value does not live long enough
LL |
LL | })
| - `s` dropped here while still borrowed
| ^^^^^^^-^^^^
| | |
| | `s` is borrowed here
| yields a value referencing data owned by the current function

error[E0626]: borrow may still be in use when generator yields
--> $DIR/issue-55850.rs:28:16
Expand All @@ -15,5 +15,5 @@ LL | yield &s[..]

error: aborting due to 2 previous errors

Some errors have detailed explanations: E0597, E0626.
For more information about an error, try `rustc --explain E0597`.
Some errors have detailed explanations: E0515, E0626.
For more information about an error, try `rustc --explain E0515`.

0 comments on commit d9ea132

Please sign in to comment.