-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix: Implement Interner::impl_specializes()
#20893
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
fix: Implement Interner::impl_specializes()
#20893
Conversation
Using specialization logic ported from rustc.
| FIXME(next-solver): | ||
| This does not work currently because I replaced homemade selection with selection by the trait solver; | ||
| This will work once we implement `Interner::impl_specializes()` properly. | ||
| "] |
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.
Oh, it's cool that this is finally working again
| return false; | ||
| } | ||
|
|
||
| // FIXME: Check impl constness (when we implement const impls). |
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.
BTW I doubt we would ever implement const trait impls. They are very complex and (currently) volatile in rustc codebase and I guess they wouldn't give us many things other than some constness diagnostics (I might be wrong as I'm not an expert 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.
When they are stabilized, I expect us to eventually implement them, and yeah for the diagnostic. Not now for sure. But a FIXME doesn't hurt :)
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.
Yeah, I think FIXME is good here, at least we can resume on this point when we need it 😄
|
BTW, a nice side bonus of this PR is that for the first time, goto def on std methods that have specialization will take you to the specialized impl, if applicable! |
|
changelog fixup #20329 |
…n-ns fix: Implement `Interner::impl_specializes()`
|
This seems to have regressed memory usage considerably fwiw |
|
Oh, it's a query (and an interned one), and it seems the solver is calling it a lot. I wonder if we should not make this a query (it is in rustc). At worst, we can revert this, since I believe not providing Alternatively, as I typed this I think that we can call the query only if the crate has |
|
I created #20921 for that; let's see if it improves the situation. |
|
Okay, it worked partially. Memory was reduced, but not back to the level of before this PR. Do that many impls overlap with std/core impls? |
Using specialization logic ported from rustc.