Skip to content
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

Jump to bottom

Improve diagnostic-related lints: untranslatable_diagnostic & diagnostic_outside_of_impl #128941

Merged
merged 3 commits into from
Aug 22, 2024
Merged

Improve diagnostic-related lints: untranslatable_diagnostic & diagnostic_outside_of_impl #128941

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

@bors r+

@bors
Copy link
Contributor

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
6 checks passed
@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
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@davidtwco davidtwco davidtwco approved these changes

Assignees

@davidtwco davidtwco

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.
Projects
None yet
Milestone
1.82.0
Development

Successfully merging this pull request may close these issues.
4 participants
@GrigorenkoPV @rustbot @davidtwco @bors