Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use hir::Place instead of Symbol in closure_kind_origin #36

Merged
merged 1 commit into from
Jan 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use crate::arena::Arena;
use crate::dep_graph::{self, DepGraph, DepKind, DepNode, DepNodeExt};
use crate::hir::exports::ExportMap;
use crate::hir::place::Place as HirPlace;
use crate::ich::{NodeIdHashingMode, StableHashingContext};
use crate::infer::canonical::{Canonical, CanonicalVarInfo, CanonicalVarInfos};
use crate::lint::{struct_lint_level, LintDiagnosticBuilder, LintLevelSource};
Expand Down Expand Up @@ -379,7 +380,7 @@ pub struct TypeckResults<'tcx> {

/// Records the reasons that we picked the kind of each closure;
/// not all closures are present in the map.
closure_kind_origins: ItemLocalMap<(Span, Symbol)>,
closure_kind_origins: ItemLocalMap<(Span, HirPlace<'tcx>)>,

/// For each fn, records the "liberated" types of its arguments
/// and return type. Liberated means that all bound regions
Expand Down Expand Up @@ -642,11 +643,13 @@ impl<'tcx> TypeckResults<'tcx> {
self.upvar_capture_map[&upvar_id]
}

pub fn closure_kind_origins(&self) -> LocalTableInContext<'_, (Span, Symbol)> {
pub fn closure_kind_origins(&self) -> LocalTableInContext<'_, (Span, HirPlace<'tcx>)> {
LocalTableInContext { hir_owner: self.hir_owner, data: &self.closure_kind_origins }
}

pub fn closure_kind_origins_mut(&mut self) -> LocalTableInContextMut<'_, (Span, Symbol)> {
pub fn closure_kind_origins_mut(
&mut self,
) -> LocalTableInContextMut<'_, (Span, HirPlace<'tcx>)> {
LocalTableInContextMut { hir_owner: self.hir_owner, data: &mut self.closure_kind_origins }
}

Expand Down
41 changes: 40 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ pub use self::IntVarValue::*;
pub use self::Variance::*;

use crate::hir::exports::ExportMap;
use crate::hir::place::Place as HirPlace;
use crate::hir::place::{
Place as HirPlace, PlaceBase as HirPlaceBase, ProjectionKind as HirProjectionKind,
};
use crate::ich::StableHashingContext;
use crate::middle::cstore::CrateStoreDyn;
use crate::middle::resolve_lifetime::ObjectLifetimeDefault;
Expand Down Expand Up @@ -734,6 +736,43 @@ pub struct CapturedPlace<'tcx> {
pub info: CaptureInfo<'tcx>,
}

pub fn place_to_string_for_capture(tcx: TyCtxt<'tcx>, place: &HirPlace<'tcx>) -> String {
let name = match place.base {
HirPlaceBase::Upvar(upvar_id) => tcx.hir().name(upvar_id.var_path.hir_id).to_string(),
_ => bug!("Capture_information should only contain upvars"),
};
let mut curr_string = name;

for (i, proj) in place.projections.iter().enumerate() {
match proj.kind {
HirProjectionKind::Deref => {
curr_string = format!("*{}", curr_string);
}
HirProjectionKind::Field(idx, variant) => match place.ty_before_projection(i).kind() {
ty::Adt(def, ..) => {
curr_string = format!(
"{}.{}",
curr_string,
def.variants[variant].fields[idx as usize].ident.name.as_str()
);
}
ty::Tuple(_) => {
curr_string = format!("{}.{}", curr_string, idx);
}
_ => {
bug!(
"Field projection applied to a type other than Adt or Tuple: {:?}.",
place.ty_before_projection(i).kind()
)
}
},
proj => bug!("{:?} unexpected because it isn't captured", proj),
}
}

curr_string.to_string()
}

/// Part of `MinCaptureInformationMap`; describes the capture kind (&, &mut, move)
/// for a particular capture as well as identifying the part of the source code
/// that triggered this capture to occur.
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_mir/src/borrow_check/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let did = did.expect_local();
let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(did);

if let Some((span, name)) =
if let Some((span, hir_place)) =
self.infcx.tcx.typeck(did).closure_kind_origins().get(hir_id)
{
diag.span_note(
*span,
&format!(
"closure cannot be invoked more than once because it moves the \
variable `{}` out of its environment",
name,
ty::place_to_string_for_capture(self.infcx.tcx, hir_place)
),
);
return;
Expand All @@ -127,15 +127,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let did = did.expect_local();
let hir_id = self.infcx.tcx.hir().local_def_id_to_hir_id(did);

if let Some((span, name)) =
if let Some((span, hir_place)) =
self.infcx.tcx.typeck(did).closure_kind_origins().get(hir_id)
{
diag.span_note(
*span,
&format!(
"closure cannot be moved more than once as it is not `Copy` due to \
moving the variable `{}` out of its environment",
name
ty::place_to_string_for_capture(self.infcx.tcx, hir_place)
),
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,23 +597,23 @@ impl<'a, 'tcx> InferCtxtExt<'tcx> for InferCtxt<'a, 'tcx> {
if let Some(typeck_results) = self.in_progress_typeck_results {
let typeck_results = typeck_results.borrow();
match (found_kind, typeck_results.closure_kind_origins().get(hir_id)) {
(ty::ClosureKind::FnOnce, Some((span, name))) => {
(ty::ClosureKind::FnOnce, Some((span, place))) => {
err.span_label(
*span,
format!(
"closure is `FnOnce` because it moves the \
variable `{}` out of its environment",
name
ty::place_to_string_for_capture(tcx, place)
),
);
}
(ty::ClosureKind::FnMut, Some((span, name))) => {
(ty::ClosureKind::FnMut, Some((span, place))) => {
err.span_label(
*span,
format!(
"closure is `FnMut` because it mutates the \
variable `{}` here",
name
ty::place_to_string_for_capture(tcx, place)
),
);
}
Expand Down
26 changes: 18 additions & 8 deletions compiler/rustc_typeck/src/check/upvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,17 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.demand_eqtype(span, inferred_kind.to_ty(self.tcx), closure_kind_ty);

// If we have an origin, store it.
if let Some(origin) = delegate.current_origin {
if let Some(origin) = delegate.current_origin.clone() {
roxelo marked this conversation as resolved.
Show resolved Hide resolved
let origin = if self.tcx.features().capture_disjoint_fields {
origin
} else {
// FIXME(project-rfc-2229#26): Once rust-lang#80092 is merged, we should restrict the
// precision of origin as well. Otherwise, this will cause issues when project-rfc-2229#26
// is fixed as we might see Index projections in the origin, which we can't print because
// we don't store enough information.
(origin.0, Place { projections: vec![], ..origin.1 })
roxelo marked this conversation as resolved.
Show resolved Hide resolved
};

self.typeck_results
.borrow_mut()
.closure_kind_origins_mut()
Expand Down Expand Up @@ -563,7 +573,7 @@ struct InferBorrowKind<'a, 'tcx> {

// If we modified `current_closure_kind`, this field contains a `Some()` with the
// variable access that caused us to do so.
current_origin: Option<(Span, Symbol)>,
current_origin: Option<(Span, Place<'tcx>)>,

/// For each Place that is captured by the closure, we track the minimal kind of
/// access we need (ref, ref mut, move, etc) and the expression that resulted in such access.
Expand Down Expand Up @@ -628,7 +638,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
upvar_id.closure_expr_id,
ty::ClosureKind::FnOnce,
usage_span,
var_name(tcx, upvar_id.var_path.hir_id),
place_with_id.place.clone(),
);

let capture_info = ty::CaptureInfo {
Expand Down Expand Up @@ -720,7 +730,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
upvar_id.closure_expr_id,
ty::ClosureKind::FnMut,
tcx.hir().span(diag_expr_id),
var_name(tcx, upvar_id.var_path.hir_id),
place_with_id.place.clone(),
);
}
}
Expand Down Expand Up @@ -765,11 +775,11 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
closure_id: LocalDefId,
new_kind: ty::ClosureKind,
upvar_span: Span,
var_name: Symbol,
place: Place<'tcx>,
) {
debug!(
"adjust_closure_kind(closure_id={:?}, new_kind={:?}, upvar_span={:?}, var_name={})",
closure_id, new_kind, upvar_span, var_name
"adjust_closure_kind(closure_id={:?}, new_kind={:?}, upvar_span={:?}, place={:?})",
closure_id, new_kind, upvar_span, place
);

// Is this the closure whose kind is currently being inferred?
Expand Down Expand Up @@ -797,7 +807,7 @@ impl<'a, 'tcx> InferBorrowKind<'a, 'tcx> {
| (ty::ClosureKind::FnMut, ty::ClosureKind::FnOnce) => {
// new kind is stronger than the old kind
self.current_closure_kind = new_kind;
self.current_origin = Some((upvar_span, var_name));
self.current_origin = Some((upvar_span, place));
}
}
}
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_typeck/src/check/writeback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,9 +384,11 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> {
assert_eq!(fcx_typeck_results.hir_owner, self.typeck_results.hir_owner);
let common_hir_owner = fcx_typeck_results.hir_owner;

for (&id, &origin) in fcx_typeck_results.closure_kind_origins().iter() {
let hir_id = hir::HirId { owner: common_hir_owner, local_id: id };
self.typeck_results.closure_kind_origins_mut().insert(hir_id, origin);
for (id, origin) in fcx_typeck_results.closure_kind_origins().iter() {
roxelo marked this conversation as resolved.
Show resolved Hide resolved
let hir_id = hir::HirId { owner: common_hir_owner, local_id: *id };
let place_span = origin.0;
let place = self.resolve(origin.1.clone(), &place_span);
self.typeck_results.closure_kind_origins_mut().insert(hir_id, (place_span, place));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#![feature(capture_disjoint_fields)]
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
//~| `#[warn(incomplete_features)]` on by default
//~| see issue #53488 <https://github.com/rust-lang/rust/issues/53488>

// Check that precise paths are being reported back in the error message.


enum MultiVariant {
Point(i32, i32),
Meta(i32)
}

fn main() {
let mut point = MultiVariant::Point(10, -10,);

let mut meta = MultiVariant::Meta(1);

let c = || {
if let MultiVariant::Point(ref mut x, _) = point {
*x += 1;
}

if let MultiVariant::Meta(ref mut v) = meta {
*v += 1;
}
};

let a = c;
let b = c; //~ ERROR use of moved value: `c` [E0382]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/closure-origin-multi-variant-diagnostics.rs:1:12
|
LL | #![feature(capture_disjoint_fields)]
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information

error[E0382]: use of moved value: `c`
--> $DIR/closure-origin-multi-variant-diagnostics.rs:30:13
|
LL | let a = c;
| - value moved here
LL | let b = c;
| ^ value used here after move
|
note: closure cannot be moved more than once as it is not `Copy` due to moving the variable `point.0` out of its environment
--> $DIR/closure-origin-multi-variant-diagnostics.rs:20:52
|
LL | if let MultiVariant::Point(ref mut x, _) = point {
| ^^^^^

error: aborting due to previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0382`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#![feature(capture_disjoint_fields)]
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
//~| `#[warn(incomplete_features)]` on by default
//~| see issue #53488 <https://github.com/rust-lang/rust/issues/53488>

// Check that precise paths are being reported back in the error message.

enum SingleVariant {
Point(i32, i32),
}

fn main() {
let mut point = SingleVariant::Point(10, -10);

let c = || {
// FIXME(project-rfc-2229#24): Change this to be a destructure pattern
// once this is fixed, to remove the warning.
if let SingleVariant::Point(ref mut x, _) = point {
//~^ WARNING: irrefutable if-let pattern
*x += 1;
}
};

let b = c;
let a = c; //~ ERROR use of moved value: `c` [E0382]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
warning: the feature `capture_disjoint_fields` is incomplete and may not be safe to use and/or cause compiler crashes
--> $DIR/closure-origin-single-variant-diagnostics.rs:1:12
|
LL | #![feature(capture_disjoint_fields)]
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information

warning: irrefutable if-let pattern
--> $DIR/closure-origin-single-variant-diagnostics.rs:18:9
|
LL | / if let SingleVariant::Point(ref mut x, _) = point {
LL | |
LL | | *x += 1;
LL | | }
| |_________^
|
= note: `#[warn(irrefutable_let_patterns)]` on by default

error[E0382]: use of moved value: `c`
--> $DIR/closure-origin-single-variant-diagnostics.rs:25:13
|
LL | let b = c;
| - value moved here
LL | let a = c;
| ^ value used here after move
|
note: closure cannot be moved more than once as it is not `Copy` due to moving the variable `point.0` out of its environment
--> $DIR/closure-origin-single-variant-diagnostics.rs:18:53
|
LL | if let SingleVariant::Point(ref mut x, _) = point {
| ^^^^^

error: aborting due to previous error; 2 warnings emitted

For more information about this error, try `rustc --explain E0382`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#![feature(capture_disjoint_fields)]
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete
//~| `#[warn(incomplete_features)]` on by default
//~| see issue #53488 <https://github.com/rust-lang/rust/issues/53488>

// Check that precise paths are being reported back in the error message.

struct Y {
y: X
}

struct X {
a: u32,
b: u32,
}

fn main() {
let mut x = Y { y: X { a: 5, b: 0 } };
let hello = || {
x.y.a += 1;
};

let b = hello;
let c = hello; //~ ERROR use of moved value: `hello` [E0382]
}
Loading