Skip to content

Suggest type next to visibility for const items#157382

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
reddevilmidzy:pub-type-const
Jun 20, 2026
Merged

Suggest type next to visibility for const items#157382
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
reddevilmidzy:pub-type-const

Conversation

@reddevilmidzy

Copy link
Copy Markdown
Member

resolve: #157368

Adjusts the type const suggestion to preserve visibility ordering, so pub const is suggested as pub type const instead of type pub const.

@rustbot

rustbot commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

HIR ty lowering was modified

cc @fmease

@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 3, 2026
@rustbot

rustbot commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

r? @petrochenkov

rustbot has assigned @petrochenkov.
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

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 73 candidates
  • Random selection from 17 candidates

@petrochenkov

Copy link
Copy Markdown
Contributor

r? compiler

@rustbot rustbot assigned Kivooeo and unassigned petrochenkov Jun 3, 2026
@Kivooeo

Kivooeo commented Jun 3, 2026

Copy link
Copy Markdown
Member

Hey, thanks for a PR, I'm a bit busy this week. I will try to review within next 3 days

@rust-log-analyzer

This comment has been minimized.

@Unique-Usman

Copy link
Copy Markdown
Contributor

I went through the pr, looks good. There is already an alternative test if it is an external crate(non-local) at tests/ui/const-generics/mgca/non-local-const-without-type_const.rs, so, not needed for this pr since it exist.

@estebank

estebank commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

There is some code duplication between the branches, but that's ok. One thing that'd be nice to have is a check for whether the feature is enabled in order to add a note explaining that this change only would work on nightly (like we do in the errors when using a feature that isn't enabled).

Other than that, r=me.

@Kivooeo Kivooeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd change this match to something like this

View changes since this review

Comment thread compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
@Kivooeo

Kivooeo commented Jun 9, 2026

Copy link
Copy Markdown
Member

@rustbot author

@rustbot rustbot 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 9, 2026
@reddevilmidzy

Copy link
Copy Markdown
Member Author

There is some code duplication between the branches, but that's ok. One thing that'd be nice to have is a check for whether the feature is enabled in order to add a note explaining that this change only would work on nightly (like we do in the errors when using a feature that isn't enabled).

Other than that, r=me.

Am I understanding correctly that you mean we should add this note only when min_generic_const_args is disabled?

If that's my understanding, then for this test case, when MGCA is not enabled, the code compiles successfully instead of emitting an error with a suggestion to use type const.

@reddevilmidzy

Copy link
Copy Markdown
Member Author

thank you for the review :)
@rustbot ready

@rustbot rustbot 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 9, 2026
@Kivooeo

Kivooeo commented Jun 9, 2026

Copy link
Copy Markdown
Member

Am I understanding correctly that you mean we should add this note only when min_generic_const_args is disabled?

yes

@reddevilmidzy

Copy link
Copy Markdown
Member Author

When min_generic_const_args is not enabled, using plain const does not produce an error. Likewise, when type const is used without min_generic_const_args enabled, the compiler already emits a help message suggesting that the feature gate should be enabled.

Given that, is there anything else you had in mind that should be added to the diagnostic?

@Kivooeo

Kivooeo commented Jun 20, 2026

Copy link
Copy Markdown
Member

given that i think it's good

@bors r+ rollup

@rust-bors

rust-bors Bot commented Jun 20, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 5ea3958 has been approved by Kivooeo

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 20, 2026
rust-bors Bot pushed a commit that referenced this pull request Jun 20, 2026
Rollup of 3 pull requests

Successful merges:

 - #157976 (Improve `NumBufferTrait` associated items)
 - #157382 (Suggest `type` next to visibility for const items)
 - #158157 (Use the direct `From` in `Clone for OnceLock<T>`)
rust-bors Bot pushed a commit that referenced this pull request Jun 20, 2026
Rollup of 3 pull requests

Successful merges:

 - #157976 (Improve `NumBufferTrait` associated items)
 - #157382 (Suggest `type` next to visibility for const items)
 - #158157 (Use the direct `From` in `Clone for OnceLock<T>`)
@rust-bors rust-bors Bot merged commit 66ad954 into rust-lang:main Jun 20, 2026
12 checks passed
@rustbot rustbot added this to the 1.98.0 milestone Jun 20, 2026
@jhpratt

jhpratt commented Jun 20, 2026

Copy link
Copy Markdown
Member

@rust-timer build 279b548

@rust-timer

Copy link
Copy Markdown
Collaborator

Missing artifact for sha 279b54833e22d63eb3e4aa3e6b230e267e64844d (https://ci-artifacts.rust-lang.org/rustc-builds/279b54833e22d63eb3e4aa3e6b230e267e64844d/rustc-nightly-x86_64-unknown-linux-gnu.tar.xz); not built yet, try again later.

@jhpratt

jhpratt commented Jun 20, 2026

Copy link
Copy Markdown
Member

@rust-timer build 279b548

@rust-timer

This comment has been minimized.

@rust-timer

Copy link
Copy Markdown
Collaborator

Finished benchmarking commit (279b548): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

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

Instruction count

This perf run didn't have relevant results for this metric.

Max RSS (memory usage)

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

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

mean range count
Regressions ❌
(primary)
2.3% [2.3%, 2.3%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-2.3% [-2.5%, -2.0%] 3
Improvements ✅
(secondary)
-3.2% [-4.3%, -2.2%] 5
All ❌✅ (primary) -1.1% [-2.5%, 2.3%] 4

Cycles

Results (secondary -3.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)
-3.9% [-3.9%, -3.9%] 1
All ❌✅ (primary) - - 0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 489.308s -> 481.474s (-1.60%)
Artifact size: 390.69 MiB -> 390.69 MiB (0.00%)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

Suggestion reorders visibility incorrectly for type const items

9 participants