Skip to content

Diagnostic translation for borrow_check:errors #112237

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

Closed
wants to merge 3 commits into from
Closed

Diagnostic translation for borrow_check:errors #112237

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e20a137
Diagnostic translation
sladyn98 Jun 3, 2023
a954475
fixed linters
sladyn98 Jun 3, 2023
737b70c
commit suggestions
sladyn98 Jun 7, 2023
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
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/messages.ftl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ borrowck_cannot_move_when_borrowed =
*[other] {$value_place}
} occurs here

borrowck_cannot_use_when_mutably_borrowed = cannot use `{$desc}` when mutably borrowed

borrowck_capture_immute =
capture is immutable because of use here

Expand Down
14 changes: 5 additions & 9 deletions compiler/rustc_borrowck/src/borrowck_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::session_diagnostics::CannotUseWhenMutablyBorrowed;
use rustc_errors::{
struct_span_err, DiagnosticBuilder, DiagnosticId, DiagnosticMessage, ErrorGuaranteed, MultiSpan,
};
Expand Down Expand Up @@ -29,17 +30,12 @@ impl<'cx, 'tcx> crate::MirBorrowckCtxt<'cx, 'tcx> {
borrow_span: Span,
borrow_desc: &str,
) -> DiagnosticBuilder<'tcx, ErrorGuaranteed> {
let mut err = struct_span_err!(
self,
self.infcx.tcx.sess.create_err(CannotUseWhenMutablyBorrowed {
span,
E0503,
"cannot use {} because it was mutably borrowed",
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change the wording from here to "cannot use {$desc} when mutably borrowed"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I read some of the examples in the ftl file and there were certain placeholders there so since desc would be put in here in the message I thought {desc} would be also substituted in the ftl file from the struct passed in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I might be completely wrong, feel free to correct me

Copy link
Member

Choose a reason for hiding this comment

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

I meant, why did you change the wording? The old phrasing was "because it was mutably borrowed", the new phrasing is "when mutably borrowed"? I think we should separate diagnostics migration from tweaks of wording.

borrow_span,
borrow_desc,
desc,
);

err.span_label(borrow_span, format!("{} is borrowed here", borrow_desc));
err.span_label(span, format!("use of borrowed {}", borrow_desc));
Comment on lines -40 to -41
Copy link
Member

Choose a reason for hiding this comment

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

These labels weren't added to the ftl file, where did they go?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should they be added as separate messages with their labels something like this

borrowck_cannot_use_when_mutably_borrowed = {$borrow_span} is borrowed here

Copy link
Member

Choose a reason for hiding this comment

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

You should preserve all the error messages as much as possible

err
})
}

pub(crate) fn cannot_mutably_borrow_multiply(
Expand Down
11 changes: 11 additions & 0 deletions compiler/rustc_borrowck/src/session_diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,17 @@ use rustc_span::Span;

use crate::diagnostics::RegionName;

#[derive(Diagnostic)]
#[diag(borrowck_cannot_use_when_mutably_borrowed, code = "E0503")]
pub(crate) struct CannotUseWhenMutablyBorrowed<'a> {
#[primary_span]
pub span: Span,
#[label]
Copy link
Member

Choose a reason for hiding this comment

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

There is no corresponding label in the ftl file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am still trying to understand how to use them, would just including the rest of the parameters under primary span work ? I can then just put their placeholders in the ftl file. I read the documentation but I was not really sure how to incorporate them in the .ftl file

Copy link
Member

Choose a reason for hiding this comment

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

Adding a #[label] is a reference to a .label = <msg> attribute on the borrowck_cannot_use_when_mutably_borrowed message, so it should have something like:

borrowck_cannot_use_when_mutably_borrowed = cannot use `{$desc}` when mutably borrowed
    .label = <msg>

pub borrow_span: Span,
pub borrow_desc: &'a str,
pub desc: &'a str,
}

#[derive(Diagnostic)]
#[diag(borrowck_move_unsized, code = "E0161")]
pub(crate) struct MoveUnsized<'tcx> {
Expand Down