Skip to content

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Apr 5, 2020

This is a follow-up to #70532.

r? @eddyb so I tried to find a case where the "subtyping on assignment" happens with a larger type, not just immediately with a function pointer. So far, I failed.

Here's my last attempt:

use std::cell::RefCell;

type Log<'a> = &'a RefCell<Vec<u32>>;

struct Wrap<T>(T, u32, u32);

struct GaspB<'a>(Wrap<for <'b> fn(u32, &'b str, Log<'a>)>);

enum E<'a> {
    B(GaspB<'a>, bool),
}

fn g_b(_y: u32, _ctxt: &str, _log: Log) {}

pub fn main() {
    // This has a type-changing assignment.
    E::B(GaspB(Wrap(g_b, 0, 1)), true);
    // This does not.
    let w: Wrap<for <'b, 'a> fn(u32, &'b str, Log<'a>)> = Wrap(g_b, 0, 1);
    E::B(GaspB(w), true);
}

If such a case exists, the assertion here is still wrong (as the larger type might or might not be a Scalar/ScalarPair). If such a case does not exist, the fully type-based check should actually work. Thus, this PR cannot actually make things any less correct. Am I missing something?

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 5, 2020
(ty::Ref(_, src_pointee, _), ty::Ref(_, dest_pointee, _)) => {
// After optimizations, there can be assignments that change reference mutability.
// This does not affect reference layout, so that is fine.
src_pointee == dest_pointee
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am slightly paranoid here about comparing the wrong thing (in an earlier version I accidentally compared mutabilities here instead of types, by using the wrong pattern). Is there any good way to assert that these things are types?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably use Ty::eq to call the implementation of ==, but idk if it looks pretty.
The other option is I guess ascription, i.e. (src_pointeee: Ty) == (dest_pointee: Ty).

@eddyb
Copy link
Member

eddyb commented Apr 5, 2020

That example is extremely complex, I can't follow it. It should be trivial to use subtyping:

fn simple(x: for<'a> fn(&'a ())) -> fn(&'static ()) {
    x
}

fn nested(x: (for<'a> fn(&'a ()), String)) -> (fn(&'static ()), String) {
    x
}

@eddyb
Copy link
Member

eddyb commented Apr 5, 2020

There's a trap you may have fallen into, looking again at your example:
You're constructing structs by passing g_b to them, without enforcing the type of the struct.
That means g_b will get an expected type passed down and subtype early, instead of the whole struct that contains g_b subtyping later. I get around that by not constructing anything myself.

@RalfJung
Copy link
Member Author

RalfJung commented Apr 5, 2020

I tried to get around exactly that by adding the intermediate w variable.

@RalfJung
Copy link
Member Author

RalfJung commented Apr 5, 2020

@eddyb ah, your nested indeed ICEs on Miri.

@RalfJung
Copy link
Member Author

RalfJung commented Apr 5, 2020

This PR is definitely wrong then, closing. See #70804 for the issue.

@RalfJung RalfJung closed this Apr 5, 2020
@RalfJung RalfJung deleted the miri-assign-check branch April 5, 2020 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants