Stream: wg-traits

Topic: reverting universes


nikomatsakis (Feb 15 2019 at 21:41, on Zulip):

So if we wanted to "revert" universes, how would we go about it? It's a bit tricky.

I could of course just try to straight-up git revert and deal with the conflicts. Probably most of it comes from @lqd's changes to improve diagnostics.

But what if we tried to reproduce the same semantics we had before, but in the new system. One challenge is that this semantics was a bit inconsistent and buggy (e.g., we had to move NLL to universes to deal with that).

Certainly one thing we could do is to just layer the "leak check" back on for now. Basicallly, wherever we instantiate placeholders now, we would keep doing so -- but we would remember the universe and we would do a "leak check" that checks to see if any placeholders in this universe are related to some variable that is an outer universe. (We could even do this bidirectionally, to be as buggy as before.)

That's...actually probably fairly easy to do.

lqd (Feb 15 2019 at 21:44, on Zulip):

interesting, is that somewhat of a backup plan or a likely direction ?

nikomatsakis (Feb 15 2019 at 21:46, on Zulip):

@lqd I see it just as a "buy some time" plan

nikomatsakis (Feb 15 2019 at 21:46, on Zulip):

I don't think it's something we'd really want to do

nikomatsakis (Feb 15 2019 at 21:47, on Zulip):

although it could be used in specific places as a kind of efficiency change

nikomatsakis (Feb 15 2019 at 21:47, on Zulip):

it's coming on 5pm here so i'm probably not going to pursue this much tonight

nikomatsakis (Feb 15 2019 at 21:48, on Zulip):

@lqd to elaborate a bit more, it's not that I think the new system is wrong, but there may be some subtleties, and there is some value in trying to preserve the existing behavior while we talk things out more fully

nikomatsakis (Feb 15 2019 at 21:48, on Zulip):

(specifically, I think it may require changes in other parts of the lang that have been kind of "papered over")

lqd (Feb 15 2019 at 21:48, on Zulip):

it's a least nice to have this possibility in case we need it !

nikomatsakis (Feb 15 2019 at 21:51, on Zulip):

Yeah, I'm looking a bit into what it would take

nikomatsakis (Feb 19 2019 at 18:25, on Zulip):

OK I started looking at this a bit

nikomatsakis (Feb 19 2019 at 18:25, on Zulip):

I didn't get too far but I think it would work

nikomatsakis (Feb 19 2019 at 18:26, on Zulip):

I'm debating whether this is something that e.g. might be a good fit for @Aaron Turon, or whether to try and tackle it myself. Could be something to hand off a bit, I'm concerned about time.

Aaron Turon (Feb 19 2019 at 18:27, on Zulip):

i'm certainly game to try!

Aaron Turon (Feb 19 2019 at 18:27, on Zulip):

how confident are we that the leak check is the only difference?

Aaron Turon (Feb 19 2019 at 18:28, on Zulip):

lemme put that differently

Aaron Turon (Feb 19 2019 at 18:28, on Zulip):

one worry, given that we're close to release, is that we try to "patch" the universes code to not change the behavior, but we don't get it totally right

Aaron Turon (Feb 19 2019 at 18:28, on Zulip):

maybe a crater run of the patch could give us some confidence?

nikomatsakis (Feb 19 2019 at 18:34, on Zulip):

that would help, although the existing PR didn't show any problems in crater

nikomatsakis (Feb 19 2019 at 18:34, on Zulip):

that said, I'm pretty confident that the leak check is the only difference

nikomatsakis (Feb 19 2019 at 18:34, on Zulip):

I mean that is literally the thing that was replaced

nikomatsakis (Feb 19 2019 at 18:34, on Zulip):

but really the question is, if we try to "restore" the old behavior, are we going to wind up "preserving" it

nikomatsakis (Feb 19 2019 at 18:35, on Zulip):

it's hard to say

nikomatsakis (Feb 19 2019 at 18:35, on Zulip):

hmm. so next release is feb 28

nikomatsakis (Feb 19 2019 at 18:35, on Zulip):

certainly the sooner we can land, the better

nikomatsakis (Feb 19 2019 at 18:35, on Zulip):

this suggests it's worth frontloading some effort to this afternoon

nikomatsakis (Feb 19 2019 at 18:36, on Zulip):

and/or now-ish

Aaron Turon (Feb 19 2019 at 18:38, on Zulip):

@nikomatsakis OK, that is a pretty tight deadline. i'm happy to start poking at it but would certainly need help. if you think it's easiest for you to just knock it out, totally fine

nikomatsakis (Feb 19 2019 at 18:38, on Zulip):

well, so, a thing we could do is to do some "pair prog" time on it

nikomatsakis (Feb 19 2019 at 18:39, on Zulip):

with the goal of being able to "hand off"

nikomatsakis (Feb 19 2019 at 18:39, on Zulip):

i.e., I have some time today, but I feel like time may be tight over next few days

Aaron Turon (Feb 19 2019 at 18:39, on Zulip):

sure thing

nikomatsakis (Feb 19 2019 at 18:39, on Zulip):

I plan to start poking at this at 3pm my time

Aaron Turon (Feb 19 2019 at 18:41, on Zulip):

@nikomatsakis sgtm, i'll be ready to pair

Aaron Turon (Feb 19 2019 at 18:42, on Zulip):

we could do zoom + record, for other WG-traits folks

nikomatsakis (Feb 19 2019 at 18:50, on Zulip):

sure

nikomatsakis (Feb 19 2019 at 18:50, on Zulip):

plus, others find it helpful to be able to review

nikomatsakis (Feb 19 2019 at 18:50, on Zulip):

particularly if I'm driving and making emacs jump all over the place :P

Aaron Turon (Feb 19 2019 at 18:51, on Zulip):

hah :+1:

nikomatsakis (Feb 19 2019 at 20:10, on Zulip):

@Aaron Turon ok I'm planning to start on this soon

Aaron Turon (Feb 19 2019 at 20:10, on Zulip):

@nikomatsakis ready when you are!

nikomatsakis (Feb 19 2019 at 20:13, on Zulip):

ok, few more minutes

nikomatsakis (Feb 19 2019 at 20:14, on Zulip):

@Aaron Turon (and anyone else who wants), connect to https://zoom.us/j/181515793

tmandry (Feb 19 2019 at 21:04, on Zulip):

Was in the air for this meeting, but happy to review when it comes out on video

nikomatsakis (Feb 19 2019 at 21:29, on Zulip):

so we didn't wind up doing any coding

nikomatsakis (Feb 19 2019 at 21:29, on Zulip):

but instead spent a while talking about the implications of subtyping and soundness

nikomatsakis (Feb 19 2019 at 21:30, on Zulip):

I convinced myself that the current coherence rules are definitely incorrect, in that they permit overlapping types

nikomatsakis (Feb 19 2019 at 21:30, on Zulip):

we also creeped each other out with some of the implications but failed so far to uncover a weaponized problem

nikomatsakis (Feb 19 2019 at 21:31, on Zulip):

and I feel mildly convinced that -- if such a problem exists -- it's because of other rules that need to be adjusted =)

nikomatsakis (Feb 19 2019 at 22:12, on Zulip):

so the question now is

nikomatsakis (Feb 19 2019 at 22:13, on Zulip):

do I try to push through the "bring back the leak check for now" change?

nikomatsakis (Feb 19 2019 at 22:14, on Zulip):

and, if not, maybe what other things...do we have to do

nikomatsakis (Feb 19 2019 at 22:14, on Zulip):

thinking on it a bit, I realize that e.g. #57639 kind of amounts to doing some limited form of the leak check

Aaron Turon (Feb 19 2019 at 22:15, on Zulip):

OK -- i personally don't have quite enough of the pieces to be able to connect the dots

Aaron Turon (Feb 19 2019 at 22:16, on Zulip):

it does seem like getting the leak check re-instated is the obviously conservative thing to do

Aaron Turon (Feb 19 2019 at 22:16, on Zulip):

and it's starting to feel like really figuring out what we want here may take some time

Aaron Turon (Feb 19 2019 at 22:16, on Zulip):

whereas, IIUC, reinstating the leak check shouldn't be too hard

Aaron Turon (Feb 19 2019 at 22:16, on Zulip):

@nikomatsakis ^

nikomatsakis (Feb 19 2019 at 22:18, on Zulip):

Yeah, that is roughly my feeling. I think it remains true that the universe check is just "more correct", but that doesn't mean that it's right to institute it now

nikomatsakis (Feb 19 2019 at 22:18, on Zulip):

i.e., we might want to put through other mitigating changes first

Aaron Turon (Feb 19 2019 at 22:18, on Zulip):

yeah

nikomatsakis (Feb 19 2019 at 22:19, on Zulip):

in retrospect, I shouldn't have pushed through the full check, I always meant to reimplement the leak check in terms of universes anyway, but the clean crater run tempted me, and I confess I didn't anticipate all of these other interactions

Aaron Turon (Feb 19 2019 at 22:20, on Zulip):

i mean the universe-based check seems a lot more elegant, i can understand the desire

nikomatsakis (Feb 19 2019 at 22:20, on Zulip):

mostly I meant to take an "in between" step

Aaron Turon (Feb 19 2019 at 22:20, on Zulip):

yeah

simulacrum (Feb 19 2019 at 22:22, on Zulip):

FWIW you technically have until ~Sunday night to land whatever it is; I will note that the queue has been quite slow so we'll want to have it up at least 9-12 hours before we expect it to land

simulacrum (Feb 19 2019 at 22:22, on Zulip):

(and if necessary there is technically some wiggle room on the release side -- we could slip by a day without affecting Thursday release)

nikomatsakis (Feb 19 2019 at 22:23, on Zulip):

Yeah, I'm just a bit nervous about things here

nikomatsakis (Feb 19 2019 at 22:23, on Zulip):

how is the crater queue etc?

nikomatsakis (Feb 19 2019 at 22:23, on Zulip):

for an e.g. check-only crater run?

nikomatsakis (Feb 19 2019 at 22:23, on Zulip):

is that still reasonably quick

nikomatsakis (Feb 19 2019 at 22:23, on Zulip):

Yeah, I'm just a bit nervous about things here

that is, I don't want to introduce ICEs etc

simulacrum (Feb 19 2019 at 22:23, on Zulip):

Crater queue is currently empty -- I believe check only is approx. 36 hours (https://crater.rust-lang.org/)

simulacrum (Feb 19 2019 at 22:24, on Zulip):

But @Pietro Albini can give more accurate timeline on how long crater takes probably

Pietro Albini (Feb 19 2019 at 22:24, on Zulip):

check-only is ~1.5 days

nikomatsakis (Feb 19 2019 at 22:24, on Zulip):

OK

nikomatsakis (Feb 19 2019 at 22:24, on Zulip):

I'll see if I can pull together a PR tonight/tomorrow morning

nikomatsakis (Feb 19 2019 at 22:24, on Zulip):

seems plausible

nikomatsakis (Feb 19 2019 at 22:25, on Zulip):

gotta go fix dinner now :)

Aaron Turon (Feb 19 2019 at 22:25, on Zulip):

ok, thanks @nikomatsakis

Aaron Turon (Feb 19 2019 at 22:45, on Zulip):

I'm putting together some notes here for the move to universes in general

nikomatsakis (Feb 19 2019 at 23:08, on Zulip):

cool

nikomatsakis (Feb 20 2019 at 11:44, on Zulip):

PR implementing the revert strategy I floated here: https://github.com/rust-lang/rust/pull/58592

nikomatsakis (Feb 20 2019 at 11:45, on Zulip):

Still doing some test runs locally

nikomatsakis (Feb 20 2019 at 15:51, on Zulip):

Did my best to summarize the universe situation here: https://github.com/rust-lang/rust/issues/56105#issuecomment-465634661

lqd (Feb 20 2019 at 16:15, on Zulip):

(in case you haven't noticed, the leak-check revival PR #58592 has been blocked from running the CI tests by the merciless tidy)

Aaron Turon (Feb 20 2019 at 17:07, on Zulip):

OK, i'm starting to review this morning

nikomatsakis (Feb 20 2019 at 17:20, on Zulip):

@lqd thanks -- I'm currently getting the tests to pass

nikomatsakis (Feb 20 2019 at 17:20, on Zulip):

and will then address the merciless tidy

Aaron Turon (Feb 20 2019 at 17:45, on Zulip):

done reviewing

nikomatsakis (Feb 20 2019 at 17:56, on Zulip):

@Aaron Turon I'll take a look -- so I also went over all the tests. Most of the new output made sense to me but for one example that surprised me.

nikomatsakis (Feb 20 2019 at 17:57, on Zulip):

This example used to error out but now does not:

fn foo(
    x: &for<'a, 'b> Foo<&'a u8, &'b u8>,
    y: &for<'a> Foo<&'a u8, &'a u8>,
) {
    let z = match 22 {
        0 => x,
        _ => y,
    };
}

However, our "LUB" code (IIRC) is meant to enforce a strict equality check -- basically I don't know why this leak check would ever make something pass that did not before

nikomatsakis (Feb 20 2019 at 17:57, on Zulip):

Seems surprising

nikomatsakis (Feb 20 2019 at 17:59, on Zulip):

Although, looking in the git notes, I see :

 This is needed for the universe-based code to handle
    `old-lub-glb-object`; that test used to work sort of by accident
    before with the old code.

from 904a0bde93f0348f69914ee90b1f8b6e4e0d7cbc

nikomatsakis (Feb 20 2019 at 17:59, on Zulip):

so something is going on there

nikomatsakis (Feb 20 2019 at 18:00, on Zulip):

oh, I sort of maybe see how this change could make an error go away -- maybe we're getting an error more eagerly than we did before, we might apply the coercions rules better

simulacrum (Feb 20 2019 at 20:14, on Zulip):

@nikomatsakis Do you feel comfortable backporting https://github.com/rust-lang/rust/pull/58056 and https://github.com/rust-lang/rust/pull/58592? I've beta-nominated them and also nominated for the compiler team to approve tomorrow morning -- we might want to make sure they merge cleanly onto beta though

nikomatsakis (Feb 20 2019 at 20:40, on Zulip):

@simulacrum I do -- they are in "seldom touched" parts of the code that haven't really changed since beta

nikomatsakis (Feb 20 2019 at 20:41, on Zulip):

that said, I did want to maybe do a crater run on #58592 if we can

nikomatsakis (Feb 20 2019 at 20:41, on Zulip):

maybe we won't have time

nikomatsakis (Feb 20 2019 at 20:41, on Zulip):

we can hopefully at least do a perf run

simulacrum (Feb 20 2019 at 22:02, on Zulip):

I think we have time

lqd (Feb 20 2019 at 22:06, on Zulip):

the try build should be ready soon

lqd (Feb 20 2019 at 22:13, on Zulip):

right on queue cue

Aaron Turon (Feb 20 2019 at 22:13, on Zulip):

@nikomatsakis @simulacrum ^

simulacrum (Feb 20 2019 at 22:16, on Zulip):

Started crater, perf runs

lqd (Feb 20 2019 at 22:16, on Zulip):

@simulacrum thanks :)

nikomatsakis (Feb 21 2019 at 16:28, on Zulip):

btw, @Aaron Turon, I was thinking that maybe -- as a "next step" once we deal with the immediate leak-check thing -- we might try adding some constraints such that 'empty is not permitted to result

Aaron Turon (Feb 21 2019 at 17:47, on Zulip):

@nikomatsakis hm, ok. i'm feeling generally uneasy, in the sense that it feels hard to judge the implications of various choices. earlier i had convinced myself that 'empty was totally fine to have around...

nikomatsakis (Feb 21 2019 at 17:48, on Zulip):

I'd like to talk out the dropck implications too

nikomatsakis (Feb 21 2019 at 17:48, on Zulip):

This is an "underspecified" area

nikomatsakis (Feb 21 2019 at 17:48, on Zulip):

Anyway not a thing we need to immediately discuss

nikomatsakis (Feb 21 2019 at 17:49, on Zulip):

I'm planning to r+ the PR as soon as I finish getting the stderr files blessed etc

Aaron Turon (Feb 21 2019 at 17:49, on Zulip):

OK, sounds good

nikomatsakis (Feb 21 2019 at 17:50, on Zulip):

Then I will attempt the backport

nikomatsakis (Feb 21 2019 at 17:50, on Zulip):

(which I expect to be fairly easy...)

centril (Feb 21 2019 at 17:56, on Zulip):

@nikomatsakis should we treeclose as well to make sure it goes through?

nikomatsakis (Feb 21 2019 at 18:23, on Zulip):

@centril hmm good question! I don't really know. Maybe see if it lands naturally first? Does seem pretty imporatnt

nikomatsakis (Feb 21 2019 at 18:23, on Zulip):

If we're trying to avert delaying the release, it feels like extreme machinations are justified

nikomatsakis (Feb 21 2019 at 18:24, on Zulip):

I'd be happy if I saw a green travis mark first

nikomatsakis (Feb 21 2019 at 18:24, on Zulip):

though locally it seems to be doing well

centril (Feb 21 2019 at 18:24, on Zulip):

definitely justified if we need to :slight_smile:

nikomatsakis (Feb 21 2019 at 18:24, on Zulip):

I'm kind of happy to defer to release team on this point ;)

centril (Feb 21 2019 at 18:25, on Zulip):

@nikomatsakis cool; I'll see what happens with travis and decide ^^

simulacrum (Feb 21 2019 at 18:57, on Zulip):

@nikomatsakis Delaying the release, beyond public perception, is essentially free (we'll have a few wrong dates in e.g. RELEASES.md but that doesn't really matter)

simulacrum (Feb 21 2019 at 18:57, on Zulip):

I don't think there's any reason to treeclose -- we just need to make sure to retry as needed

nikomatsakis (Feb 21 2019 at 18:57, on Zulip):

@simulacrum ok, yes. It's mostly public perception that I'm worried about. I think if it's on the scale of a few days etc it's fine. If we slip a release, or bump a week or something, that's a bigger deal

simulacrum (Feb 21 2019 at 18:58, on Zulip):

So basically in terms of stopping the release you have until Thursday morning -- we can build artifacts etc and be ready to go

simulacrum (Feb 21 2019 at 18:58, on Zulip):

We should have crater and some nightly baking by that point

simulacrum (Feb 21 2019 at 18:59, on Zulip):

I think we'll be fine more or less

simulacrum (Feb 21 2019 at 19:00, on Zulip):

Realistically our 6-week model means that if we delay a release by a week I'm honestly not _too_ concerned

simulacrum (Feb 21 2019 at 19:00, on Zulip):

As I think @Aaron Turon mentioned that seems a lot better then shipping potential known bugs

centril (Feb 21 2019 at 19:10, on Zulip):

@simulacrum the purpose of the tree-close is to avoid Niko or Aaron having to rebase and such... sure, we can retry all other PRs to make this one go first, but, treeclose seems cleaner and easier

simulacrum (Feb 21 2019 at 19:17, on Zulip):

Sure, I meant that treeclose doesn't -- IIRC -- help with the retry bit and I presumed conflicts would be minimal anyway (i.e., we would emulate treeclose manually)

lqd (Feb 21 2019 at 20:55, on Zulip):

@centril GG on being super quick with the retry :)

centril (Feb 21 2019 at 20:55, on Zulip):

=P

lqd (Feb 21 2019 at 20:57, on Zulip):

I hope CI issues don't keep happening like they did with the previous PR

lqd (Feb 21 2019 at 21:29, on Zulip):

@centril happened again :/

centril (Feb 21 2019 at 21:29, on Zulip):

@nikomatsakis CI is being real difficult :confused:

nikomatsakis (Feb 21 2019 at 21:30, on Zulip):

@centril argh

centril (Feb 21 2019 at 21:31, on Zulip):

whats the command for treeclose?

centril (Feb 21 2019 at 21:31, on Zulip):

@bors treeclose=99?

nikomatsakis (Feb 21 2019 at 21:36, on Zulip):

@simulacrum probably knows

centril (Feb 21 2019 at 21:39, on Zulip):

(closed tree and did retry, pending again)

nikomatsakis (Feb 21 2019 at 22:07, on Zulip):

@centril errors still look spurious, I Assume?

centril (Feb 21 2019 at 22:08, on Zulip):

@nikomatsakis ye, but its testing now, 32 min in

lqd (Feb 22 2019 at 07:41, on Zulip):

#58592 merged :tada: and the crater run is done: the 2 regressions it mentions don't seem related to the PR.

centril (Feb 22 2019 at 09:22, on Zulip):

@lqd yep, both spurious :tada:

nikomatsakis (Feb 22 2019 at 11:09, on Zulip):

I'm working on the beta backport now

centril (Feb 22 2019 at 11:17, on Zulip):

@nikomatsakis do we want to crater the backport or just r+ right away?

nikomatsakis (Feb 22 2019 at 11:18, on Zulip):

Don't know

nikomatsakis (Feb 22 2019 at 11:18, on Zulip):

One thing I wanted to check on was whether the new nightly (presuming it's issued) accrues any bug reports

nikomatsakis (Feb 22 2019 at 11:18, on Zulip):

currently my beta backport is stalled at building LLVM :P

nikomatsakis (Feb 22 2019 at 11:19, on Zulip):

but once that completes I expect to be able to put up a PR fairly easily

centril (Feb 22 2019 at 11:20, on Zulip):

@nikomatsakis neat; what date were we doing beta=>stable promotion this time? crater took 2 days on nightly, so ideally we should have crater done on monday and be able to merge on the same day if you want to wait for crater

nikomatsakis (Feb 22 2019 at 11:22, on Zulip):

Regarding date, I'm not entirely sure, but I think that @simulacrum said something about Sunday night being a deadline for things to land

nikomatsakis (Feb 22 2019 at 11:22, on Zulip):

There is also some flexibility here I think

centril (Feb 22 2019 at 11:24, on Zulip):

@nikomatsakis ok hmm... if you want to get it in by sunday we can still start crater and merge before; at least we'll get a report after

centril (Feb 22 2019 at 11:25, on Zulip):

we might get crater done before monday as well...

centril (Feb 22 2019 at 11:26, on Zulip):

btw... the PR is still just beta-nominated... should we assume beta-accepted now as well?

nikomatsakis (Feb 22 2019 at 11:41, on Zulip):

yeah, based on discussion in the compiler meeting I think so

nikomatsakis (Feb 22 2019 at 11:41, on Zulip):

still working on backport, running some tests

nikomatsakis (Feb 22 2019 at 11:41, on Zulip):

maybe 75% done

nikomatsakis (Feb 22 2019 at 12:43, on Zulip):

Backport PR: https://github.com/rust-lang/rust/pull/58641

Last update: Nov 18 2019 at 00:45UTC