Skip to content

trait_sel: delay bug for removed pointeesized #142661

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

Closed

Conversation

davidtwco
Copy link
Member

@davidtwco davidtwco commented Jun 18, 2025

Fixes #142652

This code path isn't hit during normal compilation, only in error reporting, so delaying the bug is sufficient. It might be possible to trigger this with dyn PointeeSized and code that doesn't error but I couldn't work out how to make that happen. This can only happen when you use the feature so it's not an urgent thing and if we find another case then I can fix that then.

r? @oli-obk

@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 Jun 18, 2025
@oli-obk
Copy link
Contributor

oli-obk commented Jun 18, 2025

Won't that cause the following to ICE without an error?

#![feature(sized_hierarchy)]
use std::marker::PointeeSized;

type Foo = dyn PointeeSized;

Or sth similar that doesn't cause other errors

@davidtwco
Copy link
Member Author

davidtwco commented Jun 18, 2025

Won't that cause the following to ICE without an error?

#![feature(sized_hierarchy)]
use std::marker::PointeeSized;

type Foo = dyn PointeeSized;

Or sth similar that doesn't cause other errors

This doesn't error, it just compiles. If I add a use of Foo, such as..

#![feature(sized_hierarchy)]
use std::marker::PointeeSized;

type Foo = dyn PointeeSized;

fn foo(f: &Foo) {}

fn main() {
    foo(&());
}

..then I do get an error, but not an ICE. It's an incorrect error, because () should implement PointeeSized:

error[E0277]: values of type `()` may or may not have a size
 --> f.rs:9:9
  |
9 |     foo(&());
  |         ^^^ may or may not have a known size
  |
  = help: the trait `PointeeSized` is not implemented for `()`
  = note: required for the cast from `&()` to `&(dyn PointeeSized + 'static)`

warning: unused variable: `f`
 --> f.rs:6:8
  |
6 | fn foo(f: &Foo) {}
  |        ^ help: if this is intentional, prefix it with an underscore: `_f`
  |
  = note: `#[warn(unused_variables)]` on by default

error: aborting due to 1 previous error; 1 warning emitted

For more information about this error, try `rustc --explain E0277`.

Only way to correct that is to re-implement the builtin impl for PointeeSized that we wanted to avoid.

@davidtwco
Copy link
Member Author

https://github.com/davidtwco/rust/tree/issue-142652-dyn-pointeesized-reimpl re-adds that builtin impl and makes the snippet from the previous comment pass, can replace this with that if we want.

This code path isn't hit during normal compilation, only in error
reporting, so delaying the bug is sufficient. It might be possible to
trigger this with `dyn PointeeSized` and code that doesn't error but no
such example could be found at the time of writing.
@davidtwco davidtwco force-pushed the issue-142652-dyn-pointeesized branch from 251eec8 to e7d308c Compare June 18, 2025 09:19
@oli-obk
Copy link
Contributor

oli-obk commented Jun 18, 2025

Ah right. So what about a coercion that should succeed?

#![feature(sized_hierarchy)]

use std::marker::PointeeSized;

fn main() {
      let x: Box<dyn PointeeSized + Send> = todo!();
      let y: Box<dyn PointeeSized> = x;
//~^ ERROR mismatched types
}

or maybe somemething else. I'm fine just not doing anything here and waiting for someone to figure out how to hit that delayed bug without an error, but ideally we'd try a bit.

fn foo<T: PointeeSized>(t: Box<T>) -> Box<dyn PointeeSized> { t }

maybe?

@davidtwco
Copy link
Member Author

davidtwco commented Jun 18, 2025

#![feature(sized_hierarchy)]

use std::marker::PointeeSized;

fn main() {
      let x: Box<dyn PointeeSized + Send> = todo!();
      let y: Box<dyn PointeeSized> = x;
//~^ ERROR mismatched types
}

No error on this one.

fn foo<T: PointeeSized>(t: Box<T>) -> Box<dyn PointeeSized> { t }

We do get an error on this one, but largely because Box requires MetaSized, only because of Deref::Target, not because Box shouldn't be T: PointeeSized

error[E0277]: the size for values of type `T` cannot be known
   --> g.rs:5:28
    |
5   | fn foo<T: PointeeSized>(t: Box<T>) -> Box<dyn PointeeSized> {
    |                            ^^^^^^ doesn't have a known size
    |
note: required by a bound in `Box`
   --> /home/davwoo02/rust0/library/alloc/src/boxed.rs:232:5
    |
231 | pub struct Box<
    |            --- required by a bound in this struct
232 |     T: ?Sized,
    |     ^ required by this bound in `Box`
help: consider further restricting type parameter `T` with unstable trait `MetaSized`
    |
5   | fn foo<T: PointeeSized + std::marker::MetaSized>(t: Box<T>) -> Box<dyn PointeeSized> {
    |                        ++++++++++++++++++++++++

error[E0277]: the size for values of type `T` cannot be known at compilation time
 --> g.rs:6:5
  |
5 | fn foo<T: PointeeSized>(t: Box<T>) -> Box<dyn PointeeSized> {
  |        - this type parameter needs to be `Sized`
6 |     t
  |     ^ doesn't have a size known at compile-time
  |
  = note: required for the cast from `Box<T>` to `Box<(dyn PointeeSized + 'static)>`

error[E0277]: the size for values of type `T` cannot be known
 --> g.rs:6:5
  |
6 |     t
  |     ^ doesn't have a known size
  |
  = note: required for the cast from `Box<T>` to `Box<(dyn PointeeSized + 'static)>`
help: consider further restricting type parameter `T` with unstable trait `MetaSized`
  |
5 | fn foo<T: PointeeSized + std::marker::MetaSized>(t: Box<T>) -> Box<dyn PointeeSized> {
  |                        ++++++++++++++++++++++++

error[E0277]: values of type `T` may or may not have a size
 --> g.rs:6:5
  |
6 |     t
  |     ^ may or may not have a known size
  |
  = note: required for the cast from `Box<T>` to `Box<(dyn PointeeSized + 'static)>`

error: aborting due to 4 previous errors

@oli-obk
Copy link
Contributor

oli-obk commented Jun 18, 2025

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Jun 18, 2025

📌 Commit e7d308c has been approved by oli-obk

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 18, 2025
@davidtwco
Copy link
Member Author

Do we want to do anything about this incorrectly erroring?

#![feature(sized_hierarchy)]
use std::marker::PointeeSized;

type Foo = dyn PointeeSized;

fn foo(f: &Foo) {}

fn main() {
    foo(&());
}

@oli-obk
Copy link
Contributor

oli-obk commented Jun 18, 2025

probably, but either improving the error or collecting all the info on why we wanted to avoid the builtin impl in the first place and discussing that

@davidtwco
Copy link
Member Author

probably, but either improving the error or collecting all the info on why we wanted to avoid the builtin impl in the first place and discussing that

I think avoiding the builtin impl was just because we thought it couldn't be hit with the PointeeSized bound being removed in lowering, it just seems like that was an incorrect assumption with these new examples

@oli-obk
Copy link
Contributor

oli-obk commented Jun 18, 2025

Right, in that case let's add it

@davidtwco
Copy link
Member Author

@bors r-

Closing in favour of #142663

@davidtwco davidtwco closed this Jun 18, 2025
@davidtwco davidtwco deleted the issue-142652-dyn-pointeesized branch June 18, 2025 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ICE: PointeeSized is removed during lowering
4 participants