-
Notifications
You must be signed in to change notification settings - Fork 14.1k
Check both where-clauses of ty aliases & assoc tys for late-bound vars in the post-expansion feature gater #149714
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
There was a problem hiding this 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!
There was a problem hiding this comment.
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
- remove the existing
visit_genericsimplementation entirely (the entirefn) - remove
check_ty_alias_after_where_clauseagain - instead, explicitly implement
visit_where_predicate_kindso it's callingcheck_late_bound_lifetime_defswith the expected arguments, followed byvisit::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)
- this should work because we automatically visit both before & after where clauses (due to
There was a problem hiding this comment.
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.
|
Reminder, once the PR becomes ready for a review, use |
|
Now I'm slightly worried that there might be other regressions caused by PR #147713 due to visitors only overriding I don't have the time to check right now. |
There was a problem hiding this 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_genericsto visit where predicates instead ofvisit_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
There was a problem hiding this comment.
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.
|
@bors r+ rollup |
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
resolves: #148627
resolves: #149233