Skip to content

Also emit suggestions for usages in the non_upper_case_globals lint #142645

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 6 commits into from
Jun 25, 2025

Conversation

Urgau
Copy link
Member

@Urgau Urgau commented Jun 17, 2025

This PR adds suggestions for all the usages of the renamed item in the warning of the non_upper_case_globals lint.

Fixes #124061

@rustbot
Copy link
Collaborator

rustbot commented Jun 17, 2025

r? @fee1-dead

rustbot has assigned @fee1-dead.
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. labels Jun 17, 2025
@Urgau Urgau force-pushed the usage-non_upper_case_globals branch from 2589fe0 to 17f95ca Compare June 17, 2025 22:07
@Urgau Urgau force-pushed the usage-non_upper_case_globals branch from 17f95ca to 4df9f2f Compare June 20, 2025 16:16
@fmease fmease self-assigned this Jun 20, 2025
@fmease
Copy link
Member

fmease commented Jun 20, 2025

This PR currently collects use sites eagerly, i.e., before the indirect call to lint_level which ultimately decides if the lint should be emitted or not. I wonder if this is noticeable perf-wise.

If this ends up perf heavy, it should be possible to turn this into a LintDiagnostic that computes these tool-only suggs lazily.

@bors2 try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Jun 20, 2025

⌛ Trying commit 09d0a73 with merge 4bb20cc

To cancel the try build, run the command @bors2 try cancel.

rust-bors bot added a commit that referenced this pull request Jun 20, 2025
Also emit suggestions for usages in the `non_upper_case_globals` lint

This PR adds suggestions for all the usages of the renamed item in the warning of the  `non_upper_case_globals` lint.

Fixes #124061
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 20, 2025
@rust-bors
Copy link

rust-bors bot commented Jun 20, 2025

☀️ Try build successful (CI)
Build commit: 4bb20cc (4bb20cc8be9c984fb2d6235080e1f44f349e6b9f, parent: 9c4ff566babe632af5e30281a822d1ae9972873b)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (4bb20cc): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
41.4% [33.7%, 55.9%] 9
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 41.4% [33.7%, 55.9%] 9

Max RSS (memory usage)

Results (primary 1.2%, secondary -4.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
3.1% [3.1%, 3.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.7% [-0.7%, -0.7%] 1
Improvements ✅
(secondary)
-4.2% [-6.0%, -2.3%] 2
All ❌✅ (primary) 1.2% [-0.7%, 3.1%] 2

Cycles

Results (primary 30.3%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
34.0% [23.9%, 47.1%] 9
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.6% [-2.6%, -2.6%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 30.3% [-2.6%, 47.1%] 10

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 692.606s -> 715.256s (3.27%)
Artifact size: 372.01 MiB -> 372.00 MiB (-0.00%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 20, 2025
@fmease fmease added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2025
@Urgau
Copy link
Member Author

Urgau commented Jun 21, 2025

@bors2 try @rust-timer queue

@rust-timer

This comment has been minimized.

@compiler-errors
Copy link
Member

r? fmease

@rustbot
Copy link
Collaborator

rustbot commented Jun 22, 2025

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

@fmease
Copy link
Member

fmease commented Jun 22, 2025

r=me with or without final comment addressed @bors rollup

@Urgau Urgau force-pushed the usage-non_upper_case_globals branch from 5a670b6 to 6ffd0e6 Compare June 22, 2025 14:45
@Urgau
Copy link
Member Author

Urgau commented Jun 22, 2025

@bors2 try @rust-timer queue

@rust-timer

This comment has been minimized.

@rust-bors
Copy link

rust-bors bot commented Jun 22, 2025

⌛ Trying commit 6ffd0e6 with merge 63dd59d

To cancel the try build, run the command @bors2 try cancel.

rust-bors bot added a commit that referenced this pull request Jun 22, 2025
Also emit suggestions for usages in the `non_upper_case_globals` lint

This PR adds suggestions for all the usages of the renamed item in the warning of the  `non_upper_case_globals` lint.

Fixes #124061
@rustbot rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 22, 2025
@rust-bors
Copy link

rust-bors bot commented Jun 22, 2025

☀️ Try build successful (CI)
Build commit: 63dd59d (63dd59d2a442bb31240549f7ed8ed7b5f038a7f3, parent: a30f1783fe136d92545423dd30b12eb619973cdb)

@rust-timer

This comment has been minimized.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (63dd59d): comparison URL.

Overall result: ❌ regressions - please read the text below

Benchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient writing along with @rustbot label: +perf-regression-triaged. If not, please fix the regressions and do another perf run. If its results are neutral or positive, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

mean range count
Regressions ❌
(primary)
2.9% [2.9%, 2.9%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.9% [2.9%, 2.9%] 1

Max RSS (memory usage)

Results (secondary -5.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-5.9% [-5.9%, -5.9%] 1
All ❌✅ (primary) - - 0

Cycles

Results (primary 2.8%, secondary 6.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
2.8% [2.8%, 2.8%] 1
Regressions ❌
(secondary)
6.9% [6.1%, 7.8%] 4
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 2.8% [2.8%, 2.8%] 1

Binary size

Results (primary 1.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

mean range count
Regressions ❌
(primary)
1.1% [1.1%, 1.1%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 1.1% [1.1%, 1.1%] 1

Bootstrap: 689.082s -> 689.565s (0.07%)
Artifact size: 371.89 MiB -> 371.87 MiB (-0.00%)

@rustbot rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 22, 2025
Comment on lines +502 to 510
let can_change_usages = if let Some(did) = did {
!cx.tcx.effective_visibilities(()).is_exported(did)
} else {
false
};

// We cannot provide meaningful suggestions
// if the characters are in the category of "Lowercase Letter".
let sub = if *name != uc {
Copy link
Member

Choose a reason for hiding this comment

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

Could you move let can_change_usages and let sub into the callback of emit_span_lint_lazy? Should fix perf

Copy link
Member Author

Choose a reason for hiding this comment

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

I could but I don't actually think there is any regression here, the only regressed crate is clap_derive-4.5.3/opt/full, no other benchmark, including any of the others clap_derive, or the previously regressed libc crate seems to affected and looking at the details the regression is in the backend, not the frontend. I think it's a spurious regression. What do you think?

@fmease fmease added the perf-regression-triaged The performance regression has been triaged. label Jun 24, 2025
@fmease
Copy link
Member

fmease commented Jun 24, 2025

Looks spurious indeed. @bors rollup- r+

@bors
Copy link
Collaborator

bors commented Jun 24, 2025

📌 Commit 6ffd0e6 has been approved by fmease

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 Jun 24, 2025
workingjubilee added a commit to workingjubilee/rustc that referenced this pull request Jun 24, 2025
…, r=fmease

Also emit suggestions for usages in the `non_upper_case_globals` lint

This PR adds suggestions for all the usages of the renamed item in the warning of the  `non_upper_case_globals` lint.

Fixes rust-lang#124061
bors added a commit that referenced this pull request Jun 24, 2025
Rollup of 9 pull requests

Successful merges:

 - #142645 (Also emit suggestions for usages in the `non_upper_case_globals` lint)
 - #142657 (mbe: Clean up code with non-optional `NonterminalKind`)
 - #142799 (rustc_session: Add a structure for keeping both explicit and default sysroots)
 - #142805 (Emit a single error when importing a path with `_`)
 - #142882 (Lazy init diagnostics-only local_names in borrowck)
 - #142883 (Add impl_trait_in_bindings tests from #61773)
 - #142943 (Don't include current rustc version string in feature removed help)
 - #142965 ([RTE-497] Ignore `c-link-to-rust-va-list-fn` test on SGX platform)
 - #142972 (Add a missing mailmap entry)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 27819a0 into rust-lang:master Jun 25, 2025
10 checks passed
@rustbot rustbot added this to the 1.90.0 milestone Jun 25, 2025
rust-timer added a commit that referenced this pull request Jun 25, 2025
Rollup merge of #142645 - Urgau:usage-non_upper_case_globals, r=fmease

Also emit suggestions for usages in the `non_upper_case_globals` lint

This PR adds suggestions for all the usages of the renamed item in the warning of the  `non_upper_case_globals` lint.

Fixes #124061
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. 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.

Fix for non_upper_case_globals should replace all usages
7 participants