Skip to content

Conversation

Qelxiros
Copy link
Contributor

@Qelxiros Qelxiros commented Aug 27, 2025

closes #126395

@rustbot
Copy link
Collaborator

rustbot commented Aug 27, 2025

r? @BoxyUwU

rustbot has assigned @BoxyUwU.
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 Aug 27, 2025
@rust-log-analyzer

This comment has been minimized.

Copy link
Member

@BoxyUwU BoxyUwU left a comment

Choose a reason for hiding this comment

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

I think this function fundamentally works the wrong way (not ur fault, it's prexisting).

It seems to:

  1. Iterate over the list of where clauses on an item
  2. Look at where clauses that are either Type<...>: Trait<..> or Type<...>: Trait<..., Assoc<...> = ...>
  3. Collect a map (types) of all those Type<...> to the trait on the rhs of the :. e.g. if we have where u32: Trait, u64: OtherTrait, T: Trait + OtherTrait, we'd wind up with [u32 => Trait, u64 => OtherTrait, T => { Trait, OtherTrait }]
  4. We then iterate that map filtering to only keys that are type parameters, and use that to reconstruct the list of generic parameters defined on the item.

this is completely incoherent. it assumes multiple things that aren't true:

  • Type parameters are the only kind of generic parameters. You can't have const N: usize as the self type of a trait bound, so it never winds up in types so we'll never wind up putting const generics in the final generics string. playground (run test)
  • Type parameters are always used in the self type of a trait bound. This was not true on stable until very recently when we made T: ?Sized be sugar for T: MetaSized. You can still observe this with the rustc_no_implicit_bounds attr: playground (run test). Also this means that right now T: ?Sized on stable gets re-sugared as T: MetaSized which is wrong

Instead, we should unconditionally accept an AssocItem so that we can always call generics_of to get the list of generic parameters defined on the item.

You can then independently construct the types_str by iterating over the generic parameters from generics_of and fetching any bounds on them from the types map.

The existing for loop over types can be left mostly as-is, just removing the parts of it that mutate types_str.

View changes since this review

@BoxyUwU BoxyUwU 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 Sep 7, 2025
@Qelxiros
Copy link
Contributor Author

Qelxiros commented Sep 8, 2025

@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 Sep 8, 2025
@BoxyUwU
Copy link
Member

BoxyUwU commented Sep 9, 2025

@bors r+ rollup

thanks :3 if you want to fix some of the other diagnostic bugs in this fn (ignoring non-type parameters, or us resugaring T: ?Sized as T: MetaSized) then feel free to r? me on a PR doing that

@bors
Copy link
Collaborator

bors commented Sep 9, 2025

📌 Commit 2f0c103 has been approved by BoxyUwU

It is now in the queue for this repository.

@rustbot
Copy link
Collaborator

rustbot commented Sep 9, 2025

Failed to set assignee to me: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@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 Sep 9, 2025
bors added a commit that referenced this pull request Sep 9, 2025
Rollup of 6 pull requests

Successful merges:

 - #145463 (Reject invalid literal suffixes in tuple indexing, tuple struct indexing, and struct field name position)
 - #145929 (fix APITIT being treated as a normal generic parameter in suggestions)
 - #146001 (Update getopts to remove unicode-width dependency)
 - #146365 (triagebot: warn about #[rustc_intrinsic_const_stable_indirect])
 - #146366 (add approx_delta to all gamma tests)
 - #146373 (fix comments about trait solver cycle heads)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 85f3989 into rust-lang:master Sep 9, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Sep 9, 2025
rust-timer added a commit that referenced this pull request Sep 9, 2025
Rollup merge of #145929 - Qelxiros:apitit-suggestion, r=BoxyUwU

fix APITIT being treated as a normal generic parameter in suggestions

closes #126395
@Zalathar
Copy link
Contributor

Bors, this has already been merged.

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 10, 2025
@Qelxiros Qelxiros deleted the apitit-suggestion branch September 14, 2025 21:02
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Sep 16, 2025
…, r=BoxyUwU

Display ?Sized, const, and lifetime parameters in trait item suggestions across a crate boundary

context: rust-lang#145929

This fixes the MetaSized issue and adds const generics and early bound lifetimes. Late bound lifetimes are harder because they aren't returned by `generics_of`. I'm going to look into it, but there's no guarantee I'll be successful.

Fixes rust-lang#146404.

r? `@BoxyUwu`
rust-timer added a commit that referenced this pull request Sep 17, 2025
Rollup merge of #146442 - Qelxiros:trait-suggestion-generics, r=BoxyUwU

Display ?Sized, const, and lifetime parameters in trait item suggestions across a crate boundary

context: #145929

This fixes the MetaSized issue and adds const generics and early bound lifetimes. Late bound lifetimes are harder because they aren't returned by `generics_of`. I'm going to look into it, but there's no guarantee I'll be successful.

Fixes #146404.

r? `@BoxyUwu`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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.

APITIT get treated as ordinary params in "not all trait impls implemented" error
8 participants