Skip to content

Commit

Permalink
Rollup merge of rust-lang#65277 - csmoe:fix-move, r=estebank
Browse files Browse the repository at this point in the history
Query generator kind for error reporting

Fixes rust-lang#65166 (comment)
r? @estebank
cc @cramertj
  • Loading branch information
tmandry committed Oct 11, 2019
2 parents f1d2d95 + 9f69420 commit 2252ea1
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 26 deletions.
4 changes: 4 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,10 @@ impl Body {
hir_id: self.value.hir_id,
}
}

pub fn generator_kind(&self) -> Option<GeneratorKind> {
self.generator_kind
}
}

/// The type of source expression that caused this generator to be created.
Expand Down
30 changes: 22 additions & 8 deletions src/librustc_mir/borrow_check/conflict_errors.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use rustc::hir;
use rustc::hir::def_id::DefId;
use rustc::hir::{AsyncGeneratorKind, GeneratorKind};
use rustc::mir::{
self, AggregateKind, BindingForm, BorrowKind, ClearCrossCrate, ConstraintCategory, Local,
LocalDecl, LocalKind, Location, Operand, Place, PlaceBase, PlaceRef, ProjectionElem, Rvalue,
Expand Down Expand Up @@ -788,7 +789,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
..
},
) if borrow_spans.for_closure() => self.report_escaping_closure_capture(
borrow_spans.args_or_use(),
borrow_spans,
borrow_span,
region_name,
category,
Expand All @@ -806,7 +807,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
},

) if borrow_spans.for_generator() => self.report_escaping_closure_capture(
borrow_spans.args_or_use(),
borrow_spans,
borrow_span,
region_name,
category,
Expand Down Expand Up @@ -1195,15 +1196,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

fn report_escaping_closure_capture(
&mut self,
args_span: Span,
use_span: UseSpans,
var_span: Span,
fr_name: &RegionName,
category: ConstraintCategory,
constraint_span: Span,
captured_var: &str,
) -> DiagnosticBuilder<'cx> {
let tcx = self.infcx.tcx;

let args_span = use_span.args_or_use();
let mut err = self.cannot_capture_in_long_lived_closure(
args_span,
captured_var,
Expand All @@ -1223,12 +1224,25 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
},
Err(_) => "move |<args>| <body>".to_string()
};

let kind = match use_span.generator_kind() {
Some(generator_kind) => match generator_kind {
GeneratorKind::Async(async_kind) => match async_kind {
AsyncGeneratorKind::Block => "async block",
AsyncGeneratorKind::Closure => "async closure",
_ => bug!("async block/closure expected, but async funtion found."),
},
GeneratorKind::Gen => "generator",
}
None => "closure",
};
err.span_suggestion(
args_span,
&format!("to force the closure to take ownership of {} (and any \
other referenced variables), use the `move` keyword",
captured_var),
&format!(
"to force the {} to take ownership of {} (and any \
other referenced variables), use the `move` keyword",
kind,
captured_var
),
suggestion,
Applicability::MachineApplicable,
);
Expand Down
45 changes: 28 additions & 17 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use rustc::hir;
use rustc::hir::def::Namespace;
use rustc::hir::def_id::DefId;
use rustc::hir::GeneratorKind;
use rustc::mir::{
AggregateKind, Constant, Field, Local, LocalKind, Location, Operand,
Place, PlaceBase, PlaceRef, ProjectionElem, Rvalue, Statement, StatementKind,
Expand All @@ -14,7 +15,7 @@ use syntax_pos::Span;
use syntax::symbol::sym;

use super::borrow_set::BorrowData;
use super::{MirBorrowckCtxt};
use super::MirBorrowckCtxt;
use crate::dataflow::move_paths::{InitLocation, LookupResult};

pub(super) struct IncludingDowncast(pub(super) bool);
Expand Down Expand Up @@ -604,7 +605,7 @@ pub(super) enum UseSpans {
// The access is caused by capturing a variable for a closure.
ClosureUse {
// This is true if the captured variable was from a generator.
is_generator: bool,
generator_kind: Option<GeneratorKind>,
// The span of the args of the closure, including the `move` keyword if
// it's present.
args_span: Span,
Expand All @@ -631,6 +632,13 @@ impl UseSpans {
}
}

pub(super) fn generator_kind(self) -> Option<GeneratorKind> {
match self {
UseSpans::ClosureUse { generator_kind, .. } => generator_kind,
_ => None,
}
}

// Add a span label to the arguments of the closure, if it exists.
pub(super) fn args_span_label(
self,
Expand All @@ -656,23 +664,23 @@ impl UseSpans {
/// Returns `false` if this place is not used in a closure.
pub(super) fn for_closure(&self) -> bool {
match *self {
UseSpans::ClosureUse { is_generator, .. } => !is_generator,
UseSpans::ClosureUse { generator_kind, .. } => generator_kind.is_none(),
_ => false,
}
}

/// Returns `false` if this place is not used in a generator.
pub(super) fn for_generator(&self) -> bool {
match *self {
UseSpans::ClosureUse { is_generator, .. } => is_generator,
UseSpans::ClosureUse { generator_kind, .. } => generator_kind.is_some(),
_ => false,
}
}

/// Describe the span associated with a use of a place.
pub(super) fn describe(&self) -> String {
match *self {
UseSpans::ClosureUse { is_generator, .. } => if is_generator {
UseSpans::ClosureUse { generator_kind, .. } => if generator_kind.is_some() {
" in generator".to_string()
} else {
" in closure".to_string()
Expand Down Expand Up @@ -794,19 +802,20 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if let StatementKind::Assign(
box(_, Rvalue::Aggregate(ref kind, ref places))
) = stmt.kind {
let (def_id, is_generator) = match kind {
box AggregateKind::Closure(def_id, _) => (def_id, false),
box AggregateKind::Generator(def_id, _, _) => (def_id, true),
let def_id = match kind {
box AggregateKind::Closure(def_id, _)
| box AggregateKind::Generator(def_id, _, _) => def_id,
_ => return OtherUse(stmt.source_info.span),
};

debug!(
"move_spans: def_id={:?} is_generator={:?} places={:?}",
def_id, is_generator, places
"move_spans: def_id={:?} places={:?}",
def_id, places
);
if let Some((args_span, var_span)) = self.closure_span(*def_id, moved_place, places) {
if let Some((args_span, generator_kind, var_span))
= self.closure_span(*def_id, moved_place, places) {
return ClosureUse {
is_generator,
generator_kind,
args_span,
var_span,
};
Expand Down Expand Up @@ -857,11 +866,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
"borrow_spans: def_id={:?} is_generator={:?} places={:?}",
def_id, is_generator, places
);
if let Some((args_span, var_span)) = self.closure_span(
if let Some((args_span, generator_kind, var_span)) = self.closure_span(
*def_id, Place::from(target).as_ref(), places
) {
return ClosureUse {
is_generator,
generator_kind,
args_span,
var_span,
};
Expand All @@ -884,7 +893,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
def_id: DefId,
target_place: PlaceRef<'cx, 'tcx>,
places: &Vec<Operand<'tcx>>,
) -> Option<(Span, Span)> {
) -> Option<(Span, Option<GeneratorKind>, Span)> {
debug!(
"closure_span: def_id={:?} target_place={:?} places={:?}",
def_id, target_place, places
Expand All @@ -893,14 +902,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let expr = &self.infcx.tcx.hir().expect_expr(hir_id).kind;
debug!("closure_span: hir_id={:?} expr={:?}", hir_id, expr);
if let hir::ExprKind::Closure(
.., args_span, _
.., body_id, args_span, _
) = expr {
for (upvar, place) in self.infcx.tcx.upvars(def_id)?.values().zip(places) {
match place {
Operand::Copy(place) |
Operand::Move(place) if target_place == place.as_ref() => {
debug!("closure_span: found captured local {:?}", place);
return Some((*args_span, upvar.span));
let body = self.infcx.tcx.hir().body(*body_id);
let generator_kind = body.generator_kind();
return Some((*args_span, generator_kind, upvar.span));
},
_ => {}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ note: generator is returned here
|
LL | fn foo() -> Box<impl std::future::Future<Output = u32>> {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: to force the closure to take ownership of `x` (and any other referenced variables), use the `move` keyword
help: to force the async block to take ownership of `x` (and any other referenced variables), use the `move` keyword
|
LL | Box::new(async move { x } )
| ^^^^^^^^^^
Expand Down

0 comments on commit 2252ea1

Please sign in to comment.