Skip to content

Conversation

@ChayimFriedman2
Copy link
Contributor

Using specialization logic ported from rustc.

Using specialization logic ported from rustc.
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 23, 2025
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.
"]
Copy link
Member

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

@ShoyuVanilla ShoyuVanilla added this pull request to the merge queue Oct 24, 2025
return false;
}

// FIXME: Check impl constness (when we implement const impls).
Copy link
Member

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 😅 )

Copy link
Contributor Author

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 :)

Copy link
Member

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 😄

Merged via the queue into rust-lang:master with commit 5844df0 Oct 24, 2025
15 checks passed
@rustbot rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 24, 2025
@ChayimFriedman2 ChayimFriedman2 deleted the specialization-ns branch October 24, 2025 04:36
@ChayimFriedman2
Copy link
Contributor Author

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!

@lnicola
Copy link
Member

lnicola commented Oct 24, 2025

changelog fixup #20329

mendelsshop pushed a commit to mendelsshop/rust-analyzer that referenced this pull request Oct 27, 2025
…n-ns

fix: Implement `Interner::impl_specializes()`
@Veykril
Copy link
Member

Veykril commented Oct 27, 2025

This seems to have regressed memory usage considerably fwiw

@ChayimFriedman2
Copy link
Contributor Author

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 impl_specializes() can not result in failures to trait resolution, only to selection, and it's not much of a problem for us until we implement const eval more seriously.

Alternatively, as I typed this I think that we can call the query only if the crate has #![feature(specialization)]. This should limit the impact considerably.

@ChayimFriedman2
Copy link
Contributor Author

ChayimFriedman2 commented Oct 27, 2025

I created #20921 for that; let's see if it improves the situation.

@ChayimFriedman2
Copy link
Contributor Author

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants