Skip to content

Commit

Permalink
Auto merge of #132250 - nnethercote:rustc_borrowck-cleanups, r=compil…
Browse files Browse the repository at this point in the history
…er-errors

`rustc_borrowck` cleanups

A bunch of cleanups I made while reading over this crate.

r? `@lqd`
  • Loading branch information
bors committed Nov 4, 2024
2 parents 56c6a2f + e0e7a43 commit ca87b53
Show file tree
Hide file tree
Showing 22 changed files with 251 additions and 385 deletions.
26 changes: 13 additions & 13 deletions compiler/rustc_borrowck/src/borrow_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@ pub struct BorrowSet<'tcx> {
/// by the `Location` of the assignment statement in which it
/// appears on the right hand side. Thus the location is the map
/// key, and its position in the map corresponds to `BorrowIndex`.
pub location_map: FxIndexMap<Location, BorrowData<'tcx>>,
pub(crate) location_map: FxIndexMap<Location, BorrowData<'tcx>>,

/// Locations which activate borrows.
/// NOTE: a given location may activate more than one borrow in the future
/// when more general two-phase borrow support is introduced, but for now we
/// only need to store one borrow index.
pub activation_map: FxIndexMap<Location, Vec<BorrowIndex>>,
pub(crate) activation_map: FxIndexMap<Location, Vec<BorrowIndex>>,

/// Map from local to all the borrows on that local.
pub local_map: FxIndexMap<mir::Local, FxIndexSet<BorrowIndex>>,
pub(crate) local_map: FxIndexMap<mir::Local, FxIndexSet<BorrowIndex>>,

pub locals_state_at_exit: LocalsStateAtExit,
pub(crate) locals_state_at_exit: LocalsStateAtExit,
}

impl<'tcx> Index<BorrowIndex> for BorrowSet<'tcx> {
Expand All @@ -45,7 +45,7 @@ impl<'tcx> Index<BorrowIndex> for BorrowSet<'tcx> {
/// Location where a two-phase borrow is activated, if a borrow
/// is in fact a two-phase borrow.
#[derive(Copy, Clone, PartialEq, Eq, Debug)]
pub enum TwoPhaseActivation {
pub(crate) enum TwoPhaseActivation {
NotTwoPhase,
NotActivated,
ActivatedAt(Location),
Expand All @@ -55,17 +55,17 @@ pub enum TwoPhaseActivation {
pub struct BorrowData<'tcx> {
/// Location where the borrow reservation starts.
/// In many cases, this will be equal to the activation location but not always.
pub reserve_location: Location,
pub(crate) reserve_location: Location,
/// Location where the borrow is activated.
pub activation_location: TwoPhaseActivation,
pub(crate) activation_location: TwoPhaseActivation,
/// What kind of borrow this is
pub kind: mir::BorrowKind,
pub(crate) kind: mir::BorrowKind,
/// The region for which this borrow is live
pub region: RegionVid,
pub(crate) region: RegionVid,
/// Place from which we are borrowing
pub borrowed_place: mir::Place<'tcx>,
pub(crate) borrowed_place: mir::Place<'tcx>,
/// Place to which the borrow was stored
pub assigned_place: mir::Place<'tcx>,
pub(crate) assigned_place: mir::Place<'tcx>,
}

impl<'tcx> fmt::Display for BorrowData<'tcx> {
Expand Down Expand Up @@ -120,7 +120,7 @@ impl LocalsStateAtExit {
}

impl<'tcx> BorrowSet<'tcx> {
pub fn build(
pub(crate) fn build(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
locals_are_invalidated_at_exit: bool,
Expand Down Expand Up @@ -156,7 +156,7 @@ impl<'tcx> BorrowSet<'tcx> {
self.activation_map.get(&location).map_or(&[], |activations| &activations[..])
}

pub fn len(&self) -> usize {
pub(crate) fn len(&self) -> usize {
self.location_map.len()
}

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/constraints/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ impl<'tcx> fmt::Debug for OutlivesConstraint<'tcx> {

rustc_index::newtype_index! {
#[debug_format = "OutlivesConstraintIndex({})"]
pub struct OutlivesConstraintIndex {}
pub(crate) struct OutlivesConstraintIndex {}
}

rustc_index::newtype_index! {
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/dataflow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ impl<'tcx> PoloniusOutOfScopePrecomputer<'_, 'tcx> {
let sccs = self.regioncx.constraint_sccs();
let universal_regions = self.regioncx.universal_regions();

// We first handle the cases where the loan doesn't go out of scope, depending on the issuing
// region's successors.
// We first handle the cases where the loan doesn't go out of scope, depending on the
// issuing region's successors.
for successor in graph::depth_first_search(&self.regioncx.region_graph(), issuing_region) {
// 1. Via applied member constraints
//
Expand Down
51 changes: 15 additions & 36 deletions compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use std::rc::Rc;

use rustc_errors::Diag;
use rustc_hir::def_id::LocalDefId;
use rustc_infer::infer::canonical::CanonicalQueryInput;
use rustc_infer::infer::region_constraints::{Constraint, RegionConstraintData};
use rustc_infer::infer::{
InferCtxt, RegionResolutionError, RegionVariableOrigin, SubregionOrigin, TyCtxtInferExt as _,
Expand All @@ -21,7 +20,6 @@ use rustc_span::Span;
use rustc_trait_selection::error_reporting::InferCtxtErrorExt;
use rustc_trait_selection::error_reporting::infer::nice_region_error::NiceRegionError;
use rustc_trait_selection::traits::ObligationCtxt;
use rustc_trait_selection::traits::query::type_op;
use rustc_traits::{type_op_ascribe_user_type_with_span, type_op_prove_predicate_with_cause};
use tracing::{debug, instrument};

Expand All @@ -31,12 +29,9 @@ use crate::session_diagnostics::{
HigherRankedErrorCause, HigherRankedLifetimeError, HigherRankedSubtypeError,
};

#[derive(Clone)]
pub(crate) struct UniverseInfo<'tcx>(UniverseInfoInner<'tcx>);

/// What operation a universe was created for.
#[derive(Clone)]
enum UniverseInfoInner<'tcx> {
pub(crate) enum UniverseInfo<'tcx> {
/// Relating two types which have binders.
RelateTys { expected: Ty<'tcx>, found: Ty<'tcx> },
/// Created from performing a `TypeOp`.
Expand All @@ -47,11 +42,11 @@ enum UniverseInfoInner<'tcx> {

impl<'tcx> UniverseInfo<'tcx> {
pub(crate) fn other() -> UniverseInfo<'tcx> {
UniverseInfo(UniverseInfoInner::Other)
UniverseInfo::Other
}

pub(crate) fn relate(expected: Ty<'tcx>, found: Ty<'tcx>) -> UniverseInfo<'tcx> {
UniverseInfo(UniverseInfoInner::RelateTys { expected, found })
UniverseInfo::RelateTys { expected, found }
}

pub(crate) fn report_error(
Expand All @@ -61,8 +56,8 @@ impl<'tcx> UniverseInfo<'tcx> {
error_element: RegionElement,
cause: ObligationCause<'tcx>,
) {
match self.0 {
UniverseInfoInner::RelateTys { expected, found } => {
match *self {
UniverseInfo::RelateTys { expected, found } => {
let err = mbcx.infcx.err_ctxt().report_mismatched_types(
&cause,
mbcx.param_env,
Expand All @@ -72,10 +67,10 @@ impl<'tcx> UniverseInfo<'tcx> {
);
mbcx.buffer_error(err);
}
UniverseInfoInner::TypeOp(ref type_op_info) => {
UniverseInfo::TypeOp(ref type_op_info) => {
type_op_info.report_error(mbcx, placeholder, error_element, cause);
}
UniverseInfoInner::Other => {
UniverseInfo::Other => {
// FIXME: This error message isn't great, but it doesn't show
// up in the existing UI tests. Consider investigating this
// some more.
Expand All @@ -93,46 +88,30 @@ pub(crate) trait ToUniverseInfo<'tcx> {

impl<'tcx> ToUniverseInfo<'tcx> for crate::type_check::InstantiateOpaqueType<'tcx> {
fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
UniverseInfo(UniverseInfoInner::TypeOp(Rc::new(crate::type_check::InstantiateOpaqueType {
UniverseInfo::TypeOp(Rc::new(crate::type_check::InstantiateOpaqueType {
base_universe: Some(base_universe),
..self
})))
}))
}
}

impl<'tcx> ToUniverseInfo<'tcx> for CanonicalTypeOpProvePredicateGoal<'tcx> {
fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
UniverseInfo(UniverseInfoInner::TypeOp(Rc::new(PredicateQuery {
canonical_query: self,
base_universe,
})))
UniverseInfo::TypeOp(Rc::new(PredicateQuery { canonical_query: self, base_universe }))
}
}

impl<'tcx, T: Copy + fmt::Display + TypeFoldable<TyCtxt<'tcx>> + 'tcx> ToUniverseInfo<'tcx>
for CanonicalTypeOpNormalizeGoal<'tcx, T>
{
fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
UniverseInfo(UniverseInfoInner::TypeOp(Rc::new(NormalizeQuery {
canonical_query: self,
base_universe,
})))
UniverseInfo::TypeOp(Rc::new(NormalizeQuery { canonical_query: self, base_universe }))
}
}

impl<'tcx> ToUniverseInfo<'tcx> for CanonicalTypeOpAscribeUserTypeGoal<'tcx> {
fn to_universe_info(self, base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
UniverseInfo(UniverseInfoInner::TypeOp(Rc::new(AscribeUserTypeQuery {
canonical_query: self,
base_universe,
})))
}
}

impl<'tcx, F> ToUniverseInfo<'tcx> for CanonicalQueryInput<'tcx, type_op::custom::CustomTypeOp<F>> {
fn to_universe_info(self, _base_universe: ty::UniverseIndex) -> UniverseInfo<'tcx> {
// We can't rerun custom type ops.
UniverseInfo::other()
UniverseInfo::TypeOp(Rc::new(AscribeUserTypeQuery { canonical_query: self, base_universe }))
}
}

Expand All @@ -143,7 +122,7 @@ impl<'tcx> ToUniverseInfo<'tcx> for ! {
}

#[allow(unused_lifetimes)]
trait TypeOpInfo<'tcx> {
pub(crate) trait TypeOpInfo<'tcx> {
/// Returns an error to be reported if rerunning the type op fails to
/// recover the error's cause.
fn fallback_error(&self, tcx: TyCtxt<'tcx>, span: Span) -> Diag<'tcx>;
Expand Down Expand Up @@ -289,8 +268,8 @@ where
// `rustc_traits::type_op::type_op_normalize` query to allow the span we need in the
// `ObligationCause`. The normalization results are currently different between
// `QueryNormalizeExt::query_normalize` used in the query and `normalize` called below:
// the former fails to normalize the `nll/relate_tys/impl-fn-ignore-binder-via-bottom.rs` test.
// Check after #85499 lands to see if its fixes have erased this difference.
// the former fails to normalize the `nll/relate_tys/impl-fn-ignore-binder-via-bottom.rs`
// test. Check after #85499 lands to see if its fixes have erased this difference.
let (param_env, value) = key.into_parts();
let _ = ocx.normalize(&cause, param_env, value.value);

Expand Down
47 changes: 26 additions & 21 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1345,11 +1345,13 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
// See `tests/ui/moves/needs-clone-through-deref.rs`
return false;
}
// We don't want to suggest `.clone()` in a move closure, since the value has already been captured.
// We don't want to suggest `.clone()` in a move closure, since the value has already been
// captured.
if self.in_move_closure(expr) {
return false;
}
// We also don't want to suggest cloning a closure itself, since the value has already been captured.
// We also don't want to suggest cloning a closure itself, since the value has already been
// captured.
if let hir::ExprKind::Closure(_) = expr.kind {
return false;
}
Expand Down Expand Up @@ -1381,7 +1383,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}
}
}
// Cloning the raw pointer doesn't make sense in some cases and would cause a type mismatch error. (see #126863)
// Cloning the raw pointer doesn't make sense in some cases and would cause a type mismatch
// error. (see #126863)
if inner_expr.span.lo() != expr.span.lo() && !is_raw_ptr {
// Remove "(*" or "(&"
sugg.push((expr.span.with_hi(inner_expr.span.lo()), String::new()));
Expand Down Expand Up @@ -1553,8 +1556,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, '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
// 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()),
Expand Down Expand Up @@ -2480,7 +2484,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
if let hir::ExprKind::Closure(closure) = ex.kind
&& ex.span.contains(self.borrow_span)
// To support cases like `|| { v.call(|this| v.get()) }`
// FIXME: actually support such cases (need to figure out how to move from the capture place to original local)
// FIXME: actually support such cases (need to figure out how to move from the
// capture place to original local).
&& self.res.as_ref().map_or(true, |(prev_res, _)| prev_res.span.contains(ex.span))
{
self.res = Some((ex, closure));
Expand Down Expand Up @@ -2733,7 +2738,7 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
/// cannot borrow `a.u` (via `a.u.z.c`) as immutable because it is also borrowed as
/// mutable (via `a.u.s.b`) [E0502]
/// ```
pub(crate) fn describe_place_for_conflicting_borrow(
fn describe_place_for_conflicting_borrow(
&self,
first_borrowed_place: Place<'tcx>,
second_borrowed_place: Place<'tcx>,
Expand Down Expand Up @@ -3188,8 +3193,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
/// misleading users in cases like `tests/ui/nll/borrowed-temporary-error.rs`.
/// We could expand the analysis to suggest hoising all of the relevant parts of
/// the users' code to make the code compile, but that could be too much.
/// We found the `prop_expr` by the way to check whether the expression is a `FormatArguments`,
/// which is a special case since it's generated by the compiler.
/// We found the `prop_expr` by the way to check whether the expression is a
/// `FormatArguments`, which is a special case since it's generated by the
/// compiler.
struct NestedStatementVisitor<'tcx> {
span: Span,
current: usize,
Expand Down Expand Up @@ -3420,7 +3426,8 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
let (sugg_span, suggestion) = match tcx.sess.source_map().span_to_snippet(args_span) {
Ok(string) => {
let coro_prefix = if string.starts_with("async") {
// `async` is 5 chars long. Not using `.len()` to avoid the cast from `usize` to `u32`
// `async` is 5 chars long. Not using `.len()` to avoid the cast from `usize`
// to `u32`.
Some(5)
} else if string.starts_with("gen") {
// `gen` is 3 chars long
Expand Down Expand Up @@ -3618,10 +3625,9 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
let stmt_kind =
self.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
// written by the user, so we ignore the move-outs
// created by `StorageDead` and at the beginning
// of a function.
// 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 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.
Expand Down Expand Up @@ -3706,13 +3712,12 @@ impl<'infcx, 'tcx> MirBorrowckCtxt<'_, 'infcx, 'tcx> {
}
}
if (is_argument || !reached_start) && result.is_empty() {
/* Process back edges (moves in future loop iterations) only if
the move path is definitely initialized upon loop entry,
to avoid spurious "in previous iteration" errors.
During DFS, if there's a path from the error back to the start
of the function with no intervening init or move, then the
move path may be uninitialized at loop entry.
*/
// Process back edges (moves in future loop iterations) only if
// the move path is definitely initialized upon loop entry,
// to avoid spurious "in previous iteration" errors.
// During DFS, if there's a path from the error back to the start
// of the function with no intervening init or move, then the
// move path may be uninitialized at loop entry.
while let Some(location) = back_edge_stack.pop() {
if dfs_iter(&mut result, location, true) {
continue;
Expand Down
Loading

0 comments on commit ca87b53

Please sign in to comment.