Skip to content

trait_sel: add builtin impl for PointeeSized #142663

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

trait_sel: add builtin impl for PointeeSized #142663

wants to merge 1 commit into from

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Jun 18, 2025 •
edited by rustbot
Loading

Fixes #142652
Supersedes #142661

During review of #137944, we thought we could remove the builtin impl of PointeeSized as it was removed during lowering, but users can still write dyn PointeeSized and so we need the builtin impl.

r? @oli-obk

@davidtwco
trait_sel: add builtin impl for PointeeSized
cc9699d 
During review of rust-lang#137944, we thought we could remove the
builtin impl of `PointeeSized` as it was removed during lowering, but
users can still write `dyn PointeeSized` and so we need the builtin impl.
@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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver) labels Jun 18, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jun 18, 2025

Some changes occurred to the core trait solver

cc @rust-lang/initiative-trait-system-refactor

@rustbot
Copy link
Collaborator

rustbot commented Jun 18, 2025

⚠️ Warning ⚠️

  • There are issue links (such as #123) in the commit messages of the following commits.
    Please move them to the PR description, to avoid spamming the issues with references to the commit, and so this bot can automatically canonicalize them to avoid issues with subtree.

@davidtwco davidtwco mentioned this pull request Jun 18, 2025
Closed
@compiler-errors
Copy link
Member

Do we even want to allow dyn PointeeSized? Like, we don't allow dyn ?Sized today, and a dyn PointeeSized type needs to be MetaSized anyways because dyn trait types have fixed sizes and alignments via their metadata.

I'd somewhat opt to just rejecting dyn PointeeSized altogether.

@davidtwco
Copy link
Member Author

davidtwco commented Jun 19, 2025 •
edited
Loading

Do we even want to allow dyn PointeeSized? Like, we don't allow dyn ?Sized today, and a dyn PointeeSized type needs to be MetaSized anyways because dyn trait types have fixed sizes and alignments via their metadata.

I'd somewhat opt to just rejecting dyn PointeeSized altogether.

I do agree. I don't think we should only land something like #142661 which can report things like PointeeSized not being implemented for () - because that's just incorrect and would be confusing when our documentation eventually states that PointeeSized is implemented for basically everything.

I think it would make sense to add a separate error somewhere for dyn PointeeSized specifically (maybe dyn MetaSized as well if dyn ?Sized wasn't accepted previously?). So, in addition to that, would you prefer I keep the non-delayed bug? change it to a delayed bug? or add the builtin impl for PointeeSized? With a separate error, I think keeping the non-delayed bug probably makes the most sense.

@davidtwco davidtwco mentioned this pull request Jun 27, 2025
Merged
@davidtwco
Copy link
Member Author

Closing in favour of #143104

@davidtwco davidtwco closed this Jun 27, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 27, 2025
@matthiaskrgr
Rollup merge of rust-lang#143104 - davidtwco:issue-142652-dyn-pointee…
0f89445 
…sized-deny, r=compiler-errors

hir_analysis: prohibit `dyn PointeeSized`

Fixes rust-lang#142652
Supersedes rust-lang#142663

`dyn PointeeSized` is nonsensical as a `dyn PointeeSized` needs to be `MetaSized`, so lets reject it to avoid hitting code paths that expect a builtin impl for `PointeeSized`

r? `@compiler-errors`
rust-timer added a commit that referenced this pull request Jun 28, 2025
@rust-timer
Unrolled build for #143104
69b2e27 
Rollup merge of #143104 - davidtwco:issue-142652-dyn-pointeesized-deny, r=compiler-errors

hir_analysis: prohibit `dyn PointeeSized`

Fixes #142652
Supersedes #142663

`dyn PointeeSized` is nonsensical as a `dyn PointeeSized` needs to be `MetaSized`, so lets reject it to avoid hitting code paths that expect a builtin impl for `PointeeSized`

r? `@compiler-errors`
@davidtwco davidtwco deleted the issue-142652-dyn-pointeesized-reimpl branch June 28, 2025 02:18
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Jun 28, 2025
@matthiaskrgr
Rollup merge of #143104 - davidtwco:issue-142652-dyn-pointeesized-den…
1acdd9c 
…y, r=compiler-errors

hir_analysis: prohibit `dyn PointeeSized`

Fixes rust-lang/rust#142652
Supersedes rust-lang/rust#142663

`dyn PointeeSized` is nonsensical as a `dyn PointeeSized` needs to be `MetaSized`, so lets reject it to avoid hitting code paths that expect a builtin impl for `PointeeSized`

r? `@compiler-errors`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@oli-obk oli-obk

Labels
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. WG-trait-system-refactor The Rustc Trait System Refactor Initiative (-Znext-solver)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ICE: PointeeSized is removed during lowering
4 participants
@davidtwco @rustbot @compiler-errors @oli-obk