Stream: t-compiler/wg-mir-opt

Topic: Post Christmas hack days


oli (Dec 28 2019 at 09:57, on Zulip):

Oops overslept. Breakfast now, see you soon

Wesley Wiser (Dec 28 2019 at 10:08, on Zulip):

Got my :coffee:

Santiago Pastorino (Dec 28 2019 at 10:13, on Zulip):

gonna brew mine too :coffee:

oli (Dec 28 2019 at 11:01, on Zulip):

:wave:

Wesley Wiser (Dec 28 2019 at 11:35, on Zulip):

I'm finishing up some changes to const prop so we lint overflowing integer casts and then I'll finish up the perf improvement for large inhabited ZST arrays

Santiago Pastorino (Dec 28 2019 at 11:57, on Zulip):

rebasing #67000 :)

Wesley Wiser (Dec 28 2019 at 12:02, on Zulip):

@oli

layout.size == Size::ZERO && !layout.is_uninhabited()

Do you mean layout.size == Size::ZERO && !layout.details.abi.is_unhabited()?

oli (Dec 28 2019 at 12:03, on Zulip):

oh, yea

Wesley Wiser (Dec 28 2019 at 12:03, on Zulip):

Ok, cool

oli (Dec 28 2019 at 12:04, on Zulip):

in this case this is the conservative method, as the fallback even happens for types which are uninhabited but not from all modules

Wesley Wiser (Dec 28 2019 at 12:05, on Zulip):

What does that mean "from all modules"?

oli (Dec 28 2019 at 12:07, on Zulip):

well... if you have struct Foo(!) the .0 field is not visible outside the module, thus Foo is not uninhabited outside its defining module

oli (Dec 28 2019 at 12:08, on Zulip):

but if you look at the layout's Abi, then you get the raw information of its uninhabitedness as codegen would see it

Wesley Wiser (Dec 28 2019 at 12:09, on Zulip):

Wait, are you saying you get a different layout for Foo depending on what module you're in?

Wesley Wiser (Dec 28 2019 at 12:09, on Zulip):

No, you're talking about the logical type not the layout

Wesley Wiser (Dec 28 2019 at 12:11, on Zulip):

Ok, I think I get it

Wesley Wiser (Dec 28 2019 at 12:12, on Zulip):

Outside of the module, Foo is some opaque type because it has no public fields. So we don't know anything about it from a type system perspective. Layout however knows the internals and can see it's fully uninhabited

Wesley Wiser (Dec 28 2019 at 12:14, on Zulip):

@oli Ralf wants

Also please add a test making sure that we correctly validate things like (!, !) and struct Empty(!);.

Since the change is making things faster, wouldn't this test just show this is slow? I don't think we want to add deliberately slow tests to the suite and I don't know how we'd even make the test fail if the optimization kicked in. Do you know what he is looking for?

Wesley Wiser (Dec 28 2019 at 12:15, on Zulip):

Is there a way I can do something invalid with an uninhabited ZST type array and get an error to trigger that would have been bypassed by these changes?

oli (Dec 28 2019 at 12:15, on Zulip):

No, the test would be a compile-fail test, because validation errors out on these things

oli (Dec 28 2019 at 12:15, on Zulip):

basically Ralf wants to know they are still failing

oli (Dec 28 2019 at 12:16, on Zulip):

with your PR at the point where Ralf saw it, it would have validated a value of type Empty and said "oh this is valid"

Wesley Wiser (Dec 28 2019 at 12:16, on Zulip):

Yeah, I tested that locally and that is resolved now with the change you suggested

oli (Dec 28 2019 at 12:16, on Zulip):

so const FOO: Empty = transmute(()); would have compiled successfully

oli (Dec 28 2019 at 12:17, on Zulip):

:+1: yea so just add a few tests that const FOO: [Empty; 3] or so can never compile successfully

Wesley Wiser (Dec 28 2019 at 12:18, on Zulip):

I'm just not sure what to put on the right hand side

Wesley Wiser (Dec 28 2019 at 12:20, on Zulip):

I need to call a function that returns ! right?

Wesley Wiser (Dec 28 2019 at 12:20, on Zulip):

But I can't call functions in const contexts

Wesley Wiser (Dec 28 2019 at 12:29, on Zulip):

I think I've done something wrong since this compiles:

#![feature(const_fn)]
#![feature(const_transmute)]

const fn foo() -> ! {
    unsafe { std::mem::transmute(()) }
}

enum Empty { }

#[warn(const_err)]
const FOO: [Empty; 3] = [foo(); 3];

fn main() {
}
Wesley Wiser (Dec 28 2019 at 12:31, on Zulip):

Err maybe that's expected since the transmute is hidden in the function?

Wesley Wiser (Dec 28 2019 at 12:32, on Zulip):

This fails:

#![feature(const_transmute)]

#[derive(Clone, Copy)]
enum Empty { }

#[warn(const_err)]
const FOO: [Empty; 3] = [unsafe { std::mem::transmute(()) }; 3];

fn main() {
}

with "type validation failed: encountered a value of an uninhabited type"

oli (Dec 28 2019 at 12:32, on Zulip):

well it errors the moment you actually use FOO

oli (Dec 28 2019 at 12:32, on Zulip):

like if you add FOO; in fn main()

Wesley Wiser (Dec 28 2019 at 12:32, on Zulip):

Ah ok

oli (Dec 28 2019 at 12:33, on Zulip):

constants having errors only fails the build if they are actually used

Wesley Wiser (Dec 28 2019 at 14:34, on Zulip):

@oli I think I misunderstood what that branch was doing. I thought it skipped checking for numeric types. It actually checks the entire array at once. So the PR already checks the array for ZSTs.

Wesley Wiser (Dec 28 2019 at 15:47, on Zulip):

This sort of feels like we're re-inventing FromAnyBytes from this Pre-RFC https://internals.rust-lang.org/t/pre-rfc-v2-safe-transmute/11431

Wesley Wiser (Dec 28 2019 at 15:48, on Zulip):

Maybe we should do the simple thing for now and leverage that code later?

Wesley Wiser (Dec 28 2019 at 15:49, on Zulip):

cc @oli ^

oli (Dec 28 2019 at 16:10, on Zulip):

ok

oli (Dec 28 2019 at 16:11, on Zulip):

so basically struct with no fields and struct with no fields?

oli (Dec 28 2019 at 16:11, on Zulip):

just like you had in the beginning?

oli (Dec 28 2019 at 16:11, on Zulip):

leave a FIXME though

Wesley Wiser (Dec 28 2019 at 16:11, on Zulip):

tuple with no fields and struct with no fields yeah

Santiago Pastorino (Dec 28 2019 at 16:26, on Zulip):

btw https://github.com/rust-lang/rust/pull/67658#discussion_r361801255 unsure about this to be honest?

Last update: Apr 05 2020 at 01:00UTC