Skip to content

remove extraneous note on UnableToRunDsymutil diagnostic #124414

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
May 2, 2024

Conversation

lqd
Copy link
Member

@lqd lqd commented Apr 26, 2024

If I understand this FIXME correctly, it seems we don't yet validate subdiagnostics, so #[note] and co in the #[derive(Diagnostic] item could be out-of-sync with the fluent message, without causing compile errors.

It was the case for rustc_codegen_ssa::errors::UnableToRunDsymutil, causing the ICE in #124392.

I've grepped and scripted my way through most of our diagnostics structs and fluent bundles and the above was the only such extraneous #[note]/#[note(name)]/#[help]/#[warning] I could find, so hopefully there aren't many others like it.

I haven't checked if the opposite can happen, a .note = in a fluent message that is lacking a corresponding #[note] on the struct and not causing an error, but maybe it's possible?

r? @davidtwco
fixes #124392

@rustbot rustbot added 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 Apr 26, 2024
@davidtwco
Copy link
Member

If I understand this FIXME correctly, it seems we don't yet validate subdiagnostics, so #[note] and co in the #[derive(Diagnostic] item could be out-of-sync with the fluent message, without causing compile errors.

It was the case for rustc_codegen_ssa::errors::UnableToRunDsymutil, causing the ICE in #124392.

I've grepped and scripted my way through most of our diagnostics structs and fluent bundles and the above was the only such extraneous #[note]/#[note(name)]/#[help]/#[warning] I could find, so hopefully there aren't many others like it.

I'm surprised this isn't caught actually - it should be a compiler error. We generate a constant for each Fluent message and attribute (basically just containing the Fluent slug), and then that's what we reference in the diagnostic derive.

A missing .note should mean a missing constant and a compiler error.

The check that you've found is for checking if each Fluent message doesn't reference any fields on the diagnostic type that don't exist - so that's actually checking something different than this case, if I understand everything correctly. That doesn't work for subdiagnostics because the field being referenced by the Fluent message might be part of the parent diagnostic.

I haven't checked if the opposite can happen, a .note = in a fluent message that is lacking a corresponding #[note] on the struct and not causing an error, but maybe it's possible?

This isn't possible - extraneous messages in Fluent don't cause any issues. I think we do have checks for this for top-level messages, but I guess not for Fluent attributes.

@davidtwco
Copy link
Member

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented May 2, 2024

📌 Commit 1367827 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 May 2, 2024
fmease added a commit to fmease/rust that referenced this pull request May 2, 2024
remove extraneous note on `UnableToRunDsymutil` diagnostic

If I understand [this FIXME](https://github.com/rust-lang/rust/blob/1367827eac3d813a261a4c444037af9736996daa/compiler/rustc_macros/src/diagnostics/diagnostic.rs#L205) correctly, it seems we don't yet validate subdiagnostics, so `#[note]` and co in the `#[derive(Diagnostic]` item could be out-of-sync with the fluent message, without causing compile errors.

It was the case for `rustc_codegen_ssa::errors::UnableToRunDsymutil`, causing the ICE in rust-lang#124392.

I've grepped and scripted my way through most of our diagnostics structs and fluent bundles and the above was the only such extraneous `#[note]`/`#[note(name)]`/`#[help]`/`#[warning]` I could find, so hopefully there aren't many others like it.

I haven't checked if the opposite can happen, a `.note = ` in a fluent message that is lacking a corresponding `#[note]` on the struct and not causing an error, but maybe it's possible?

r? `@davidtwco`
fixes rust-lang#124392
bors added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2024
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#124138 (Ignore LLVM ABI in dlltool tests since those targets don't use dlltool)
 - rust-lang#124414 (remove extraneous note on `UnableToRunDsymutil` diagnostic)
 - rust-lang#124579 (Align: add bytes_usize and bits_usize)
 - rust-lang#124622 (Cleanup: Rid the `rmake` test runners of `extern crate run_make_support;`)
 - rust-lang#124623 (shallow resolve in orphan check)
 - rust-lang#124624 (Use `tcx.types.unit` instead of `Ty::new_unit(tcx)`)
 - rust-lang#124627 (interpret: hide some reexports in rustdoc)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit e9e2ebb into rust-lang:master May 2, 2024
@rustbot rustbot added this to the 1.80.0 milestone May 2, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request May 2, 2024
Rollup merge of rust-lang#124414 - lqd:subdiagnostics, r=davidtwco

remove extraneous note on `UnableToRunDsymutil` diagnostic

If I understand [this FIXME](https://github.com/rust-lang/rust/blob/1367827eac3d813a261a4c444037af9736996daa/compiler/rustc_macros/src/diagnostics/diagnostic.rs#L205) correctly, it seems we don't yet validate subdiagnostics, so `#[note]` and co in the `#[derive(Diagnostic]` item could be out-of-sync with the fluent message, without causing compile errors.

It was the case for `rustc_codegen_ssa::errors::UnableToRunDsymutil`, causing the ICE in rust-lang#124392.

I've grepped and scripted my way through most of our diagnostics structs and fluent bundles and the above was the only such extraneous `#[note]`/`#[note(name)]`/`#[help]`/`#[warning]` I could find, so hopefully there aren't many others like it.

I haven't checked if the opposite can happen, a `.note = ` in a fluent message that is lacking a corresponding `#[note]` on the struct and not causing an error, but maybe it's possible?

r? ``@davidtwco``
fixes rust-lang#124392
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
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
Development

Successfully merging this pull request may close these issues.

called Result::unwrap() on an Err value: failed while formatting fluent string codegen_ssa_unable_to_run_dsymutil
4 participants