Stream: t-compiler/const-eval

Topic: Promoting consts with `if`


ecstatic-morse (Sep 24 2019 at 20:19, on Zulip):

Even when const if is implemented, we don't want to promote code like &(if true { 5 } else { 6 }). However, we may want to do promotion when that if statement appears in the definition of a const.

const BRANCHY: i32 = if true { 5 } else { 6 };
fn main() {
    let x: &'static i32 = &BRANCHY;
}
ecstatic-morse (Sep 24 2019 at 20:21, on Zulip):

If we do not want to promote BRANCHY in the previous code, we have a bit of a problem with backwards compatibility because of the way we lower && and || to allow them in consts.

ecstatic-morse (Sep 24 2019 at 20:22, on Zulip):

Namely, this code compiles on stable but would stop compiling if we removed the destroy control flow hack that allows short circuiting logic in consts.

Matthew Jasper (Sep 24 2019 at 20:22, on Zulip):

I think that we're happy to promote such consts. We don't promote most const fn, but do promote consts that use them.

Matthew Jasper (Sep 24 2019 at 20:23, on Zulip):

You've not linked to you're actual example there.

ecstatic-morse (Sep 24 2019 at 20:24, on Zulip):

Does "this code" not have a hyperlink?

Matthew Jasper (Sep 24 2019 at 20:24, on Zulip):

It's linking to https://play.rust-lang.org/?version=stable&mode=debug&edition=2018

ecstatic-morse (Sep 24 2019 at 20:25, on Zulip):
const BOO: bool = (true && false);

fn main() {
    let x: &'static bool = &BOO;
}
ecstatic-morse (Sep 24 2019 at 20:25, on Zulip):

oh duh sorry

oli (Sep 24 2019 at 20:26, on Zulip):

yea, we promote constants by looking at their type, nothing else

oli (Sep 24 2019 at 20:26, on Zulip):

(I think?)

ecstatic-morse (Sep 24 2019 at 20:26, on Zulip):

I guess the question is: is there any time where we don't want to promote &CONST?

ecstatic-morse (Sep 24 2019 at 20:26, on Zulip):

So part of the result of the mir_const_qualif query is the qualifications of the return place of that const

oli (Sep 24 2019 at 20:26, on Zulip):

so even const FOO: Option<Cell<u32>> = None; should not get promoted

oli (Sep 24 2019 at 20:27, on Zulip):

I'm wrong

oli (Sep 24 2019 at 20:27, on Zulip):

obviously

oli (Sep 24 2019 at 20:27, on Zulip):

uh

ecstatic-morse (Sep 24 2019 at 20:28, on Zulip):

https://github.com/rust-lang/rust/blob/6ef275e6c3cb1384ec78128eceeb4963ff788dca/src/librustc_mir/transform/qualify_consts.rs#L1012

oli (Sep 24 2019 at 20:28, on Zulip):

yea

oli (Sep 24 2019 at 20:28, on Zulip):

but that should still work out correctly in your scheme, right?

oli (Sep 24 2019 at 20:28, on Zulip):

we'd get the worst qualifs of both arms

oli (Sep 24 2019 at 20:29, on Zulip):

const FOO: Option<Cell<u32>> = if false { Some(Cell::new(42)) } else {None}; will prevent FOO from getting promoted

ecstatic-morse (Sep 24 2019 at 20:30, on Zulip):

Yes, this is only tangentially related to dataflow-based qualification

ecstatic-morse (Sep 24 2019 at 20:31, on Zulip):

At the moment, const BRANCHY: i32 = if true { 5 } else { 6 }; will also (I believe) be marked as unpromotable, because the return place is assigned to more than once.

ecstatic-morse (Sep 24 2019 at 20:33, on Zulip):

The question is whether we want that behavior or if we want to mark BRANCHY as promotable

oli (Sep 24 2019 at 20:35, on Zulip):

so this is a fun topic, because I've repeatedly tried to make the promotability depend on the value of the constant, so we could just eval it instead of looking at it

oli (Sep 24 2019 at 20:36, on Zulip):

then we'd only have to keep looking at associated constants

oli (Sep 24 2019 at 20:36, on Zulip):

@eddyb hates me a bit more every time I bring it up, so obviously I'm gonna ping them

oli (Sep 24 2019 at 20:36, on Zulip):

anyway

Matthew Jasper (Sep 24 2019 at 20:37, on Zulip):

Doesn't qualifs = self.qualifs_in_any_value_of_ty(body.return_ty()); ensure that integer constants are always promotable.

ecstatic-morse (Sep 24 2019 at 20:38, on Zulip):

Do you know if there's anything that could someday appear in a const body that would make us not want to promote &CONST (stuff that's not covered by HasMutInterior and NeedsDrop? Maybe !Sync stuff?

ecstatic-morse (Sep 24 2019 at 20:38, on Zulip):

Ah yes, that's correct

ecstatic-morse (Sep 24 2019 at 20:40, on Zulip):

btw, that line will always clear IsNotPromotable for every type, but sometimes not IsNotImplicitlyPromotable due to the Option<bool> returned by Qualif::in_any_value_of_ty

oli (Sep 24 2019 at 20:42, on Zulip):

@RalfJ is probably very interested in the !Sync question

oli (Sep 24 2019 at 20:43, on Zulip):

but I think the Sync thing is not something we can actually fix, I think it was cratered and bascially unfixable

oli (Sep 24 2019 at 20:43, on Zulip):

we could start linting it though

centril (Sep 24 2019 at 21:54, on Zulip):

@oli my understanding is that the Sync stuff is a soundness hole?... so then we must fix it

centril (Sep 24 2019 at 21:55, on Zulip):

I'm wrong

What exactly were you wrong about?

oli (Sep 24 2019 at 21:55, on Zulip):

const FOO: Option<Cell<u32>> = None; gets promoted

oli (Sep 24 2019 at 21:55, on Zulip):

so &FOO is &'static Option<Cell<u32>>

centril (Sep 24 2019 at 21:56, on Zulip):

right; so we don't just look at the type

oli (Sep 24 2019 at 21:56, on Zulip):

well, we could start with a future incompat lint based on https://github.com/rust-lang/rust/pull/54424

oli (Sep 24 2019 at 21:57, on Zulip):

right; so we don't just look at the type

that's what I meant with "I'm wrong" :D

oli (Sep 24 2019 at 21:57, on Zulip):

and we're using zulip the wrong way

oli (Sep 24 2019 at 21:57, on Zulip):

having two conversations in one topic

RalfJ (Oct 09 2019 at 12:59, on Zulip):

so even const FOO: Option<Cell<u32>> = None; should not get promoted

this is promotion looking into the initializer of a const, right? dang I almost forgot about that.
we should re-fomulate this check as just looking at the final value, not the code computing it.

RalfJ (Oct 09 2019 at 12:59, on Zulip):

oh lol @oli already said that :)

RalfJ (Oct 09 2019 at 13:00, on Zulip):

but I think the Sync thing is not something we can actually fix, I think it was cratered and bascially unfixable

yeah I want to come back to this eventually and see what we can do, but it's a sad mess

RalfJ (Oct 09 2019 at 13:01, on Zulip):

oli my understanding is that the Sync stuff is a soundness hole?... so then we must fix it

@centril well I think it is but our spec isn't detailed enough to give a definite answer ;)

RalfJ (Oct 09 2019 at 13:01, on Zulip):

FTR, the issue for that is https://github.com/rust-lang/rust/issues/49206

centril (Oct 09 2019 at 13:02, on Zulip):

yeah that's fair

centril (Oct 09 2019 at 13:02, on Zulip):

@RalfJ back from vacation btw?

RalfJ (Oct 09 2019 at 13:02, on Zulip):

I am, yes (the last 2 weeks was travelling for work, not vacation)

RalfJ (Oct 09 2019 at 13:02, on Zulip):

just 606 emails to go in my Rust folder :P

RalfJ (Oct 09 2019 at 13:02, on Zulip):

oh, and two POPL papers to revise, and a talk to prepare, and a thesis to write...

centril (Oct 09 2019 at 13:03, on Zulip):

Jesus mother of...

RalfJ (Oct 09 2019 at 14:30, on Zulip):

so coming back to value-based promotion, my hope was that https://github.com/rust-rfcs/const-eval/pull/27 could help move in that direction by getting a better understanding of what it even is that we need

RalfJ (Oct 09 2019 at 14:30, on Zulip):

and these days we have the value visitor which could be used to give more precise answers to questions like "will dropping this value do anything"

RalfJ (Oct 09 2019 at 14:31, on Zulip):

Option<Cell<u32>> is particularly interesting though because I just recently changed Stacked Borrows to no longer be "value-based" here and instead treat this like a union... because otherwise things just become crazy complicated

RalfJ (Oct 09 2019 at 14:31, on Zulip):

see https://github.com/rust-lang/unsafe-code-guidelines/issues/204

RalfJ (Oct 09 2019 at 14:32, on Zulip):

@ecstatic-morse ^

Last update: Nov 15 2019 at 20:35UTC