Skip to content

Fix the const impl suggestion - #159968

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Randl:fix_sugg
Jul 27, 2026
Merged

Fix the const impl suggestion#159968
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
Randl:fix_sugg

Conversation

@Randl

@Randl Randl commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

@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 Jul 26, 2026
mu001999
mu001999 reviewed Jul 26, 2026
View reviewed changes
Comment thread compiler/rustc_trait_selection/src/error_reporting/traits/fulfillment_errors.rs Outdated
oli-obk
oli-obk requested changes Jul 26, 2026
View reviewed changes

@oli-obk oli-obk left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Also add a test for unsafe impl const Trait

View changes since this review

@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 26, 2026
@rustbot

rustbot commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 26, 2026
@Randl

Randl commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@rustbot ready
Wasn't sure where to add the test since there was no explicit test for this diagnostic, so added it to comptime. Can make a separate test for both instead.

Also noted a weird thing if we have

#[rustc_comptime]
const unsafe impl Baz for Bar {

then the suggestion is to make it

#[rustc_comptime]
const const unsafe impl Baz for Bar {

Probably because #[rustc_comptime] rewrites its constness before the diagnostic runs. Not sure if it's worth fixing, though.

@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 Jul 26, 2026
@Randl

Randl commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

Never mind, somehow missed the relevant test when skimming

@oli-obk

oli-obk commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Why is comptime relevant here? Aren't the diagnostics for normal impls, too? I mean, the comptime suggestion will fail obviously as you can't have both const and comptime

@Randl

Randl commented Jul 27, 2026

Copy link
Copy Markdown
Contributor Author

Yeah, no, I accidentally added these tests in the wrong place. Let me revert them

oli-obk
oli-obk approved these changes Jul 27, 2026
View reviewed changes

@oli-obk oli-obk left a comment •
edited by rustbot
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@bors r+ rollup

View changes since this review

@rust-bors

rust-bors Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

📌 Commit 87a3711 has been approved by oli-obk

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 100. This pull request will be tested once the tree is reopened.

Reason for tree closure: spurious failures

@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 Jul 27, 2026
@JonathanBrouwer JonathanBrouwer mentioned this pull request Jul 27, 2026
Merged
rust-bors Bot pushed a commit that referenced this pull request Jul 27, 2026
@bors
Auto merge of #160021 - JonathanBrouwer:rollup-o52lFrD, r=JonathanBro…
09ee43b 
…uwer

Rollup of 14 pull requests

Successful merges:

 - #160009 (`rust-analyzer` subtree update)
 - #159833 (ci: Make the `x86_64-gnu-parallel-frontend` job non-optional)
 - #158873 (applying `/Brepro` in bootstrap on MSVC and adding a run-make test.)
 - #159777 (Enforce that we don't generate code for comptime fns)
 - #159997 (On many bindings with move error, limit the number of `Span`s)
 - #158205 (fix: defer const normalize in coherence mode)
 - #159312 (Windows implementation for stdio set/take/replace)
 - #159772 (Fix lowering of resolved const inference variables)
 - #159785 (Share _Unwind_Exception definition between native and wasm)
 - #159968 (Fix the const impl suggestion)
 - #159974 (Update thinvec to 0.2.19)
 - #160013 (tests/ui: Ignore one query cycle test in parallel frontend mode)
 - #160016 (compiletest: do not talk about JSON when the user never sees any)
 - #160018 (test suite: add ARM case to ABI-required target feature check for -Ctarget-cpu)
@rust-bors
rust-bors Bot merged commit 1fa9eaf into rust-lang:main Jul 27, 2026
13 checks passed
@rustbot rustbot added this to the 1.99.0 milestone Jul 27, 2026
rust-timer added a commit that referenced this pull request Jul 27, 2026
@rust-timer
Unrolled build for #159968
2f3f895 
Rollup merge of #159968 - Randl:fix_sugg, r=oli-obk

Fix the const impl suggestion

r? @oli-obk
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mu001999 mu001999 mu001999 left review comments
@oli-obk oli-obk oli-obk approved these changes

Assignees

@oli-obk oli-obk

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

Milestone

1.99.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Randl @rustbot @oli-obk @mu001999