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

rustc_borrowck cleanups #132250

Merged
merged 10 commits into from
Nov 4, 2024
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
Loading