Skip to content

Improve diagnostic-related lints: untranslatable_diagnostic & diagnostic_outside_of_impl #128941

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
Merged
bors merged 3 commits into from
Aug 22, 2024
Merged

Improve diagnostic-related lints: untranslatable_diagnostic & diagnostic_outside_of_impl #128941

bors merged 3 commits into from
Aug 22, 2024

Conversation

@GrigorenkoPV
Copy link
Contributor

@GrigorenkoPV GrigorenkoPV commented Aug 10, 2024 •
edited
Loading

Summary:

  • Made untranslatable_diagnostic point to problematic arguments instead of the function call
    (I found this misleading while working on some A-translation PRs: my first impression was that
    the methods themselves were not translation-aware and needed to be changed,
    while in reality the problem was with the hardcoded strings passed as arguments).
  • Made the shared pass of untranslatable_diagnostic & diagnostic_outside_of_impl more efficient.

@rustbot label D-imprecise-spans A-translation
@GrigorenkoPV
Add more test cases for untranslatable_diagnostic lint
10ef666
@GrigorenkoPV
untranslatable_diagnostic lint: point at the untranslated thing
3cc2a6f 
and not the function/method call
@GrigorenkoPV
Refactor: diagnostic_outside_of_impl, untranslatable_diagnostic
e94a4ee 
1. Decouple them. 2. Make logic around `diagnostic_outside_of_impl`'s early exits simpler. 3. Make `untranslatable_diagnostic` run one loop instead of two and not allocate an intermediate vec. 4. Overall, reduce the amount of code executed when the lints do not end up firing.
@rustbot
Copy link
Collaborator

rustbot commented Aug 10, 2024

r? @davidtwco

rustbot has assigned @davidtwco.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer
@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. A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic D-imprecise-spans Diagnostics: spans don't point to exactly the erroneous code labels Aug 10, 2024
@davidtwco
Copy link
Member

davidtwco commented Aug 21, 2024

@bors r+
@bors
Copy link
Collaborator

bors commented Aug 21, 2024

📌 Commit e94a4ee 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 Aug 21, 2024
@matthiaskrgr matthiaskrgr mentioned this pull request Aug 21, 2024
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 21, 2024
@bors
Auto merge of rust-lang#129365 - matthiaskrgr:rollup-ebwx6ya, r=matth…
562df53 
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127279 (use old ctx if has same expand environment during decode span) - rust-lang#127945 (Implement `debug_more_non_exhaustive`) - rust-lang#128941 ( Improve diagnostic-related lints: `untranslatable_diagnostic` & `diagnostic_outside_of_impl`) - rust-lang#129070 (Point at explicit `'static` obligations on a trait) - rust-lang#129187 (bootstrap: fix clean's remove_dir_all implementation) - rust-lang#129231 (improve submodule updates) - rust-lang#129264 (Update `library/Cargo.toml` in weekly job) - rust-lang#129284 (rustdoc: animate the `:target` highlight) - rust-lang#129302 (compiletest: use `std::fs::remove_dir_all` now that it is available) r? `@ghost` `@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 21, 2024
@bors
Auto merge of rust-lang#129365 - matthiaskrgr:rollup-ebwx6ya, r=matth…
02557f6 
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127279 (use old ctx if has same expand environment during decode span) - rust-lang#127945 (Implement `debug_more_non_exhaustive`) - rust-lang#128941 ( Improve diagnostic-related lints: `untranslatable_diagnostic` & `diagnostic_outside_of_impl`) - rust-lang#129070 (Point at explicit `'static` obligations on a trait) - rust-lang#129187 (bootstrap: fix clean's remove_dir_all implementation) - rust-lang#129231 (improve submodule updates) - rust-lang#129264 (Update `library/Cargo.toml` in weekly job) - rust-lang#129284 (rustdoc: animate the `:target` highlight) - rust-lang#129302 (compiletest: use `std::fs::remove_dir_all` now that it is available) r? `@ghost` `@rustbot` modify labels: rollup
jieyouxu added a commit to jieyouxu/rust that referenced this pull request Aug 22, 2024
@jieyouxu
Rollup merge of rust-lang#128941 - GrigorenkoPV:internal-diagnostic-l…
126a5c2 
…ints, r=davidtwco Improve diagnostic-related lints: `untranslatable_diagnostic` & `diagnostic_outside_of_impl` Summary: - Made `untranslatable_diagnostic` point to problematic arguments instead of the function call (I found this misleading while working on some `A-translation` PRs: my first impression was that the methods themselves were not translation-aware and needed to be changed, while in reality the problem was with the hardcoded strings passed as arguments). - Made the shared pass of `untranslatable_diagnostic` & `diagnostic_outside_of_impl` more efficient. ``@rustbot`` label D-imprecise-spans A-translation
@jieyouxu jieyouxu mentioned this pull request Aug 22, 2024
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2024
@bors
Auto merge of rust-lang#129365 - matthiaskrgr:rollup-ebwx6ya, r=matth…
739b1fd 
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#127279 (use old ctx if has same expand environment during decode span) - rust-lang#127945 (Implement `debug_more_non_exhaustive`) - rust-lang#128941 ( Improve diagnostic-related lints: `untranslatable_diagnostic` & `diagnostic_outside_of_impl`) - rust-lang#129070 (Point at explicit `'static` obligations on a trait) - rust-lang#129187 (bootstrap: fix clean's remove_dir_all implementation) - rust-lang#129231 (improve submodule updates) - rust-lang#129264 (Update `library/Cargo.toml` in weekly job) - rust-lang#129284 (rustdoc: animate the `:target` highlight) - rust-lang#129302 (compiletest: use `std::fs::remove_dir_all` now that it is available) r? `@ghost` `@rustbot` modify labels: rollup
@bors bors merged commit 47af700 into rust-lang:master Aug 22, 2024
@rustbot rustbot added this to the 1.82.0 milestone Aug 22, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 22, 2024
@rust-timer
Unrolled build for rust-lang#128941
c33147e 
Rollup merge of rust-lang#128941 - GrigorenkoPV:internal-diagnostic-lints, r=davidtwco Improve diagnostic-related lints: `untranslatable_diagnostic` & `diagnostic_outside_of_impl` Summary: - Made `untranslatable_diagnostic` point to problematic arguments instead of the function call (I found this misleading while working on some `A-translation` PRs: my first impression was that the methods themselves were not translation-aware and needed to be changed, while in reality the problem was with the hardcoded strings passed as arguments). - Made the shared pass of `untranslatable_diagnostic` & `diagnostic_outside_of_impl` more efficient. `@rustbot` label D-imprecise-spans A-translation
@GrigorenkoPV GrigorenkoPV deleted the internal-diagnostic-lints branch August 22, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-translation Area: Translation infrastructure, and migrating existing diagnostics to SessionDiagnostic D-imprecise-spans Diagnostics: spans don't point to exactly the erroneous code 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.

4 participants

@GrigorenkoPV @rustbot @davidtwco @bors