-
Notifications
You must be signed in to change notification settings - Fork 14k
misc coercion cleanups and handle safety correctly #148602
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
base: main
Are you sure you want to change the base?
Changes from 4 commits
8e8794f
05b9fb6
a6462ac
042b530
cdbdcdb
d3e3eaa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| // We have two function parameters with types: | ||
| // - `&?0` | ||
| // - `Box<for<'a> fn(<?0 as Trait<'a>>::Item)>` | ||
| // | ||
| // As the alias in the second parameter has a `?0` it is an ambig | ||
| // alias, and as it references bound vars it can't be normalized to | ||
| // an infer var. | ||
| // | ||
| // When checking function arguments we try to coerce both: | ||
| // - `&()` to `&?0` | ||
| // - `FnDef(f)` to `Box<for<'a> fn(<?0 as Trait<'a>>::Item)>` | ||
| // | ||
| // The first coercion infers `?0=()`. Previously when handling | ||
| // the second coercion we wound *re-normalize* the alias, which | ||
| // now that `?0` has been inferred allowed us to determine this | ||
| // alias is not wellformed and normalize it to some infer var `?1`. | ||
| // | ||
| // We would then see that `FnDef(f)` can't be coerced to `Box<fn(?1)>` | ||
| // and return a `TypeError` referencing this new variable `?1`. This | ||
| // then caused ICEs as diagnostics would encounter inferences variables | ||
| // from the result of normalization inside of the probe used be coercion. | ||
|
|
||
|
|
||
| trait LendingIterator { | ||
| type Item<'q>; | ||
| fn for_each(&self, _f: Box<fn(Self::Item<'_>)>) {} | ||
| } | ||
|
|
||
| fn f(_: ()) {} | ||
|
|
||
| fn main() { | ||
| LendingIterator::for_each(&(), f); | ||
| //~^ ERROR: the trait bound `(): LendingIterator` is not satisfied | ||
| //~| ERROR: the trait bound `(): LendingIterator` is not satisfied | ||
| //~| ERROR: mismatched types | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| error[E0277]: the trait bound `(): LendingIterator` is not satisfied | ||
| --> $DIR/hr_alias_normalization_leaking_vars.rs:32:31 | ||
| | | ||
| LL | LendingIterator::for_each(&(), f); | ||
| | ------------------------- ^^^ the trait `LendingIterator` is not implemented for `()` | ||
| | | | ||
| | required by a bound introduced by this call | ||
| | | ||
| help: this trait has no implementations, consider adding one | ||
| --> $DIR/hr_alias_normalization_leaking_vars.rs:24:1 | ||
| | | ||
| LL | trait LendingIterator { | ||
| | ^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| error[E0308]: mismatched types | ||
| --> $DIR/hr_alias_normalization_leaking_vars.rs:32:36 | ||
| | | ||
| LL | LendingIterator::for_each(&(), f); | ||
| | ------------------------- ^ expected `Box<fn(...)>`, found fn item | ||
| | | | ||
| | arguments to this function are incorrect | ||
| | | ||
| = note: expected struct `Box<for<'a> fn(<() as LendingIterator>::Item<'a>)>` | ||
| found fn item `fn(()) {f}` | ||
| note: method defined here | ||
| --> $DIR/hr_alias_normalization_leaking_vars.rs:26:8 | ||
| | | ||
| LL | fn for_each(&self, _f: Box<fn(Self::Item<'_>)>) {} | ||
| | ^^^^^^^^ --------------------------- | ||
|
|
||
| error[E0277]: the trait bound `(): LendingIterator` is not satisfied | ||
| --> $DIR/hr_alias_normalization_leaking_vars.rs:32:36 | ||
| | | ||
| LL | LendingIterator::for_each(&(), f); | ||
| | ^ the trait `LendingIterator` is not implemented for `()` | ||
| | | ||
| help: this trait has no implementations, consider adding one | ||
| --> $DIR/hr_alias_normalization_leaking_vars.rs:24:1 | ||
| | | ||
| LL | trait LendingIterator { | ||
| | ^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| error: aborting due to 3 previous errors | ||
|
|
||
| Some errors have detailed explanations: E0277, E0308. | ||
| For more information about an error, try `rustc --explain E0277`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| //@ check-pass | ||
|
|
||
| fn foo<T>() {} | ||
|
|
||
| fn fndef_lub_leak_check() { | ||
| macro_rules! lub { | ||
| ($lhs:expr, $rhs:expr) => { | ||
| if true { $lhs } else { $rhs } | ||
| }; | ||
| } | ||
|
|
||
| // These don't currently lub but could in theory one day. | ||
| // If that happens this test should be adjusted to use | ||
| // fn ptrs that can't be lub'd. | ||
|
Comment on lines
+12
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm what? 🤔
|
||
| let lhs = foo::<for<'a> fn(&'static (), &'a ())>; | ||
| let rhs = foo::<for<'a> fn(&'a (), &'static ())>; | ||
|
|
||
| // If we leak check then we know we should coerce these | ||
| // to `fn()`, if we don't leak check we may try to keep | ||
| // them as `FnDef`s which would result in a borrowck | ||
| // error. | ||
| let lubbed = lub!(lhs, rhs); | ||
|
|
||
| // assert that we coerced lhs/rhs to a fn ptr | ||
| is_fnptr(lubbed); | ||
| } | ||
|
|
||
| trait FnPtr {} | ||
| impl FnPtr for fn() {} | ||
| fn is_fnptr<T: FnPtr>(_: T) {} | ||
|
|
||
| fn main() {} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| fn foo<T>() {} | ||
|
|
||
| fn fndef_lub_leak_check() { | ||
| macro_rules! lub { | ||
| ($lhs:expr, $rhs:expr) => { | ||
| if true { $lhs } else { $rhs } | ||
| }; | ||
| } | ||
|
|
||
| // These don't currently lub but could in theory one day. | ||
| // If that happens this test should be adjusted to use | ||
| // fn ptrs that can't be lub'd. | ||
| let lhs = foo::<for<'a> fn(&'static (), &'a ())>; | ||
| let rhs = foo::<for<'a> fn(&'a (), &'static ())>; | ||
|
|
||
| loop {} | ||
|
|
||
| // If we leak check then we know we should coerce these | ||
| // to `fn()`, if we don't leak check we may try to keep | ||
| // them as `FnDef`s which would cause this code to compile | ||
| // as borrowck won't emit errors for deadcode. | ||
| let lubbed = lub!(lhs, rhs); | ||
|
|
||
| // assert that `lubbed` is a ZST/`FnDef` | ||
| unsafe { std::mem::transmute::<_, ()>(lubbed) } | ||
| //~^ ERROR: cannot transmute between types of different sizes | ||
| } | ||
|
|
||
| fn main() {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| error[E0512]: cannot transmute between types of different sizes, or dependently-sized types | ||
| --> $DIR/leak_check_fndef_lub_deadcode_breakage.rs:25:14 | ||
| | | ||
| LL | unsafe { std::mem::transmute::<_, ()>(lubbed) } | ||
| | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
| | | ||
| = note: source type: `fn()` (64 bits) | ||
| = note: target type: `()` (0 bits) | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
||
| For more information about this error, try `rustc --explain E0512`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| //@ check-pass | ||
| //@ compile-flags: -Znext-solver | ||
|
|
||
| #![feature(type_alias_impl_trait)] | ||
|
|
||
| // Test that when lubbing two equal closure tys with different | ||
| // structural identities (i.e. `PartialEq::eq` on `ty::Ty` would be false) | ||
| // we don't coerce-lub to a fnptr. | ||
| // | ||
| // Most of this test is involved jank to be able to leak the hidden type | ||
| // of an opaque with a hidden type of `Closure<C1, C2>`. This then allows | ||
| // us to substitute `C1` and `C2` for arbitrary types in the parent scope. | ||
| // | ||
| // See: <https://github.com/lcnr/random-rust-snippets/issues/13> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this test is cool given the amount of jank necessary to get it to work and it felt really good to get this to work. I don't think it's useful for our UI suite however and will likely be annoying to handle if its behavior ever changes (which feels likely) 🤔 I would prefer to just have that snippet somewhere else, e.g. add it to lcnr/random-rust-snippets#13 or sth, instead of keeping it in our ui test suite |
||
|
|
||
| struct WaddupGamers<T, U>(Option<T>, U); | ||
| impl<T: Leak<Unpin = U>, U> Unpin for WaddupGamers<T, U> {} | ||
| unsafe impl<T: Leak<Send = U>, U> Send for WaddupGamers<T, U> {} | ||
| pub trait Leak { | ||
| type Unpin; | ||
| type Send; | ||
| } | ||
| impl<T> Leak for (T,) { | ||
| type Unpin = T; | ||
| type Send = T; | ||
| } | ||
| fn define<C1, C2>() -> impl Sized { | ||
| WaddupGamers(None::<C1>, || ()) | ||
| } | ||
|
|
||
| fn require_unpin<T: Unpin>(_: T) {} | ||
| fn require_send<T: Send>(_: T) {} | ||
| fn mk<T>() -> T { todo!() } | ||
|
|
||
| type NameMe<T> = impl Sized; | ||
| type NameMe2<T> = impl Sized; | ||
|
|
||
| #[define_opaque(NameMe, NameMe2)] | ||
| fn leak<T>() | ||
| where | ||
| T: Leak<Unpin = NameMe<T>, Send = NameMe2<T>>, | ||
| { | ||
| require_unpin(define::<T, for<'a> fn(&'a ())>()); | ||
| require_send(define::<T, for<'a> fn(&'a ())>()); | ||
|
|
||
| // This is the actual logic for lubbing two closures | ||
| // with syntactically different `ty::Ty`s: | ||
|
|
||
| // lhs: Closure<C1=T, C2=for<'a1> fn(&'a1 ())> | ||
| let lhs = mk::<NameMe<T>>(); | ||
| // lhs: Closure<C1=T, C2=for<'a2> fn(&'a2 ())> | ||
| let rhs = mk::<NameMe2<T>>(); | ||
|
|
||
| macro_rules! lub { | ||
| ($lhs:expr, $rhs:expr) => { | ||
| if true { $lhs } else { $rhs } | ||
| }; | ||
| } | ||
|
|
||
| // Lubbed to either: | ||
| // - `Closure<C1=T, C2=for<'a> fn(&'a ())>` | ||
| // - `fn(&())` | ||
| let lubbed = lub!(lhs, rhs); | ||
|
|
||
| // Use transmute to assert the size of `lubbed` is (), i.e. | ||
| // that it is a ZST closure type not a fnptr. | ||
| unsafe { std::mem::transmute::<_, ()>(lubbed) }; | ||
| } | ||
|
|
||
| fn main() {} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| //@ edition: 2024 | ||
|
|
||
| // We avoid emitting reborrow coercions if it seems like it would | ||
| // not result in a different lifetime on the borrow. This can effect | ||
| // capture analysis resulting in borrow checking errors. | ||
|
|
||
| fn foo<'a>(b: &'a ()) -> impl Fn() { | ||
| || { | ||
| expected::<&()>(b); | ||
| } | ||
| } | ||
|
|
||
| // No reborrow of `b` is emitted which means our closure captures | ||
| // `b` by ref resulting in an upvar of `&&'a ()` | ||
| fn bar<'a>(b: &'a ()) -> impl Fn() { | ||
| || { | ||
| //~^ ERROR: closure may outlive the current function | ||
| expected::<&'a ()>(b); | ||
| } | ||
| } | ||
|
|
||
| fn expected<T>(_: T) {} | ||
|
|
||
| fn main() {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| error[E0373]: closure may outlive the current function, but it borrows `b`, which is owned by the current function | ||
| --> $DIR/structural_identity_dependent_reborrows.rs:16:5 | ||
| | | ||
| LL | || { | ||
| | ^^ may outlive borrowed value `b` | ||
| LL | | ||
| LL | expected::<&'a ()>(b); | ||
| | - `b` is borrowed here | ||
| | | ||
| note: closure is returned here | ||
| --> $DIR/structural_identity_dependent_reborrows.rs:16:5 | ||
| | | ||
| LL | / || { | ||
| LL | | | ||
| LL | | expected::<&'a ()>(b); | ||
| LL | | } | ||
| | |_____^ | ||
| help: to force the closure to take ownership of `b` (and any other referenced variables), use the `move` keyword | ||
| | | ||
| LL | move || { | ||
| | ++++ | ||
|
|
||
| error: aborting due to 1 previous error | ||
|
|
||
| For more information about this error, try `rustc --explain E0373`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add comment that regression test for #132765?