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.

Sign up for GitHub

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

bring in path spans to ClosureUse, to_string for captured_place #50

Open
wants to merge 4 commits into
base: wr-mir-replace-methods2
Choose a base branch
from
Open

bring in path spans to ClosureUse, to_string for captured_place #50

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a54eefb
Replace closures_captures and upvar_capture with closure_min_captures
jenniferwills Dec 23, 2020
1ad8bfb
bring in path spans to ClosureUse, to_string for captured_place
ChrisPardy Mar 17, 2021
674c229
add tests
ChrisPardy Mar 17, 2021
4a9850f
closure diags
ChrisPardy Apr 15, 2021
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
11 changes: 11 additions & 0 deletions compiler/rustc_middle/src/mir/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,15 @@ impl BorrowKind {
BorrowKind::Mut { allow_two_phase_borrow } => allow_two_phase_borrow,
}
}

pub fn describe_mutability(&self) -> String {
match *self {
BorrowKind::Shared | BorrowKind::Shallow | BorrowKind::Unique => {
"immutable".to_string()
}
BorrowKind::Mut { .. } => "mutable".to_string(),
}
}
}

///////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -2318,6 +2327,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
};
let mut struct_fmt = fmt.debug_struct(&name);

// FIXME: This should be a list of capture names/places
if let Some(upvars) = tcx.upvars_mentioned(def_id) {
for (&var_id, place) in upvars.keys().zip(places) {
let var_name = tcx.hir().name(var_id);
Expand All @@ -2337,6 +2347,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
let name = format!("[generator@{:?}]", tcx.hir().span(hir_id));
let mut struct_fmt = fmt.debug_struct(&name);

// FIXME: This should be a list of capture names/places
if let Some(upvars) = tcx.upvars_mentioned(def_id) {
for (&var_id, place) in upvars.keys().zip(places) {
let var_name = tcx.hir().name(var_id);
Expand Down
37 changes: 2 additions & 35 deletions compiler/rustc_middle/src/ty/context.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,7 @@ use rustc_attr as attr;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_data_structures::profiling::SelfProfilerRef;
use rustc_data_structures::sharded::{IntoPointer, ShardedHashMap};
use rustc_data_structures::stable_hasher::{
hash_stable_hashmap, HashStable, StableHasher, StableVec,
};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher, StableVec};
use rustc_data_structures::steal::Steal;
use rustc_data_structures::sync::{self, Lock, Lrc, WorkerLocal};
use rustc_errors::ErrorReported;
Expand Down Expand Up @@ -382,9 +380,6 @@ pub struct TypeckResults<'tcx> {
/// <https://github.com/rust-lang/rfcs/blob/master/text/2005-match-ergonomics.md#definitions>
pat_adjustments: ItemLocalMap<Vec<Ty<'tcx>>>,

/// Borrows
pub upvar_capture_map: ty::UpvarCaptureMap<'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, HirPlace<'tcx>)>,
Expand Down Expand Up @@ -420,12 +415,6 @@ pub struct TypeckResults<'tcx> {
/// by this function.
pub concrete_opaque_types: FxHashMap<DefId, ResolvedOpaqueTy<'tcx>>,

/// Given the closure ID this map provides the list of UpvarIDs used by it.
/// The upvarID contains the HIR node ID and it also contains the full path
/// leading to the member of the struct or tuple that is used instead of the
/// entire variable.
pub closure_captures: ty::UpvarListMap,

/// Tracks the minimum captures required for a closure;
/// see `MinCaptureInformationMap` for more details.
pub closure_min_captures: ty::MinCaptureInformationMap<'tcx>,
Expand Down Expand Up @@ -454,15 +443,13 @@ impl<'tcx> TypeckResults<'tcx> {
adjustments: Default::default(),
pat_binding_modes: Default::default(),
pat_adjustments: Default::default(),
upvar_capture_map: Default::default(),
closure_kind_origins: Default::default(),
liberated_fn_sigs: Default::default(),
fru_field_types: Default::default(),
coercion_casts: Default::default(),
used_trait_imports: Lrc::new(Default::default()),
tainted_by_errors: None,
concrete_opaque_types: Default::default(),
closure_captures: Default::default(),
closure_min_captures: Default::default(),
generator_interior_types: ty::Binder::dummy(Default::default()),
treat_byte_string_as_slice: Default::default(),
Expand Down Expand Up @@ -646,10 +633,6 @@ impl<'tcx> TypeckResults<'tcx> {
.flatten()
}

pub fn upvar_capture(&self, upvar_id: ty::UpvarId) -> ty::UpvarCapture<'tcx> {
self.upvar_capture_map[&upvar_id]
}

pub fn closure_kind_origins(&self) -> LocalTableInContext<'_, (Span, HirPlace<'tcx>)> {
LocalTableInContext { hir_owner: self.hir_owner, data: &self.closure_kind_origins }
}
Expand Down Expand Up @@ -693,7 +676,7 @@ impl<'tcx> TypeckResults<'tcx> {
impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
let ty::TypeckResults {
hir_owner,
hir_owner: _,
ref type_dependent_defs,
ref field_indices,
ref user_provided_types,
Expand All @@ -703,17 +686,13 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
ref adjustments,
ref pat_binding_modes,
ref pat_adjustments,
ref upvar_capture_map,
ref closure_kind_origins,
ref liberated_fn_sigs,
ref fru_field_types,

ref coercion_casts,

ref used_trait_imports,
tainted_by_errors,
ref concrete_opaque_types,
ref closure_captures,
ref closure_min_captures,
ref generator_interior_types,
ref treat_byte_string_as_slice,
Expand All @@ -729,17 +708,6 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
adjustments.hash_stable(hcx, hasher);
pat_binding_modes.hash_stable(hcx, hasher);
pat_adjustments.hash_stable(hcx, hasher);
hash_stable_hashmap(hcx, hasher, upvar_capture_map, |up_var_id, hcx| {
let ty::UpvarId { var_path, closure_expr_id } = *up_var_id;

assert_eq!(var_path.hir_id.owner, hir_owner);

(
hcx.local_def_path_hash(var_path.hir_id.owner),
var_path.hir_id.local_id,
hcx.local_def_path_hash(closure_expr_id),
)
});

closure_kind_origins.hash_stable(hcx, hasher);
liberated_fn_sigs.hash_stable(hcx, hasher);
Expand All @@ -748,7 +716,6 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for TypeckResults<'tcx> {
used_trait_imports.hash_stable(hcx, hasher);
tainted_by_errors.hash_stable(hcx, hasher);
concrete_opaque_types.hash_stable(hcx, hasher);
closure_captures.hash_stable(hcx, hasher);
closure_min_captures.hash_stable(hcx, hasher);
generator_interior_types.hash_stable(hcx, hasher);
treat_byte_string_as_slice.hash_stable(hcx, hasher);
Expand Down
72 changes: 71 additions & 1 deletion compiler/rustc_middle/src/ty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -673,14 +673,84 @@ pub struct CapturedPlace<'tcx> {
}

impl CapturedPlace<'tcx> {
pub fn to_string(&self, tcx: TyCtxt<'tcx>) -> String {
place_to_string_for_capture(tcx, &self.place)
}

/// Returns the hir-id of the root variable for the captured place.
/// e.g., if `a.b.c` was captured, would return the hir-id for `a`.
pub fn get_root_variable(&self) -> hir::HirId {
match self.place.base {
HirPlaceBase::Upvar(upvar_id) => upvar_id.var_path.hir_id,
base => bug!("Expected upvar, found={:?}", base),
base => bug!("expected upvar, found={:?}", base),
}
}

/// Returns the `LocalDefId` of the closure that captureed this Place
pub fn get_closure_local_def_id(&self) -> LocalDefId {
match self.place.base {
HirPlaceBase::Upvar(upvar_id) => upvar_id.closure_expr_id,
base => bug!("expected upvar, found={:?}", base),
}
}

/// Return span pointing to use that resulted in selecting the captured path
pub fn get_path_span(&self, tcx: TyCtxt<'tcx>) -> Span {
if let Some(path_expr_id) = self.info.path_expr_id {
tcx.hir().span(path_expr_id)
} else if let Some(capture_kind_expr_id) = self.info.capture_kind_expr_id {
tcx.hir().span(capture_kind_expr_id)
} else {
// Fallback on upvars mentioned if neither path or capture expr id is captured

// Safe to unwrap since we know this place is captured by the closure, therefore the closure must have upvars.
tcx.upvars_mentioned(self.get_closure_local_def_id()).unwrap()
[&self.get_root_variable()]
.span
}
}

/// Return span pointing to use that resulted in selecting the current capture kind
pub fn get_capture_kind_span(&self, tcx: TyCtxt<'tcx>) -> Span {
if let Some(capture_kind_expr_id) = self.info.capture_kind_expr_id {
tcx.hir().span(capture_kind_expr_id)
} else if let Some(path_expr_id) = self.info.path_expr_id {
tcx.hir().span(path_expr_id)
} else {
// Fallback on upvars mentioned if neither path or capture expr id is captured

// Safe to unwrap since we know this place is captured by the closure, therefore the closure must have upvars.
tcx.upvars_mentioned(self.get_closure_local_def_id()).unwrap()
[&self.get_root_variable()]
.span
}
}
}

/// Return true if the `proj_possible_ancestor` represents an ancestor path
/// to `proj_capture` or `proj_possible_ancestor` is same as `proj_capture`,
/// assuming they both start off of the same root variable.
///
/// **Note:** It's the caller's responsibility to ensure that both lists of projections
/// start off of the same root variable.
///
/// Eg: 1. `foo.x` which is represented using `projections=[Field(x)]` is an ancestor of
/// `foo.x.y` which is represented using `projections=[Field(x), Field(y)]`.
/// Note both `foo.x` and `foo.x.y` start off of the same root variable `foo`.
/// 2. Since we only look at the projections here function will return `bar.x` as an a valid
/// ancestor of `foo.x.y`. It's the caller's responsibility to ensure that both projections
/// list are being applied to the same root variable.
pub fn is_ancestor_or_same_capture(
proj_possible_ancestor: &[HirProjectionKind],
proj_capture: &[HirProjectionKind],
) -> bool {
// We want to make sure `is_ancestor_or_same_capture("x.0.0", "x.0")` to return false.
// Therefore we can't just check if all projections are same in the zipped iterator below.
if proj_possible_ancestor.len() > proj_capture.len() {
return false;
}

proj_possible_ancestor.iter().zip(proj_capture).all(|(a, b)| a == b)
}

pub fn place_to_string_for_capture(tcx: TyCtxt<'tcx>, place: &HirPlace<'tcx>) -> String {
Expand Down
44 changes: 31 additions & 13 deletions compiler/rustc_mir/src/borrow_check/diagnostics/conflict_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
);
err.span_label(span, format!("use of possibly-uninitialized {}", item_msg));

use_spans.var_span_label(
use_spans.var_span_label_path_only(
&mut err,
format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()),
);
Expand Down Expand Up @@ -242,6 +242,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
partially_str,
move_spans.describe()
),
"moved",
);
}
}
Expand Down Expand Up @@ -274,7 +275,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

use_spans.var_span_label(
use_spans.var_span_label_path_only(
&mut err,
format!("{} occurs due to use{}", desired_action.as_noun(), use_spans.describe()),
);
Expand Down Expand Up @@ -404,13 +405,16 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
err.span_label(borrow_span, format!("borrow of {} occurs here", borrow_msg));
err.span_label(span, format!("move out of {} occurs here", value_msg));

borrow_spans.var_span_label(
borrow_spans.var_span_label_path_only(
&mut err,
format!("borrow occurs due to use{}", borrow_spans.describe()),
);

move_spans
.var_span_label(&mut err, format!("move occurs due to use{}", move_spans.describe()));
move_spans.var_span_label(
&mut err,
format!("move occurs due to use{}", move_spans.describe()),
"moved",
);

self.explain_why_borrow_contains_point(location, borrow, None)
.add_explanation_to_diagnostic(
Expand Down Expand Up @@ -438,18 +442,24 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let use_spans = self.move_spans(place.as_ref(), location);
let span = use_spans.var_or_use();

// If the attempted use is in a closure then we do not care about the path span of the place we are currently trying to use
// we call `var_span_label` on `borrow_spans` to annotate if the existing borrow was in a closure
let mut err = self.cannot_use_when_mutably_borrowed(
span,
&self.describe_any_place(place.as_ref()),
borrow_span,
&self.describe_any_place(borrow.borrowed_place.as_ref()),
);

borrow_spans.var_span_label(&mut err, {
let place = &borrow.borrowed_place;
let desc_place = self.describe_any_place(place.as_ref());
format!("borrow occurs due to use of {}{}", desc_place, borrow_spans.describe())
});
borrow_spans.var_span_label(
&mut err,
{
let place = &borrow.borrowed_place;
let desc_place = self.describe_any_place(place.as_ref());
format!("borrow occurs due to use of {}{}", desc_place, borrow_spans.describe())
},
"mutable",
);

self.explain_why_borrow_contains_point(location, borrow, None)
.add_explanation_to_diagnostic(
Expand Down Expand Up @@ -561,6 +571,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
desc_place,
borrow_spans.describe(),
),
"immutable",
);

return err;
Expand Down Expand Up @@ -637,7 +648,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
if issued_spans == borrow_spans {
borrow_spans.var_span_label(
&mut err,
format!("borrows occur due to use of {}{}", desc_place, borrow_spans.describe()),
format!("borrows occur due to use of {}{}", desc_place, borrow_spans.describe(),),
gen_borrow_kind.describe_mutability(),
);
} else {
let borrow_place = &issued_borrow.borrowed_place;
Expand All @@ -649,6 +661,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
borrow_place_desc,
issued_spans.describe(),
),
issued_borrow.kind.describe_mutability(),
);

borrow_spans.var_span_label(
Expand All @@ -658,6 +671,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
desc_place,
borrow_spans.describe(),
),
gen_borrow_kind.describe_mutability(),
);
}

Expand Down Expand Up @@ -1545,6 +1559,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
loan_spans.var_span_label(
&mut err,
format!("borrow occurs due to use{}", loan_spans.describe()),
loan.kind.describe_mutability(),
);

err.buffer(&mut self.errors_buffer);
Expand All @@ -1555,8 +1570,11 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

let mut err = self.cannot_assign_to_borrowed(span, loan_span, &descr_place);

loan_spans
.var_span_label(&mut err, format!("borrow occurs due to use{}", loan_spans.describe()));
loan_spans.var_span_label(
&mut err,
format!("borrow occurs due to use{}", loan_spans.describe()),
loan.kind.describe_mutability(),
);

self.explain_why_borrow_contains_point(location, loan, None).add_explanation_to_diagnostic(
self.infcx.tcx,
Expand Down
Loading