Skip to content

Conversation

@reddevilmidzy
Copy link
Contributor

@reddevilmidzy reddevilmidzy commented Dec 6, 2025

resolves: #148627
resolves: #149233

@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 Dec 6, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2025

r? @nnethercote

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

@fmease fmease assigned fmease and unassigned nnethercote Dec 7, 2025
Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

View changes since this review

Copy link
Member

@fmease fmease Dec 7, 2025

Choose a reason for hiding this comment

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

Instead of duplicating what visit_generics is doing & manually visiting the after_where_clause, could you instead please

  1. remove the existing visit_generics implementation entirely (the entire fn)
  2. remove check_ty_alias_after_where_clause again
  3. instead, explicitly implement visit_where_predicate_kind so it's calling check_late_bound_lifetime_defs with the expected arguments, followed by visit::walk_where_predicate_kind(…) to continue the recursion
    • this should work because we automatically visit both before & after where clauses (due to derive(Walkable)) (and if that doesn't happen, then that's a bug)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you for the review! I've applied the suggested changes.

@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 Dec 7, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 7, 2025

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

@fmease
Copy link
Member

fmease commented Dec 7, 2025

Now I'm slightly worried that there might be other regressions caused by PR #147713 due to visitors only overriding visit_generics to visit where predicates instead of visit_where_predicate{,_kind} (they wouldn't really be at fault since the rug (some hidden assumptions) was pulled under them).

I don't have the time to check right now.

Copy link
Contributor Author

@reddevilmidzy reddevilmidzy left a comment

Choose a reason for hiding this comment

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

Now I'm slightly worried that there might be other regressions caused by PR #147713 due to visitors only overriding visit_generics to visit where predicates instead of visit_where_predicate{,_kind} (they wouldn't really be at fault since the rug (some hidden assumptions) was pulled under them).

I did a search and found that very few places override visit_generics. Those that do either already call visit_where_predicate directly or just use the default walker , so they still reach visit_where_predicate_kind.

@rustbot ready

View changes since this review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you for the review! I've applied the suggested changes.

@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 Dec 7, 2025
@fmease
Copy link
Member

fmease commented Dec 7, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Dec 7, 2025

📌 Commit d53f767 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 Dec 7, 2025
@fmease fmease changed the title Check associated type where-clauses for lifetimes Check both where-clauses of ty aliases & assoc tys for late-bound lifetimes Dec 8, 2025
@fmease fmease changed the title Check both where-clauses of ty aliases & assoc tys for late-bound lifetimes Check both where-clauses of ty aliases & assoc tys for late-bound lifetimes in the post-expansion feature gater Dec 8, 2025
@fmease fmease changed the title Check both where-clauses of ty aliases & assoc tys for late-bound lifetimes in the post-expansion feature gater Check both where-clauses of ty aliases & assoc tys for late-bound vars in the post-expansion feature gater Dec 8, 2025
bors added a commit that referenced this pull request Dec 8, 2025
Rollup of 8 pull requests

Successful merges:

 - #148935 (Fix division syntax in doc comments)
 - #149207 (Add `ilog10` result range hints)
 - #149676 (Tidying up tests/ui/issues tests [3/N])
 - #149710 (Move ambient gdb discovery from compiletest to bootstrap)
 - #149714 (Check associated type where-clauses for lifetimes)
 - #149722 (contracts: fix lowering final declaration without trailing semicolon)
 - #149736 (contracts: clean up feature flag warning duplicated across tests)
 - #149739 (mailmap: add binarycat)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 82af0ee into rust-lang:main Dec 8, 2025
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Dec 8, 2025
rust-timer added a commit that referenced this pull request Dec 8, 2025
Rollup merge of #149714 - reddevilmidzy:atd, r=fmease

Check associated type where-clauses for lifetimes

resolves: #148627
resolves: #149233
@reddevilmidzy reddevilmidzy deleted the atd branch December 8, 2025 09:44
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.

ICE: path with Res::Err but no error emitted ICE Res::Err but no error emitted

5 participants