Stream: t-compiler/const-eval

Topic: #55454


oli (Oct 29 2018 at 11:52, on Zulip):

I looked through the commit range and I have to aggree it must be my PRs fault

oli (Oct 29 2018 at 11:53, on Zulip):

There's two separate issues. One pre-existing: we can get 1..=0 as a valid "all elements" range, not just 0..=max

oli (Oct 29 2018 at 11:53, on Zulip):

the other (introduced by my PR): promoteds are evaluated with Reveal::UserFacing. I don't think that ever happened before

oli (Oct 29 2018 at 11:55, on Zulip):

We can just eval promoteds directly. I'll check if that assessment is true, if it is, I'm going to fix it, but I'd also like to fix it so that other full ranges are accepted as full ranges (my original validation code did that I believe)

oli (Oct 29 2018 at 12:01, on Zulip):

The problem is that I won't have a repro for the latter

RalfJ (Oct 29 2018 at 12:16, on Zulip):

There's two separate issues. One pre-existing: we can get 1..=0 as a valid "all elements" range, not just 0..=max

ah and then we bail because of pointers?

RalfJ (Oct 29 2018 at 12:17, on Zulip):

yeah that one is my fault. I didn't think we'd do such wrap-around if it wasn't needed. oh well.

oli (Oct 29 2018 at 12:20, on Zulip):

I reviewed your change and could neither produce such ranges nor point to layout code that would hypothetically produce them

oli (Oct 29 2018 at 12:21, on Zulip):

so we can safely implement Sync for this blame ;)

RalfJ (Oct 29 2018 at 12:22, on Zulip):

^^

oli (Oct 29 2018 at 13:51, on Zulip):

oh god. I know why this issue was introduced by my PR

oli (Oct 29 2018 at 13:51, on Zulip):

before, validation was a lint

oli (Oct 29 2018 at 13:51, on Zulip):

and lints don't see promoteds

oli (Oct 29 2018 at 13:52, on Zulip):

promoteds were never validated

RalfJ (Oct 29 2018 at 13:52, on Zulip):

oh

RalfJ (Oct 29 2018 at 13:52, on Zulip):

uh

RalfJ (Oct 29 2018 at 13:52, on Zulip):

do we want them validated?

oli (Oct 29 2018 at 13:52, on Zulip):

yes

RalfJ (Oct 29 2018 at 13:53, on Zulip):

well if you say so^^

oli (Oct 29 2018 at 13:53, on Zulip):

oh you mean...

oli (Oct 29 2018 at 13:53, on Zulip):

hm

oli (Oct 29 2018 at 13:53, on Zulip):

oh god

oli (Oct 29 2018 at 13:53, on Zulip):

maybe not!?

RalfJ (Oct 29 2018 at 13:53, on Zulip):

I'm a bit worried about breaking compilation

RalfJ (Oct 29 2018 at 13:54, on Zulip):

I mean it is a good sanity check for us

RalfJ (Oct 29 2018 at 13:54, on Zulip):

making sure we got validation right

oli (Oct 29 2018 at 13:54, on Zulip):

lol, we just emit lints for promoteds failing validation (I hope)

RalfJ (Oct 29 2018 at 13:54, on Zulip):

and it SHOULD never fail because we shouldnt promote stuff that could fail

oli (Oct 29 2018 at 13:54, on Zulip):

indeed

RalfJ (Oct 29 2018 at 13:55, on Zulip):

has this been cratered?^^

oli (Oct 29 2018 at 13:55, on Zulip):

obviously not :P

RalfJ (Oct 29 2018 at 13:55, on Zulip):

well there'll be a beta with this change... today or so

RalfJ (Oct 29 2018 at 13:55, on Zulip):

I guess someone will crater that

RalfJ (Oct 29 2018 at 13:55, on Zulip):

:P

oli (Oct 29 2018 at 13:56, on Zulip):

it's a trivial backportable fix if we want to back out for promoteds

RalfJ (Oct 29 2018 at 13:56, on Zulip):

might be better to do that, and then revert the backing-out and then crater that change?

oli (Oct 29 2018 at 13:59, on Zulip):

let's wait for the beta crater, no need to run essentially the same crater multiple times

RalfJ (Oct 29 2018 at 14:00, on Zulip):

kk

RalfJ (Oct 30 2018 at 16:01, on Zulip):

actually probably the PR is NOT in beta... seems beta was branched off yesterday but with an old commit because something something RLS

Last update: Nov 15 2019 at 21:20UTC