fn() -> Out is a valid type for unsized types Out, and it implements FnOnce<(), Output = Out>
#82633
Comments
steffahn
changed the title
fn() -> Out is a valid type unsized types Out, and it implements FnOnce<(), Output = Out>fn() -> Out is a valid type for unsized types Out, and it implements FnOnce<(), Output = Out>
jonas-schievink
added
A-closures
|
The labeling is a bit tricky on this one. This is most definitely On the other hand, the fact that there is a type implementing |
|
@rustbot modify labels: A-associated-items, A-typesystem, T-compiler |
rustbot
added
A-associated-items
matthewjasper
added
the
I-unsound
rustbot
added
the
I-prioritize
|
I had trouble seeing the unsoundness, so here's another example: #![feature(trivial_bounds)]
#![feature(unboxed_closures)]
trait A { fn a() where Self: Sized; }
impl A for str {
fn a() where Self: Sized {
unsafe { std::hint::unreachable_unchecked() }
}
}
fn foo<F: FnOnce<()>>() where F::Output: A { F::Output::a() }
fn main() { foo::<fn() -> str>() }
yet it is anyway: |
|
And note that you can observe the unsoundness on stable ( fn weird_situation<F: FnOnce() -> str>() {
println!("if this is reachable, there’s a function type with unsized Output type `str`");
}
fn main() {
weird_situation::<fn() -> str>()
}This code compiles, but it shouldn't. |
LeSeulArtichaut
added
P-high
|
Assigning |
|
Another way to observe unsoundness: fn foo<F: FnOnce() -> R, R: ?Sized>() {}This code compiles, but it shouldn't because |
|
That code used to give a compiler error: but regressed in 1.49 such that it is no longer erroring. |
camelid
added
E-needs-bisection
|
I suspect this might be more fallout from #73905, but we'll need a bisection to be sure. |
|
Yep, it's #73905 alright: searched nightlies: from nightly-2020-10-01 to nightly-2020-12-31 bisected with cargo-bisect-rustc v0.6.0Host triple: x86_64-apple-darwin cargo bisect-rustc --regress=success --start=2020-10-01 --end=2020-12-31 -- check |
camelid
removed
the
E-needs-bisection
|
I wonder if the issue is that, quoting from #73905:
But the compiler is the one that implements |
|
I’m not 100% sure what you mean by “observing the unsoundness”. Something like
doesn’t feel necessarily unsound in and by itself. As you quoted,
which sounds pretty reasonable to me. Similarly my reproduction on stable only observes that As to ways this should be fixed: Either disallow all types |
|
In particular, I don’t think that #![feature(unboxed_closures)]
fn foo<F: FnOnce<()>>()
{
let _: Vec<F::Output> = vec![];
}
fn main() {
foo::<fn() -> str>()
}Similarly, reading static memory and segfaulting can be archieved all the way back to e.g. CLICK TO EXPAND#![feature(unboxed_closures)]
trait Foo<T> {
fn foo(&mut self) -> T;
}
impl<T> Foo<T> for Option<T> {
fn foo(&mut self) -> T {
self.take().unwrap()
}
}
trait Bar<T> {
fn bar(&mut self) -> T;
}
impl<S, T> Bar<T> for S {
fn bar(&mut self) -> T {
unimplemented!()
}
}
fn transmute<S, T>(x: S) -> T {
let x: &mut (Foo<S>) = &mut Some(x);
step_one::<fn() -> _, T>(x)
}
fn step_one<F: FnOnce<()>, T>(x: &mut F::Output) -> T {
let x: &mut Bar<T> = x; // casts &mut dyn Foo<S> to &mut dyn Bar<T>
// compiler is happy since `F::Output: Sized`
// on the other hand, the compiler later implements the actual
// cast as a no-op since the reference `x` already contains a vtable pointer
x.bar() // effectively calls x.foo() from the vtable
}
static X: usize = 12345;
fn main() {
let x = transmute::<&'static usize, &'static [usize; 1000000 as _]>(&X);
assert_eq!(x[0], 12345); // works, nice!
// let’s keep reading the rest of static memory and whatever follows:
println!("{:?}", &x[1..]);
// eventually segfaults
}I would believe that the bug is not a regression but just always has been there. @rustbot modify labels: -regression-from-stable-to-stable |
rustbot
removed
the
regression-from-stable-to-stable
|
@rustbot label +I-nominated |
|
Starting a poll to check lang-team consensus: @rfcbot poll T-lang Should we prohibit types like |
|
Team member @joshtriplett has asked teams: T-lang, for consensus on:
|
|
Clarification: this does not prevent us from potentially improving support for unsized values in the future, it just prevents such function types from existing until we do. |
|
ping @estebank Do you still have the bandwidth to implement this? Looks like we have consensus. |
|
(we may want to warning-cycle the prohibition, to be clear) |
|
@joshtriplett I do. I'll cook up a PR. |
|
@joshtriplett published #83915. Sorry it took me so long, I had multiple false starts. It might need some changes to be good enough for merging, but that PR enforces As follow up work, I would like to change the spans to be tighter/easier to understand, but for now this is good enough: |
|
Is there any way the exceptions for |
|
@joshtriplett I could envision that being the case, but haven't come up with a way to trigger it. |
In a `fn() -> Out` bound, enforce `Out: Sized` to avoid unsoundness. Fix rust-lang#82633.
|
Niko presented an alternative approach. |
jackh726
added
the
WG-traits
oli-obk
added
T-types
|
fixed by #100096 which removes the builtin marking as needs test |
lcnr
added
the
E-needs-test
|
Feel free to take https://github.com/rust-lang/rust/pull/83915/files#diff-8c64c576ccaceb816e71d2279a6ee4bf79211bc06f55c46dda3f98a18748ad7a to close this. |
estebank
added
the
E-easy
|
@rustbot claim For adding the test. |
fn() -> Outis a valid type, even for unsized typesOut, and it implementsFnOnce<(), Output = Out>, even thoughFnOnce::Outputis not?Sized. So far, I haven’t found a way to turn this into unsoundness without usingunboxed_closures.(Playground)
code after
rustfmtErrors:
The text was updated successfully, but these errors were encountered: