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

Aggregation of cosmetic changes made during work on REPL PRs: librustc_mir #64274

Closed
wants to merge 1 commit into from
Closed
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
7 changes: 4 additions & 3 deletions src/librustc_mir/borrow_check/borrow_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use crate::borrow_check::nll::ToRegionVid;
use crate::borrow_check::path_utils::allow_two_phase_borrow;
use crate::dataflow::indexes::BorrowIndex;
use crate::dataflow::move_paths::MoveData;

use rustc::mir::traversal;
use rustc::mir::visit::{PlaceContext, Visitor, NonUseContext, MutatingUseContext};
use rustc::mir::{self, Location, Body, Local};
Expand Down Expand Up @@ -168,7 +169,7 @@ struct GatherBorrows<'a, 'tcx> {
activation_map: FxHashMap<Location, Vec<BorrowIndex>>,
local_map: FxHashMap<mir::Local, FxHashSet<BorrowIndex>>,

/// When we encounter a 2-phase borrow statement, it will always
/// When we encounter a two-phase borrow statement, it will always
/// be assigning into a temporary TEMP:
///
/// TEMP = &foo
Expand Down Expand Up @@ -228,7 +229,7 @@ impl<'a, 'tcx> Visitor<'tcx> for GatherBorrows<'a, 'tcx> {
}

// We found a use of some temporary TMP
// check whether we (earlier) saw a 2-phase borrow like
// check whether we (earlier) saw a two-phase borrow like
//
// TMP = &mut place
if let Some(&borrow_index) = self.pending_activations.get(temp) {
Expand Down Expand Up @@ -309,7 +310,7 @@ impl<'a, 'tcx> GatherBorrows<'a, 'tcx> {
return;
}

// When we encounter a 2-phase borrow statement, it will always
// When we encounter a two-phase borrow statement, it will always
// be assigning into a temporary TEMP:
//
// TEMP = &foo
Expand Down
29 changes: 14 additions & 15 deletions src/librustc_mir/borrow_check/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use crate::util::borrowck_errors;

#[derive(Debug)]
struct MoveSite {
/// Index of the "move out" that we found. The `MoveData` can
/// The index of the "move out" that we found. The `MoveData` can
/// then tell us where the move occurred.
moi: MoveOutIndex,

Expand All @@ -35,7 +35,7 @@ struct MoveSite {
traversed_back_edge: bool
}

/// Which case a StorageDeadOrDrop is for.
/// Which case a `StorageDeadOrDrop` is for.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
enum StorageDeadOrDrop<'tcx> {
LocalStorageDead,
Expand Down Expand Up @@ -1130,8 +1130,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
format!("`{}` is borrowed here", place_desc),
)
} else {
let root_place = self.prefixes(borrow.borrowed_place.as_ref(),
PrefixSet::All)
let root_place = self.prefixes(borrow.borrowed_place.as_ref(), PrefixSet::All)
.last()
.unwrap();
let local = if let PlaceRef {
Expand Down Expand Up @@ -1223,7 +1222,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
&format!("function requires argument type to outlive `{}`", fr_name),
);
}
_ => bug!("report_escaping_closure_capture called with unexpected constraint \
_ => bug!("`report_escaping_closure_capture` called with unexpected constraint \
category: `{:?}`", category),
}
err
Expand All @@ -1245,7 +1244,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
match tables.node_type(mir_hir_id).sty {
ty::Closure(..) => "closure",
ty::Generator(..) => "generator",
_ => bug!("Closure body doesn't have a closure or generator type"),
_ => bug!("closure body doesn't have a closure or generator type"),
}
} else {
"function"
Expand Down Expand Up @@ -1302,31 +1301,31 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

'dfs: while let Some((location, is_back_edge)) = stack.pop() {
debug!(
"report_use_of_moved_or_uninitialized: (current_location={:?}, back_edge={})",
"report_use_of_moved_or_uninitialized: current_location={:?} back_edge={}",
location, is_back_edge
);

if !visited.insert(location) {
continue;
}

// check for moves
// Check for moves.
let stmt_kind = body[location.block]
.statements
.get(location.statement_index)
.map(|s| &s.kind);
if let Some(StatementKind::StorageDead(..)) = stmt_kind {
// this analysis only tries to find moves explicitly
// This analysis only tries to find moves explicitly
// written by the user, so we ignore the move-outs
// created by `StorageDead` and at the beginning
// created by StorageDead and at the beginning
// of a function.
} else {
// If we are found a use of a.b.c which was in error, then we want to look for
// moves not only of a.b.c but also a.b and a.
// If we are found a use of `a.b.c` which was in error, then we want to look for
// moves not only of `a.b.c` but also `a.b` and `a`.
//
// Note that the moves data already includes "parent" paths, so we don't have to
// worry about the other case: that is, if there is a move of a.b.c, it is already
// marked as a move of a.b and a as well, so we will generate the correct errors
// worry about the other case: that is, if there is a move of `a.b.c`, it is already
// marked as a move of `a.b` and `a` as well, so we will generate the correct errors
// there.
let mut mpis = vec![mpi];
let move_paths = &self.move_data.move_paths;
Expand Down Expand Up @@ -1362,7 +1361,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

// check for inits
// Check for inits.
let mut any_match = false;
drop_flag_effects::for_location_inits(
self.infcx.tcx,
Expand Down
56 changes: 31 additions & 25 deletions src/librustc_mir/borrow_check/error_reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,13 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

/// End-user visible description of `place` if one can be found. If the
/// Gets an end-user visible description of `place` if one can be found. If the
/// place is a temporary for instance, None will be returned.
pub(super) fn describe_place(&self, place_ref: PlaceRef<'cx, 'tcx>) -> Option<String> {
self.describe_place_with_options(place_ref, IncludingDowncast(false))
}

/// End-user visible description of `place` if one can be found. If the
/// Gets an end-user visible description of `place` if one can be found. If the
/// place is a temporary for instance, None will be returned.
/// `IncludingDowncast` parameter makes the function return `Err` if `ProjectionElem` is
/// `Downcast` and `IncludingDowncast` is true
Expand Down Expand Up @@ -195,7 +195,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
} else {
if autoderef {
// FIXME turn this recursion into iteration
// FIXME: turn this recursion into iteration.
self.append_place_to_string(
PlaceRef {
base,
Expand All @@ -219,7 +219,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
&including_downcast,
)?;
} else {
// FIXME deduplicate this and the _ => body below
// FIXME: deduplicate this and the `_ =>` body below.
buf.push_str(&"*");
self.append_place_to_string(
PlaceRef {
Expand Down Expand Up @@ -307,11 +307,12 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
buf.push_str("]");
}
ProjectionElem::ConstantIndex { .. } | ProjectionElem::Subslice { .. } => {
ProjectionElem::ConstantIndex { .. } |
ProjectionElem::Subslice { .. } => {
autoderef = true;
// Since it isn't possible to borrow an element on a particular index and
// then use another while the borrow is held, don't output indices details
// to avoid confusing the end-user
// to avoid confusing the end-user.
self.append_place_to_string(
PlaceRef {
base,
Expand Down Expand Up @@ -343,7 +344,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

/// End-user visible description of the `field`nth field of `base`
/// Gets an end-user visible description of the `field`nth field of `base`.
fn describe_field(&self, place: PlaceRef<'cx, 'tcx>, field: Field) -> String {
// FIXME Place2 Make this work iteratively
match place {
Expand Down Expand Up @@ -375,9 +376,9 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
ProjectionElem::Field(_, field_type) => {
self.describe_field_from_ty(&field_type, field, None)
}
ProjectionElem::Index(..)
| ProjectionElem::ConstantIndex { .. }
| ProjectionElem::Subslice { .. } => {
ProjectionElem::Index(..) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is formatting away from the official rustfmt style; can you please remove the styling changes to match arms in the PR (and if there are other ones in the other PR) altogether? It's not clear to me that they are a win; they mostly just introduce more commits in blame and rustfmt will reformat them anyways later.

ProjectionElem::ConstantIndex { .. } |
ProjectionElem::Subslice { .. } => {
self.describe_field(PlaceRef {
base,
projection: &proj.base,
Expand All @@ -387,15 +388,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

/// End-user visible description of the `field_index`nth field of `ty`
/// Gets an end-user visible description of the `field_index`nth field of `ty`.
fn describe_field_from_ty(
&self,
ty: Ty<'_>,
field: Field,
variant_index: Option<VariantIdx>
) -> String {
if ty.is_box() {
// If the type is a box, the field is described from the boxed type
// If the type is a box, the field is described from the boxed type.
self.describe_field_from_ty(&ty.boxed_ty(), field, variant_index)
} else {
match ty.sty {
Expand All @@ -411,12 +412,15 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
.to_string()
},
ty::Tuple(_) => field.index().to_string(),
ty::Ref(_, ty, _) | ty::RawPtr(ty::TypeAndMut { ty, .. }) => {
ty::Ref(_, ty, _) |
ty::RawPtr(ty::TypeAndMut { ty, .. }) => {
self.describe_field_from_ty(&ty, field, variant_index)
}
ty::Array(ty, _) | ty::Slice(ty) =>
ty::Array(ty, _) |
ty::Slice(ty) =>
self.describe_field_from_ty(&ty, field, variant_index),
ty::Closure(def_id, _) | ty::Generator(def_id, _, _) => {
ty::Closure(def_id, _) |
ty::Generator(def_id, _, _) => {
// `tcx.upvars(def_id)` returns an `Option`, which is `None` in case
// the closure comes from another crate. But in that case we wouldn't
// be borrowck'ing it, so we can just unwrap:
Expand Down Expand Up @@ -491,7 +495,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// we'll use this to check whether it was originally from an overloaded
// operator.
match self.move_data.rev_lookup.find(deref_base) {
LookupResult::Exact(mpi) | LookupResult::Parent(Some(mpi)) => {
LookupResult::Exact(mpi) |
LookupResult::Parent(Some(mpi)) => {
debug!("borrowed_content_source: mpi={:?}", mpi);

for i in &self.move_data.init_path_map[mpi] {
Expand Down Expand Up @@ -557,8 +562,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// this by hooking into the pretty printer and telling it to label the
// lifetimes without names with the value `'0`.
match ty.sty {
ty::Ref(ty::RegionKind::ReLateBound(_, br), _, _)
| ty::Ref(
ty::Ref(ty::RegionKind::ReLateBound(_, br), _, _) |
ty::Ref(
ty::RegionKind::RePlaceholder(ty::PlaceholderRegion { name: br, .. }),
_,
_,
Expand All @@ -579,8 +584,8 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
let region = match ty.sty {
ty::Ref(region, _, _) => {
match region {
ty::RegionKind::ReLateBound(_, br)
| ty::RegionKind::RePlaceholder(ty::PlaceholderRegion { name: br, .. }) => {
ty::RegionKind::ReLateBound(_, br) |
ty::RegionKind::RePlaceholder(ty::PlaceholderRegion { name: br, .. }) => {
printer.region_highlight_mode.highlighting_bound_region(*br, counter)
}
_ => {}
Expand Down Expand Up @@ -618,14 +623,15 @@ impl UseSpans {
match self {
UseSpans::ClosureUse {
args_span: span, ..
}
| UseSpans::OtherUse(span) => span,
} |
UseSpans::OtherUse(span) => span,
}
}

pub(super) fn var_or_use(self) -> Span {
match self {
UseSpans::ClosureUse { var_span: span, .. } | UseSpans::OtherUse(span) => span,
UseSpans::ClosureUse { var_span: span, .. } |
UseSpans::OtherUse(span) => span,
}
}

Expand Down Expand Up @@ -726,8 +732,8 @@ impl BorrowedContentSource<'tcx> {
BorrowedContentSource::DerefMutableRef => Some("mutable reference"),
// Overloaded deref and index operators should be evaluated into a
// temporary. So we don't need a description here.
BorrowedContentSource::OverloadedDeref(_)
| BorrowedContentSource::OverloadedIndex(_) => None
BorrowedContentSource::OverloadedDeref(_) |
BorrowedContentSource::OverloadedIndex(_) => None
}
}

Expand Down
Loading