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.

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

Jump to bottom

Improve fluent error messages #106427

Merged
merged 3 commits into from
Jan 11, 2023
Merged

Improve fluent error messages #106427

merged 3 commits into from
Jan 11, 2023

Conversation

mejrs
Copy link
Contributor

@mejrs mejrs commented Jan 3, 2023

These have been really frustrating me while migrating diagnostics.

@rustbot
Copy link
Collaborator

rustbot commented Jan 3, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @TaKO8Ki (or someone else) soon.

Please see the contribution instructions for more information.

@rustbot rustbot added A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 3, 2023
@rustbot
Copy link
Collaborator

rustbot commented Jan 3, 2023

rustc_errors::translation was changed

cc @davidtwco, @compiler-errors, @JohnTitor, @estebank, @TaKO8Ki

davidtwco
davidtwco reviewed Jan 7, 2023
View reviewed changes
Copy link
Member

@davidtwco davidtwco left a comment

Choose a reason for hiding this comment

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

Are you able to add tests to this? Or at least post examples of the output before and after?

@davidtwco davidtwco assigned davidtwco and unassigned TaKO8Ki Jan 7, 2023
@mejrs
Copy link
Contributor Author

mejrs commented Jan 8, 2023

Are you able to add tests to this?

I'll take a look at adding some tests 👍

Or at least post examples of the output before and after?

mir_build_irrefutable_let_patterns_while_let = irrefutable `while let` {$coun ->

thread 'rustc' panicked at 'failed to find message in primary or fallback fluent bundles:
failed while formatting fluent string `mir_build_irrefutable_let_patterns_while_let`:
the fluent string has an argument `coun` that was not found.
help: the argument `count` is available
', compiler\rustc_errors\src\translation.rs:109:14

.mutable_borrow = another mutable borrow, by `{name_mut}`, occurs here

thread 'rustc' panicked at 'failed to find message in primary or fallback fluent bundles:
failed while formatting fluent string `mir_build_multiple_mut_borrows`:
argument `name_mut` exists but was not referenced correctly
help: try using `{$name_mut}` instead

.mutable_borrow = another mutable borrow, by `{$oops}`, occurs here

thread 'rustc' panicked at 'failed to find message in primary or fallback fluent bundles:
failed while formatting fluent string `mir_build_multiple_mut_borrows`:
the fluent string has an argument `oops` that was not found.
help: the arguments `name` and `name_mut` are available

@rust-log-analyzer

This comment has been minimized.

Sign in to view
@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jan 9, 2023

📌 Commit 4c0c32c has been approved by davidtwco

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 9, 2023
klensy
klensy reviewed Jan 9, 2023
View reviewed changes
compiler/rustc_errors/src/translation.rs
|bundle: &'a FluentBundle| -> Result<Cow<'_, str>, TranslateError<'_>> {
let message = bundle
.get_message(identifier)
.ok_or(TranslateError::message(identifier, args))?;
Copy link
Contributor

Choose a reason for hiding this comment

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

ok_or eagerly evaluated, looks not ok?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These constructors don't call any functions themselves, so this is just the construction of an enum variant which should be effectively free. As such it shouldn't matter.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, i see now.

klensy
klensy reviewed Jan 9, 2023
View reviewed changes
compiler/rustc_errors/src/translation.rs
let value = match attr {
Some(attr) => message
.get_attribute(attr)
.ok_or(TranslateError::attribute(identifier, args, attr))?
Copy link
Contributor

Choose a reason for hiding this comment

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

same

klensy
klensy reviewed Jan 9, 2023
View reviewed changes
compiler/rustc_errors/src/translation.rs
.get_attribute(attr)
.ok_or(TranslateError::attribute(identifier, args, attr))?
.value(),
None => message.value().ok_or(TranslateError::value(identifier, args))?,
Copy link
Contributor

Choose a reason for hiding this comment

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

same

Noratrieb added a commit to Noratrieb/rust that referenced this pull request Jan 11, 2023
@Noratrieb
Rollup merge of rust-lang#106427 - mejrs:translation_errors, r=davidtwco
d3df1e4 
Improve fluent error messages

These have been really frustrating me while migrating diagnostics.
This was referenced Jan 11, 2023
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 11, 2023
@bors
Auto merge of rust-lang#106730 - Nilstrieb:rollup-f7p8dsa, r=Nilstrieb
ef4046e 
Rollup of 9 pull requests

Successful merges:

 - rust-lang#106321 (Collect and emit proper backtraces for `delay_span_bug`s)
 - rust-lang#106397 (Check `impl`'s `where` clauses in `consider_impl_candidate` in experimental solver)
 - rust-lang#106427 (Improve fluent error messages)
 - rust-lang#106570 (add tests for div_duration_* functions)
 - rust-lang#106648 (Polymorphization cleanup)
 - rust-lang#106664 (Remove unnecessary lseek syscall when using std::fs::read)
 - rust-lang#106709 (Disable "split dwarf inlining" by default.)
 - rust-lang#106715 (Autolabel and ping wg for changes to new solver)
 - rust-lang#106717 (fix typo LocalItemId -> ItemLocalId)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 7347655 into rust-lang:master Jan 11, 2023
@rustbot rustbot added this to the 1.68.0 milestone Jan 11, 2023
This was referenced Jan 13, 2023
Closed
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 23, 2023
@Dylan-DPC
Rollup merge of rust-lang#106923 - mejrs:fluent_err, r=davidtwco
58baee1 
Restore behavior when primary bundle is missing

Fixes rust-lang#106755 by restoring some of the behavior prior to rust-lang#106427

Still, I have no idea how this debug assertion can even hit while using `en-US` as primary  bundle.

r? `@davidtwco`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Feb 23, 2023
@matthiaskrgr
Rollup merge of rust-lang#106923 - mejrs:fluent_err, r=davidtwco
f6c606c 
Restore behavior when primary bundle is missing

Fixes rust-lang#106755 by restoring some of the behavior prior to rust-lang#106427

Still, I have no idea how this debug assertion can even hit while using `en-US` as primary  bundle.

r? ``@davidtwco``
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Feb 24, 2023
@Dylan-DPC
Rollup merge of rust-lang#106923 - mejrs:fluent_err, r=davidtwco
6826a96 
Restore behavior when primary bundle is missing

Fixes rust-lang#106755 by restoring some of the behavior prior to rust-lang#106427

Still, I have no idea how this debug assertion can even hit while using `en-US` as primary  bundle.

r? ```@davidtwco```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidtwco davidtwco davidtwco left review comments

@klensy klensy klensy left review comments

Assignees

@davidtwco davidtwco

Labels
A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.68.0
Development

Successfully merging this pull request may close these issues.
7 participants
@mejrs @rustbot @rust-log-analyzer @davidtwco @bors @klensy @TaKO8Ki