Stream: t-compiler/wg-nll

Topic: nll-in-2015?


nikomatsakis (Sep 05 2018 at 13:16, on Zulip):

Hey all -- so after the meeting yesterday the question was raised about whether -- when we "stabilize" NLL -- we should make it the default for 2015 and 2018 or just 2015. I personally lean towards "everywhere" — I want to minimize the time period in which we have two borrow checkers to the extent we can.

nikomatsakis (Sep 05 2018 at 13:17, on Zulip):

Among other benefits, this would mean that we get "incremental perf results" "for free"

nikomatsakis (Sep 05 2018 at 13:17, on Zulip):

cc @pnkfelix — thoughts?

pnkfelix (Sep 05 2018 at 13:23, on Zulip):

Seems riskier than what I had planned

pnkfelix (Sep 05 2018 at 13:24, on Zulip):

You meant “... or just 2018” above, right?

nikomatsakis (Sep 05 2018 at 13:24, on Zulip):

yes :)

nikomatsakis (Sep 05 2018 at 13:24, on Zulip):

there is some risk

nikomatsakis (Sep 05 2018 at 13:24, on Zulip):

though..minimal?

nikomatsakis (Sep 05 2018 at 13:25, on Zulip):

well, I guess we may accept things we don't want to accept

lqd (Sep 05 2018 at 13:46, on Zulip):

@nikomatsakis so we'd want to start with migrate mode on both editions "soon", and then after a while switch both to MIR mode ?

nikomatsakis (Sep 05 2018 at 13:47, on Zulip):

I guess there are two possibilities

nikomatsakis (Sep 05 2018 at 13:48, on Zulip):
nikomatsakis (Sep 05 2018 at 13:48, on Zulip):

or

nikomatsakis (Sep 05 2018 at 13:48, on Zulip):
nikomatsakis (Sep 05 2018 at 13:48, on Zulip):

depends how much confidence we have in the new code I suppose =)

lqd (Sep 05 2018 at 13:49, on Zulip):

:)

lqd (Sep 05 2018 at 13:49, on Zulip):

I was mostly wondering about the 2PB expansion possibilities and similar

nikomatsakis (Sep 05 2018 at 13:49, on Zulip):

I guess a side benefit is that it makes the Edition that much more appealing :)

nikomatsakis (Sep 05 2018 at 13:49, on Zulip):

versus hanging out in 2015

nikomatsakis (Sep 05 2018 at 13:49, on Zulip):

there is one other factor

nikomatsakis (Sep 05 2018 at 13:50, on Zulip):

one that sort of pushes me towards the 2018 plan

nikomatsakis (Sep 05 2018 at 13:50, on Zulip):

I believe, and @simulacrum can confirm, that the plan is to enable the 2018 features for this beta -- but then disable them before going to master

nikomatsakis (Sep 05 2018 at 13:50, on Zulip):

so that we can run a second RC

nikomatsakis (Sep 05 2018 at 13:50, on Zulip):

in which case, that means that the beta would be using (for 2015 code) a totally different borrowck than the one that the corresponding stable would use

nikomatsakis (Sep 05 2018 at 13:50, on Zulip):

that feels .. sort of wrong

nikomatsakis (Sep 05 2018 at 13:51, on Zulip):

(2018 code will go on to use the next beta, presumably)

lqd (Sep 05 2018 at 13:51, on Zulip):

hmm yeah

lqd (Sep 05 2018 at 13:53, on Zulip):

for the stabilizing PR we mentioned yesterday I was expecting enabling NLL in migrate only on the 2018 edition, and leave 2015 as-is, but maybe that was wrong

pnkfelix (Sep 05 2018 at 14:30, on Zulip):

@nikomatsakis I don’t understand why you say under “the plan“, the beta would stop using AST-borrowck for edition:2015

pnkfelix (Sep 05 2018 at 14:32, on Zulip):

Is “the plan”, assuming we only do 2018=mIgrate In short term, something different from leaving the flags/switches as they are currently set?

pnkfelix (Sep 05 2018 at 14:33, on Zulip):

Maybe I need to read some other docs (or code!) of “the plan”

nikomatsakis (Sep 05 2018 at 14:35, on Zulip):

the Edition RC plan, iirc, is to "temporarily" allow the use of 2018 features on beta and then revert that when we go into stable

nikomatsakis (Sep 05 2018 at 14:35, on Zulip):

e.g. by patching the beta branch

nikomatsakis (Sep 05 2018 at 14:39, on Zulip):

but I might be remembering wrong or we may have changed the plan

nikomatsakis (Sep 05 2018 at 14:39, on Zulip):

i see https://internals.rust-lang.org/t/rust-2018-the-home-stretch/7810

Santiago Pastorino (Sep 05 2018 at 14:48, on Zulip):

I thought 2015 was always going to be AST

nikomatsakis (Sep 05 2018 at 14:51, on Zulip):

what do you mean by always? certainly not in perpetuity

Santiago Pastorino (Sep 05 2018 at 14:54, on Zulip):

I meant that, I thought the edition was not going to receive any of this kind of changes that in some way could affect the behavior

Santiago Pastorino (Sep 05 2018 at 14:55, on Zulip):

it could break some code as far as I have understood

Santiago Pastorino (Sep 05 2018 at 14:55, on Zulip):

but I guess if crater pass all tests in 2015 with MIR mode enabled it will be fine :)

pnkfelix (Sep 05 2018 at 14:59, on Zulip):

Okay, so I'm away aware of this

nikomatsakis: the Edition RC plan, iirc, is to "temporarily" allow the use of 2018 features on beta and then revert that when we go into stable

however, I guess I viewed NLL as in a slightly different space, because for edition:2018, you (currently) do not need a #![feature(nll)] flag for it

pnkfelix (Sep 05 2018 at 15:00, on Zulip):

and therefore I figured we would treat NLL as something that just "leaks" out into master

pnkfelix (Sep 05 2018 at 15:00, on Zulip):

I guess its a question of whether NLL is "just another feature"

pnkfelix (Sep 05 2018 at 15:01, on Zulip):

I agree that I do not want the stable and beta branches to be using totally different borrowck's for edition:2018. That would be a disaster. I'd rather turn NLL on universally than do that.

pnkfelix (Sep 05 2018 at 15:02, on Zulip):

but it seems super weird adopt a riskier plan (w.r.t. 2015 compatibility) on the basis of "well this plan says we turn everything off before going to stable"

pnkfelix (Sep 05 2018 at 15:07, on Zulip):

I guess my real point is this: If we're going to have a lot more bureaucratic headaches (in terms of people trying to approve things for the 2018 edition) trying to do solely edition:2018=migrate, then fine, go ahead and turn migrate mode on universally.

pnkfelix (Sep 05 2018 at 15:09, on Zulip):

but I personally still think that the best bet is to plan to leave things just as they are (edition:2018=migrate, edition:2015=ast). And the only question for the various teams should be whether NLL gets turned on at all; if enough people veto it (for whatever reason), then it gets turned off across the board.

pnkfelix (Sep 05 2018 at 15:10, on Zulip):

That previous comment was from the view point of "what do I see as least risk for Rust itself as a product, in both the 2015 and 2018 editions"

pnkfelix (Sep 05 2018 at 15:10, on Zulip):

If I put on my rustc-maintainer hat on, then sure I'm all for turning it on universally and throwing away AST-borrowck.

nikomatsakis (Sep 05 2018 at 15:31, on Zulip):

it could break some code as far as I have understood

those are all bug fixes

nikomatsakis (Sep 05 2018 at 15:31, on Zulip):

and therefore I figured we would treat NLL as something that just "leaks" out into master

well, either way, the point is that if we applied to 2015

nikomatsakis (Sep 05 2018 at 15:31, on Zulip):

there is risk either way:

nikomatsakis (Sep 05 2018 at 15:31, on Zulip):
nikomatsakis (Sep 05 2018 at 15:32, on Zulip):

but I personally still think that the best bet is to plan to leave things just as they are (edition:2018=migrate, edition:2015=ast). And the only question for the various teams should be whether NLL gets turned on at all; if enough people veto it (for whatever reason), then it gets turned off across the board.

yeah I'm on board this is what we should do — I'm not sure for how long, but definitely for now

pnkfelix (Sep 05 2018 at 15:33, on Zulip):

I'm confused again, what did you mean by "... if we applied to 2015 ..." ?

pnkfelix (Sep 05 2018 at 15:33, on Zulip):

Does that mean "if we apply that kind of thinking to NLL's role in the 2015 edition" ?

pnkfelix (Sep 05 2018 at 15:35, on Zulip):

because I think I'm still confused about the plan. It sound like you are saying that "the plan" is to enable NLL (as if it were another 2018 edition feature) so that it gets applied without one having to opt into it via a #![feature(nll)]

pnkfelix (Sep 05 2018 at 15:36, on Zulip):

and I agree that if the beta PR does "just turn #![feature(nll)] on by default, regardless of the edition setting, then that would be bad.

pnkfelix (Sep 05 2018 at 15:36, on Zulip):

but I also do not see how that could possibly be "the plan"

pnkfelix (Sep 05 2018 at 15:36, on Zulip):

surely the set of flags that are enabled has to be implied by the edition setting?

pnkfelix (Sep 05 2018 at 15:38, on Zulip):

And so, the person setting up the beta release could (for whatever reason), set things up so that edition:2018 implies #![feature(nll)] (which would be a mistake in my opinion). But that would not affect the edition:2015 code, right?

lqd (Sep 05 2018 at 15:39, on Zulip):

(that's how I understood this: beta on 2015 == AST, beta on 2018 == migrate)

nikomatsakis (Sep 05 2018 at 15:47, on Zulip):

@pnkfelix I guess it depends on what other features do. But basically I think that we should do:

The key point being that "opt in" is not possible on stable

pnkfelix (Sep 05 2018 at 15:47, on Zulip):

even if "opt in" means selecting `edition:2018" ?

pnkfelix (Sep 05 2018 at 15:48, on Zulip):

or what your bullet point meant to imply "edition:2015" as context?

nikomatsakis (Sep 05 2018 at 15:48, on Zulip):

As such, the plan is that on September 13th, for the 1.30 beta, we will produce a beta with the Rust 2018 features as stable.

nikomatsakis (Sep 05 2018 at 15:48, on Zulip):

from https://internals.rust-lang.org/t/rust-2018-release-schedule-and-extended-beta/8076

nikomatsakis (Sep 05 2018 at 15:48, on Zulip):

even if "opt in" means selecting `edition:2018" ?

yes, and based on that post, that sounds like what "opt in" means

nikomatsakis (Sep 05 2018 at 15:48, on Zulip):

so it becomes:

nikomatsakis (Sep 05 2018 at 15:49, on Zulip):

(Hmm, I suppose there would also be the option of using #![feature(nll)] -- i.e., hard errors?)

nikomatsakis (Sep 05 2018 at 15:49, on Zulip):

was that the whole point of this discussion and I was overlooking it? :)

pnkfelix (Sep 05 2018 at 15:49, on Zulip):

I don't know, what are we talking about again?

nikomatsakis (Sep 05 2018 at 15:50, on Zulip):

I don't know anymore :P

nikomatsakis (Sep 05 2018 at 15:50, on Zulip):

Let's reset I guess. The question is "what behavior will the RC have"

nikomatsakis (Sep 05 2018 at 15:50, on Zulip):

I think we all agree that for 2015 Rust, it will use AST

nikomatsakis (Sep 05 2018 at 15:50, on Zulip):

For 2018 Rust, it will either use Migrate or .. perhaps we just want NLL, full stop?

pnkfelix (Sep 05 2018 at 15:50, on Zulip):

I think we all agree that for 2015 Rust, it will use AST

hah that the was the one thing that I thought you might not agree on

nikomatsakis (Sep 05 2018 at 15:52, on Zulip):

well we all agree now, right?

nikomatsakis (Sep 05 2018 at 15:52, on Zulip):

I was disputing that initially :)

pnkfelix (Sep 05 2018 at 15:52, on Zulip):

the question of whether to have edition:2018 be NLL, full stop, is interesting

pnkfelix (Sep 05 2018 at 15:53, on Zulip):

but probably needs a different topic. :)

lqd (Sep 05 2018 at 15:53, on Zulip):

as in, not the migrate mode ?

pnkfelix (Sep 05 2018 at 15:53, on Zulip):

Sadly I think a big reason that I would argue to keep edition:2018=Migrate is because of the "breathing room" that I've often referenced it giving us

pnkfelix (Sep 05 2018 at 15:54, on Zulip):

As in, the time to explore ways to fix completeness bugs that, under migrate mode, it would warn about

nikomatsakis (Sep 05 2018 at 15:55, on Zulip):

Hmm. I think migration mode is probably fine.

nikomatsakis (Sep 05 2018 at 15:55, on Zulip):

Though I'd be happy if we could ship pure NLL mode in Rust 2018 sooner rather than later

nikomatsakis (Sep 05 2018 at 15:55, on Zulip):

but I'm happy to start with Migration

pnkfelix (Sep 05 2018 at 15:56, on Zulip):

An upgrade from edition:2018=Migrate to edition:2018=NLL seems like something that would be easy to backport, if desired.

pnkfelix (Sep 05 2018 at 15:56, on Zulip):

(famous last words?)

nikomatsakis (Sep 05 2018 at 15:57, on Zulip):

I could imagine maybe RC2 being the full mode

nikomatsakis (Sep 05 2018 at 15:57, on Zulip):

if we make progress on the last few regressions

nikomatsakis (Sep 05 2018 at 15:57, on Zulip):

but let's not worry about that

nikomatsakis (Sep 05 2018 at 15:57, on Zulip):

+1 to migrate

lqd (Sep 05 2018 at 15:58, on Zulip):

quick question about that (but might be better suited for another topic) stabilization by changing such defaults, impacts tests right ?

pnkfelix (Sep 05 2018 at 15:59, on Zulip):

yeah, since there's some number of tests that are checking that we warn rather than error, etc.

lqd (Sep 05 2018 at 15:59, on Zulip):

what would be required for those tests to be done, in the PR implementing this edition2015=AST, edition2018=migrate switch ?

pnkfelix (Sep 05 2018 at 15:59, on Zulip):

but we don't have a compare-mode=2018 ("yet"), so the impact may be less than one might think

nikomatsakis (Sep 05 2018 at 16:00, on Zulip):

I am also reminded that to truly kill the AST checker, we have to port the WF code to use NLL. Bah.

pnkfelix (Sep 05 2018 at 16:00, on Zulip):

as in, the tests that are checking the behavior of edition:2018 are all explicitly individually opting into selecting that edition

lqd (Sep 05 2018 at 16:03, on Zulip):

I guess I can also try to implement it and see what breaks

lqd (Sep 05 2018 at 16:04, on Zulip):

flipping the switches is the easy part :)

pnkfelix (Sep 05 2018 at 16:04, on Zulip):

@lqd what is there to be implemented? I thought we already had things set up so that edition:2018 implies borrowck=migrate

pnkfelix (Sep 05 2018 at 16:05, on Zulip):

( I'm so confused by this conversation )

lqd (Sep 05 2018 at 16:05, on Zulip):

that is true :thinking:

lqd (Sep 05 2018 at 16:05, on Zulip):

it's this "we need to make a PR to stabilize NLL for the RC / beta" that is confusing

pnkfelix (Sep 05 2018 at 16:05, on Zulip):

right, I keep trying to say

lqd (Sep 05 2018 at 16:06, on Zulip):

things are setup exactly like you say for sure

pnkfelix (Sep 05 2018 at 16:06, on Zulip):

"to 'stabilize' NLL according to our internal plan, ... we dont do anything."

lqd (Sep 05 2018 at 16:06, on Zulip):

maybe we don't need to do anything

lqd (Sep 05 2018 at 16:06, on Zulip):

right

pnkfelix (Sep 05 2018 at 16:06, on Zulip):

"its if someone wants to stop NLL from getting implicitly stabilized under our internal plan, then someone would need to do something." :)

lqd (Sep 05 2018 at 16:06, on Zulip):

(so just comparing perf for the incremental use cases is all I needed to do, and we should have these numbers soon)

Last update: Nov 21 2019 at 13:10UTC