Skip to content

[WIP] borrowck diagnostic migration with eager booted #104941

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

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
364 changes: 356 additions & 8 deletions compiler/rustc_borrowck/messages.ftl

Large diffs are not rendered by default.

384 changes: 222 additions & 162 deletions compiler/rustc_borrowck/src/borrowck_errors.rs

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1911,6 +1911,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, '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]
/// ```
// FIXME(#100717): In the return value, the first three strings can contain untranslated text.
pub(crate) fn describe_place_for_conflicting_borrow(
&self,
first_borrowed_place: Place<'tcx>,
Expand Down
29 changes: 20 additions & 9 deletions compiler/rustc_borrowck/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use crate::session_diagnostics::{
CaptureVarKind, CaptureVarPathUseCause, OnClosureNote,
};
use itertools::Itertools;
use rustc_errors::{Applicability, Diag};
use rustc_errors::{Applicability, Diag, DiagArgValue, IntoDiagnosticArg};
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, Namespace};
use rustc_hir::CoroutineKind;
Expand Down Expand Up @@ -48,7 +48,7 @@ mod region_errors;

pub(crate) use bound_region_errors::{ToUniverseInfo, UniverseInfo};
pub(crate) use move_errors::{IllegalMoveOriginKind, MoveError};
pub(crate) use mutability_errors::AccessKind;
pub(crate) use mutability_errors::{AccessKind, PlaceAndReason};
pub(crate) use outlives_suggestion::OutlivesSuggestionBuilder;
pub(crate) use region_errors::{ErrorConstraintInfo, RegionErrorKind, RegionErrors};
pub(crate) use region_name::{RegionName, RegionNameSource};
Expand All @@ -64,6 +64,18 @@ pub(super) struct DescribePlaceOpt {

pub(super) struct IncludingTupleField(pub(super) bool);

#[derive(Debug)]
pub(super) struct DescribedPlace(pub(super) Option<String>);
Copy link
Member

Choose a reason for hiding this comment

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

Why is this necessary if self.0 already implements IntoDiagnosticArg?


impl IntoDiagnosticArg for DescribedPlace {
fn into_diagnostic_arg(self) -> DiagArgValue {
match self.0 {
Some(descr) => descr.into_diagnostic_arg(),
None => "value".into_diagnostic_arg(),
Copy link
Member

Choose a reason for hiding this comment

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

Maybe bug! here?

}
}
}

impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
/// Adds a suggestion when a closure is invoked twice with a moved variable or when a closure
/// is moved after being invoked.
Expand All @@ -76,7 +88,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
/// LL | for (key, value) in dict {
/// | ^^^^
/// ```
#[allow(rustc::diagnostic_outside_of_impl)] // FIXME
pub(super) fn add_moved_or_invoked_closure_note(
&self,
location: Location,
Expand Down Expand Up @@ -176,6 +187,10 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
}
}

pub(super) fn describe_place_typed(&self, place_ref: PlaceRef<'tcx>) -> DescribedPlace {
Copy link
Member

Choose a reason for hiding this comment

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

I presume this exists because changing describe_place to return a DescribedPlace` isn't practical?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

describe_place is used a lot, I'm currently narrowing the scope so it covers only PlaceAndReason cases.

DescribedPlace(self.describe_place(place_ref))
}

/// 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<'tcx>) -> Option<String> {
Expand Down Expand Up @@ -586,7 +601,6 @@ impl UseSpans<'_> {
}

/// Add a span label to the arguments of the closure, if it exists.
#[allow(rustc::diagnostic_outside_of_impl)]
pub(super) fn args_subdiag(
self,
dcx: &rustc_errors::DiagCtxt,
Expand All @@ -600,7 +614,6 @@ impl UseSpans<'_> {

/// Add a span label to the use of the captured variable, if it exists.
/// only adds label to the `path_span`
#[allow(rustc::diagnostic_outside_of_impl)]
pub(super) fn var_path_only_subdiag(
self,
dcx: &rustc_errors::DiagCtxt,
Expand Down Expand Up @@ -638,7 +651,6 @@ impl UseSpans<'_> {
}

/// Add a subdiagnostic to the use of the captured variable, if it exists.
#[allow(rustc::diagnostic_outside_of_impl)]
pub(super) fn var_subdiag(
self,
dcx: &rustc_errors::DiagCtxt,
Expand Down Expand Up @@ -703,6 +715,7 @@ impl UseSpans<'_> {
}
}

#[derive(Clone, Copy, Debug)]
pub(super) enum BorrowedContentSource<'tcx> {
DerefRawPointer,
DerefMutableRef,
Expand Down Expand Up @@ -754,7 +767,7 @@ impl<'tcx> BorrowedContentSource<'tcx> {
_ => None,
})
.unwrap_or_else(|| format!("dereference of `{ty}`")),
BorrowedContentSource::OverloadedIndex(ty) => format!("an index of `{ty}`"),
BorrowedContentSource::OverloadedIndex(ty) => format!("`{ty}`"),
}
}

Expand Down Expand Up @@ -1012,8 +1025,6 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
self.borrow_spans(span, borrow.reserve_location)
}

#[allow(rustc::diagnostic_outside_of_impl)]
#[allow(rustc::untranslatable_diagnostic)] // FIXME: make this translatable
fn explain_captures(
&mut self,
err: &mut Diag<'_>,
Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_borrowck/src/diagnostics/move_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use rustc_span::{BytePos, ExpnKind, MacroKind, Span};
use crate::diagnostics::CapturedMessageOpt;
use crate::diagnostics::{DescribePlaceOpt, UseSpans};
use crate::prefixes::PrefixSet;
use crate::session_diagnostics::AddMoveErr;
use crate::MirBorrowckCtxt;

#[derive(Debug)]
Expand Down Expand Up @@ -575,9 +576,9 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
let binding_span = bind_to.source_info.span;

if j == 0 {
err.span_label(binding_span, "data moved here");
err.subdiagnostic(self.dcx(), AddMoveErr::Here { binding_span });
} else {
err.span_label(binding_span, "...and here");
err.subdiagnostic(self.dcx(), AddMoveErr::AndHere { binding_span });
}

if binds_to.len() == 1 {
Expand All @@ -595,10 +596,7 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, 'tcx> {
}

if binds_to.len() > 1 {
err.note(
"move occurs because these variables have types that don't implement the `Copy` \
trait",
);
err.subdiagnostic(self.dcx(), AddMoveErr::MovedNotCopy);
}
}

Expand Down
Loading