-
Notifications
You must be signed in to change notification settings - Fork 13k
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
def_collector: rework trivial const-arg handling #133455
Conversation
613056b
to
5f05a18
Compare
see the comment on `handle_lazy_anon_const_def`
51a3ab0
to
2c0df64
Compare
// See `DefCollector::handle_lazy_anon_const_def` for more details. | ||
// | ||
// FIXME(lcnr): I believe that this shouldn't cause any issues wrt to | ||
// incremental compilation, but am not 100% confident. | ||
let parent_def = self.cx.resolver.invocation_parent(expn_id); |
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.
an existing concern. would accept this as is for now and handle it in case it is problematic
I looked a bit into ways to make this more resilient but gave up on that for now. If there's any affected code which is not covered by the existing tests, I'll look into this at that point.
i've looked into an alternative approach for a bit which changes |
This comment has been minimized.
This comment has been minimized.
closing in favor of #133468 |
always create `DefId`s for anon consts but don't use them anywhere, we intentionally don't encode them in the crate metadata. best reviewed by disabling whitespace. This pretty much reimplements rust-lang#133285 while adding the tests of rust-lang#133455. Fixes rust-lang#133064 r? `@BoxyUwU` `@compiler-errors`
see the comment on
handle_lazy_anon_const_def
r? @compiler-errors @BoxyUwU