Skip to content

Remove more Erasable impls#154917

Closed
nnethercote wants to merge 2 commits intorust-lang:mainfrom
nnethercote:rm-more-Erasable-impls
Closed

Remove more Erasable impls#154917
nnethercote wants to merge 2 commits intorust-lang:mainfrom
nnethercote:rm-more-Erasable-impls

Conversation

@nnethercote
Copy link
Copy Markdown
Contributor

@nnethercote nnethercote commented Apr 7, 2026 •
edited by rustbot
Loading

This PR moves more of the hand-written Erasable impls into the impl_erasable_for_types_with_no_type_params! macro. A follow-up to #154351. Details in individual commits.

r? @petrochenkov

@nnethercote
Shorten paths in impl_erasable_for_types_with_no_type_params.
fdce394 
The full paths are annoying me, especially when there are also some
short paths also present.

I allow the "well-known" qualifiers (`ty::`, `mir::`, `hir::`, `thir::`)
to be used and fully import all other names.

This results in a lot of reordering due to the alphabetical order
requirement.
@nnethercote
Remove more hand-written Erasable impls.
4ba565a 
The rationale is that doing `Erasable` impls via the macro is preferable
because the type in the `size_of` is the same as the impl type. In
contrast, for the hand-written ones the type in the `size_of` is a
de-genericized version of the impl type, which is a bit odd and only
works for pointer types. So the hand-written ones should only be used if
they covers many cases, and the remaining ones do.

(The removed hand-written impls are replaced with at most eight macro
entries. The remaining hand-written impls cover dozens and dozens of
cases.)
@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 Apr 7, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors bot commented Apr 7, 2026

☔ The latest upstream changes (presumably #154916) made this pull request unmergeable. Please resolve the merge conflicts.

@petrochenkov
Copy link
Copy Markdown
Contributor

petrochenkov commented Apr 7, 2026 •
edited
Loading

Both changes seem questionable to me.

In the first commit, I'd rather normalize to absolute paths (this approach is used in other similar lists like compiler\rustc_middle\src\arena.rs) to avoid guessing what is "well-known" and what is not.
Sorting by absolute paths also seems nicer.
Also less changes compared to the current state.

In the second commit I asked myself whether I'd approve a change doing the opposite thing (introducing generic trait impls instead of filling the lists manually), and I think I certainly would.
I think my rule of thumb would be "if a manually written list is shorter than the trait impl (1-2 lines), then use a list, otherwise use a trait impl".
The jump from size_of(&T) to size_of(&()) in the impls seems obvious to me, and not as something to avoid.

@petrochenkov petrochenkov 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 Apr 7, 2026
@nnethercote nnethercote closed this Apr 7, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@petrochenkov petrochenkov

Labels

T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nnethercote @petrochenkov @rustbot