Also emit suggestions for usages in the `non_upper_case_globals` lint #142645

Urgau · 2025-06-17T21:28:50Z

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

rustbot · 2025-06-17T21:28:55Z

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

tests/ui/lint/lint-non-uppercase-usages.stderr

compiler/rustc_lint/src/lints.rs

fmease · 2025-06-20T17:51:05Z

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-bors · 2025-06-20T17:51:09Z

⌛ Trying commit 09d0a73 with merge 4bb20cc…

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

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

rust-bors · 2025-06-20T20:20:14Z

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

rust-timer · 2025-06-20T22:07:48Z

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%)

compiler/rustc_lint/src/nonstandard_style.rs

Urgau · 2025-06-21T09:50:27Z

@bors2 try @rust-timer queue

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

rust-bors · 2025-06-21T12:20:05Z

☀️ Try build successful (CI)
Build commit: 5aafa84 (5aafa84b2abbb29683a7d7c5bc618fe756a406c5, parent: df4ad9e28b9fb973e244ebc65a8167a261b8f45e)

rust-timer · 2025-06-21T15:04:04Z

Finished benchmarking commit (5aafa84): comparison URL.

Overall result: no relevant changes - no action needed

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.

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

Instruction count

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

Max RSS (memory usage)

Results (primary -3.2%, secondary -2.5%)

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)	5.4%	[5.4%, 5.4%]	1
Improvements ✅ (primary)	-3.2%	[-3.2%, -3.2%]	1
Improvements ✅ (secondary)	-3.3%	[-6.8%, -2.1%]	11
All ❌✅ (primary)	-3.2%	[-3.2%, -3.2%]	1

Cycles

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

Binary size

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

Bootstrap: 689.042s -> 689.239s (0.03%)
Artifact size: 371.85 MiB -> 371.86 MiB (0.00%)

compiler/rustc_lint/src/nonstandard_style.rs

fmease · 2025-06-21T18:59:35Z

compiler/rustc_lint/src/nonstandard_style.rs

+                    path: &rustc_hir::Path<'v>,
+                    _id: rustc_hir::HirId,
+                ) -> Self::Result {
+                    for seg in path.segments {


Wouldn't it be sufficient to just look at the Res of the last segment or just at path.res? STATIC::something and CONST::something are always an error and will likely always be one.

We could but I don't think that would change much as we would still (I think) iterate over the segments to find the span. It's also (no longer) an expensive check anyway.

It's alright.

we would still (I think) iterate over the segments to find the span

I guess that's true for looking at path.res only. However, if you went with if let Some(final_seg) = path.segments.last() && …, you could just use final_seg.ident.span.

It's also (no longer) an expensive check anyway.

Well, we still run this logic whenever the lint actually triggers, it just doesn't show up in the perf profiles because presumably the benchmark suite doesn't contain many or any non-upper-case globals

compiler/rustc_lint/src/nonstandard_style.rs

compiler/rustc_lint/src/lints.rs

fmease · 2025-06-21T19:19:41Z

compiler/rustc_lint/src/lints.rs

+#[suggestion(
+    lint_suggestion,
+    code = "{replace}",
+    applicability = "machine-applicable",


(corner case that's unlikely to happen in practice) If the global is pub, downstream crates could depend on the name (and we wouldn't know about that here)

Yeah, we already have that issue with other lints anyway. I don't how we could solve that.

Well if the effective visibility is reachable (I don't know the compiler API for that very well), we could make the def site suggestion maybe-incorrect and stop looking for usage sites 🤷

Needn't be done in this PR

compiler-errors · 2025-06-22T14:09:59Z

r? fmease

rustbot · 2025-06-22T14:10:03Z

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

fmease · 2025-06-22T14:18:30Z

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

Urgau · 2025-06-22T14:47:04Z

@bors2 try @rust-timer queue

rust-timer · 2025-06-22T14:47:07Z

Awaiting bors try build completion.

@rustbot label: +S-waiting-on-perf

rust-bors · 2025-06-22T14:47:08Z

⌛ Trying commit 6ffd0e6 with merge 63dd59d…

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

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 assigned fee1-dead Jun 17, 2025

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

compiler-errors requested changes Jun 17, 2025

View reviewed changes

tests/ui/lint/lint-non-uppercase-usages.stderr Outdated Show resolved Hide resolved

compiler-errors assigned compiler-errors and unassigned fee1-dead Jun 17, 2025

Also emit suggestions for usages in the non_upper_case_globals lint

42bb66a

Urgau force-pushed the usage-non_upper_case_globals branch from 2589fe0 to 17f95ca Compare June 17, 2025 22:07

fmease reviewed Jun 17, 2025

View reviewed changes

compiler/rustc_lint/src/lints.rs Show resolved Hide resolved

Emit the usages suggestions as tool-only suggestions

4df9f2f

Urgau force-pushed the usage-non_upper_case_globals branch from 17f95ca to 4df9f2f Compare June 20, 2025 16:16

fmease self-assigned this Jun 20, 2025

fmease reviewed Jun 20, 2025

View reviewed changes

compiler/rustc_lint/src/lints.rs Outdated Show resolved Hide resolved

Switch non_upper_case_globals suggestions to being machine-applicable

09d0a73

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 20, 2025

This comment has been minimized.

Sign in to view

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 reviewed Jun 21, 2025

View reviewed changes

compiler/rustc_lint/src/nonstandard_style.rs Show resolved Hide resolved

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

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 21, 2025

Urgau added 2 commits June 21, 2025 13:26

Lazily collect NonUpperCaseGlobalSubTool diagnostics

8b88265

Add emit_span_lint_lazy to lazily create LintDiagnostic structs

1b5ec3f

This comment has been minimized.

Sign in to view

Urgau force-pushed the usage-non_upper_case_globals branch from 3939316 to 1b5ec3f Compare June 21, 2025 12:45

rustbot removed S-waiting-on-perf Status: Waiting on a perf run to be completed. perf-regression Performance regression. labels Jun 21, 2025

Urgau added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 21, 2025

fmease reviewed Jun 21, 2025

View reviewed changes

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 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 21, 2025

Urgau force-pushed the usage-non_upper_case_globals branch from f245ac2 to 5a670b6 Compare June 21, 2025 21:41

fmease approved these changes Jun 21, 2025

View reviewed changes

fmease unassigned compiler-errors Jun 22, 2025

Address review comments

6ffd0e6

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

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 22, 2025

Also emit suggestions for usages in the non_upper_case_globals lint #142645

Are you sure you want to change the base?

Also emit suggestions for usages in the non_upper_case_globals lint #142645

Conversation

Urgau commented Jun 17, 2025

Uh oh!

rustbot commented Jun 17, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fmease commented Jun 20, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jun 20, 2025

Uh oh!

rust-bors bot commented Jun 20, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jun 20, 2025

Overall result: ❌ regressions - please read the text below

Uh oh!

Uh oh!

Urgau commented Jun 21, 2025

Uh oh!

This comment has been minimized.

rust-bors bot commented Jun 21, 2025

Uh oh!

This comment has been minimized.

rust-timer commented Jun 21, 2025

Overall result: no relevant changes - no action needed

Uh oh!

Uh oh!

fmease Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fmease Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fmease Jun 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

compiler-errors commented Jun 22, 2025

Uh oh!

rustbot commented Jun 22, 2025

Uh oh!

fmease commented Jun 22, 2025

Uh oh!

Urgau commented Jun 22, 2025

Uh oh!

rust-timer commented Jun 22, 2025

Uh oh!

rust-bors bot commented Jun 22, 2025

Uh oh!

Uh oh!

Also emit suggestions for usages in the `non_upper_case_globals` lint #142645

Also emit suggestions for usages in the `non_upper_case_globals` lint #142645

fmease Jun 21, 2025 •

edited

Loading

fmease Jun 21, 2025 •

edited

Loading

fmease Jun 22, 2025 •

edited

Loading