Stream: t-compiler/wg-nll

Topic: #53172-nll-bootstrap


nikomatsakis (Aug 07 2018 at 20:52, on Zulip):

I'm opening this topic to discuss #53172. cc @Bernardo Meurer, who is interested in doing some of the crates there.

Bernardo Meurer (Aug 07 2018 at 20:53, on Zulip):

thanks @nikomatsakis ! I'm going to start with all the liballoc crates and see how it goes

Matthew Jasper (Aug 07 2018 at 20:54, on Zulip):

surely start with librustc_mir :mischievous:

Bernardo Meurer (Aug 07 2018 at 20:55, on Zulip):

Sure, but why do you say so?

nikomatsakis (Aug 07 2018 at 20:55, on Zulip):

because that is where the NLL implementation is, I suspect. =)

lqd (Aug 07 2018 at 20:55, on Zulip):

borrowckception!

Matthew Jasper (Aug 07 2018 at 20:56, on Zulip):

Because it's the one I work on most

Bernardo Meurer (Aug 07 2018 at 20:57, on Zulip):

Ha! Alright!

lqd (Aug 07 2018 at 20:57, on Zulip):

I think between the ice and other fixes and (I think) Matthew's PR, bootstrapping should probably go smoothly :)

lqd (Aug 07 2018 at 21:16, on Zulip):

hum btw do we just want the nll feature flag here, or just on stage1 as we did last time we tried ?

lqd (Aug 07 2018 at 21:16, on Zulip):

(or say, not stage 0)

nikomatsakis (Aug 07 2018 at 21:20, on Zulip):

ah @lqd good point

nikomatsakis (Aug 07 2018 at 21:20, on Zulip):

@Bernardo Meurer you probably actually want #![cfg_attr(not(stage0), feature(nll))]

Matthew Jasper (Aug 07 2018 at 21:21, on Zulip):

I think that #52810 and #52834 are the only issues that will block bootstrapping on the stage 0 compile, I think they affect one crate each (actually, I think one only affects a dependency).

nikomatsakis (Aug 07 2018 at 21:22, on Zulip):

@Matthew Jasper aren't both of them closed?

nikomatsakis (Aug 07 2018 at 21:22, on Zulip):

or are you saying "with stage0 builds"

Matthew Jasper (Aug 07 2018 at 21:22, on Zulip):

They didn't make beta

nikomatsakis (Aug 07 2018 at 21:22, on Zulip):

k

Bernardo Meurer (Aug 07 2018 at 21:23, on Zulip):

Just got the compiler to build :)

Bernardo Meurer (Aug 07 2018 at 21:25, on Zulip):

Hmm, do I need something else to run x.py test? Mine is failing with some missing files

Bernardo Meurer (Aug 07 2018 at 21:26, on Zulip):

https://gist.github.com/bemeurer/fd38ae4df8ffcf610f4e663869d32f8a

lqd (Aug 07 2018 at 21:26, on Zulip):

@Matthew Jasper do you usually try this with x.py check as I did ? Should be way faster for Bernardo if they're doing a crate at a time

nikomatsakis (Aug 07 2018 at 21:26, on Zulip):

definitely use ./x.py check !

Bernardo Meurer (Aug 07 2018 at 21:27, on Zulip):

I have quite a lot of server power :P

nikomatsakis (Aug 07 2018 at 21:27, on Zulip):

wait, does ./x.py check check also stage1 etc?

nikomatsakis (Aug 07 2018 at 21:29, on Zulip):

I updated https://github.com/rust-lang/rust/issues/53172#issue-348463765 with better instructions

Bernardo Meurer (Aug 07 2018 at 21:29, on Zulip):

What's stage0/stage1?

lqd (Aug 07 2018 at 21:29, on Zulip):

I happened to be doing this in the background so I don't remember :/ I did a lot of regular stage2 builds though, but no tests

Matthew Jasper (Aug 07 2018 at 21:29, on Zulip):

I ran ./x.py clean, RUSTFLAGS_STAGE_NOT_0='...' ./x.py build, not that I'd recommend it.

nikomatsakis (Aug 07 2018 at 21:29, on Zulip):

@Bernardo Meurer https://rust-lang-nursery.github.io/rustc-guide/how-to-build-and-run.html#running-xpy-and-building-a-stage1-compiler

nikomatsakis (Aug 07 2018 at 21:29, on Zulip):

take a look at that, which explains the background :)

Bernardo Meurer (Aug 07 2018 at 21:29, on Zulip):

Woohoo, check pass

nikomatsakis (Aug 07 2018 at 21:30, on Zulip):

I guess to be safe we can do ./x.py build --stage 1

nikomatsakis (Aug 07 2018 at 21:30, on Zulip):

um yeah

nikomatsakis (Aug 07 2018 at 21:30, on Zulip):

now that I think about it

lqd (Aug 07 2018 at 21:30, on Zulip):

oh yeah rustflags would fail for the dependencies

nikomatsakis (Aug 07 2018 at 21:30, on Zulip):

x.py check doesn't produce an executable

nikomatsakis (Aug 07 2018 at 21:30, on Zulip):

so how could it possibly check stage1 =)

lqd (Aug 07 2018 at 21:30, on Zulip):

lol

Bernardo Meurer (Aug 07 2018 at 21:31, on Zulip):

heh

Matthew Jasper (Aug 07 2018 at 21:31, on Zulip):

All our dependencies worked when I tried.

nikomatsakis (Aug 07 2018 at 21:31, on Zulip):

@Matthew Jasper I was figuring we could add #![feature] gates so that we can actually commit them

nikomatsakis (Aug 07 2018 at 21:31, on Zulip):

but it's probably worth just trying the RUSTFLAGS thing and see what happens

nikomatsakis (Aug 07 2018 at 21:31, on Zulip):

maybe everything Just Works

lqd (Aug 07 2018 at 21:33, on Zulip):

don't need x.py test for sure though @Bernardo Meurer in case you need a bit quicker turnaround

nikomatsakis (Aug 07 2018 at 21:33, on Zulip):

yes

nikomatsakis (Aug 07 2018 at 21:33, on Zulip):

./x.py build or ./x.py build --stage 1 ought to suffice

lqd (Aug 07 2018 at 21:33, on Zulip):

but at this point I myself would add the flag to half the crates in one go

lqd (Aug 07 2018 at 21:34, on Zulip):

instead of waiting 5-10 mins for each (but I had less server power)

Bernardo Meurer (Aug 07 2018 at 21:36, on Zulip):

I'm compiling on server with 2x Intel(R) Xeon(R) CPU E5-2630 v4

Bernardo Meurer (Aug 07 2018 at 21:36, on Zulip):

God bless companies that let you use stuff for personal projects!

nikomatsakis (Aug 07 2018 at 21:45, on Zulip):

that reminds me that I wanted to try enabling parallel compilation

nikomatsakis (Aug 07 2018 at 21:46, on Zulip):

I feel like I tried it without success once, but if so, I should try again and figure out why

Bernardo Meurer (Aug 07 2018 at 21:48, on Zulip):

We don't have parallel compilation already?

nikomatsakis (Aug 07 2018 at 21:54, on Zulip):

nope

nikomatsakis (Aug 07 2018 at 21:54, on Zulip):

I mean across crates yes

nikomatsakis (Aug 07 2018 at 21:54, on Zulip):

but within one crate, not

nikomatsakis (Aug 07 2018 at 21:54, on Zulip):

there is experimental support for it though

Bernardo Meurer (Aug 07 2018 at 21:59, on Zulip):

That would be really, really nice

nikomatsakis (Aug 08 2018 at 10:45, on Zulip):

@Bernardo Meurer how'd it go?

memoryruins (Aug 08 2018 at 22:54, on Zulip):

Hey :wave: I'll build librustc_metadata tonight and report back :)

memoryruins (Aug 09 2018 at 00:37, on Zulip):

The build was successful! Would a single PR for each indivdual crate be preferred, or would a PR with multiple commits, one per build passing crate, be acceptable after trying a few in succession? Next-up: librustc_lint

simulacrum (Aug 09 2018 at 01:46, on Zulip):

@memoryruins I'd probably just have one commit if there's no changes for all crates, and if a crate has changes then isolate it into its own commit

memoryruins (Aug 09 2018 at 02:33, on Zulip):

Sounds good! stage1 successfully built with nll enabled on librustc_lint and libgraphviz.

memoryruins (Aug 09 2018 at 02:35, on Zulip):

For the kicks and to try multiple at once while I sleep, I'll see if it builds with nll enabled on liballoc, liballoc_jemalloc, liballoc_system, librustc_borrowck, librustc_codegen_llvm, and librustc_codegen_utils

memoryruins (Aug 09 2018 at 04:16, on Zulip):

It hit an error while compiling stage1 of librustc_codegen_llvm https://gist.github.com/memoryruins/14a2aad7fc85d0429ae9e4240ec0dacb

memoryruins (Aug 09 2018 at 04:19, on Zulip):

Trying the set again without the above.

memoryruins (Aug 09 2018 at 05:58, on Zulip):

They built! Opened a PR https://github.com/rust-lang/rust/pull/53211

memoryruins (Aug 09 2018 at 08:21, on Zulip):

more crates are building, didn't list these beforehand in here https://github.com/rust-lang/rust/pull/53214

nikomatsakis (Aug 09 2018 at 09:57, on Zulip):

@memoryruins nice! I updated the tracking issue

memoryruins (Aug 09 2018 at 11:09, on Zulip):

another chunk successfully builds locally :) https://github.com/rust-lang/rust/pull/53219

memoryruins (Aug 09 2018 at 11:22, on Zulip):

now to try some of these larger crates: hello librustc, libcore, and data_structures

nikomatsakis (Aug 09 2018 at 11:27, on Zulip):

very nice

nikomatsakis (Aug 09 2018 at 11:28, on Zulip):

I'm wondering whether this will affect bootstrap time, and how much to worry about (cc @simulacrum).

I guess the only real answer is "land it and see"

memoryruins (Aug 09 2018 at 11:29, on Zulip):

wasn't it only a month ago that the last bootstrap was filled with unused mut warnings? y'all have done some incredible work since then.

memoryruins (Aug 09 2018 at 11:32, on Zulip):

I had that in mind as well, and so far the build times between stage0 and stage1 appear to be similar (no idea if thats a 'good' way to roughly judge it. first time i've built rustc haha)

nikomatsakis (Aug 09 2018 at 12:25, on Zulip):

@memoryruins have you measured, or just "feels similar"?

nikomatsakis (Aug 09 2018 at 12:25, on Zulip):

still, even the latter is good =)

simulacrum (Aug 09 2018 at 12:51, on Zulip):

stage0 vs stage1 is a decent, if not great comparison. Stage 0 is usually faster IIRC for unknown reasons.

I'm not too worried about build times -- if we have perf down to ~5-10% over previous then realistically on Travis that moves the minimum up by ~2-3 minutes which we should be able to afford I think

simulacrum (Aug 09 2018 at 12:51, on Zulip):

And it'll encourage us to optimize performance :)

memoryruins (Aug 09 2018 at 13:02, on Zulip):

feels similar. the difference might end up more apparent when all of the crates are enabled altogether

nikomatsakis (Aug 09 2018 at 13:24, on Zulip):

@memoryruins we should invetigate that one error

nikomatsakis (Aug 09 2018 at 13:24, on Zulip):

/me looks

nikomatsakis (Aug 09 2018 at 13:25, on Zulip):

This was the gist (for reference): https://gist.github.com/memoryruins/14a2aad7fc85d0429ae9e4240ec0dacb

nikomatsakis (Aug 09 2018 at 13:28, on Zulip):

I think the error is legit... @eddyb, you around?

nikomatsakis (Aug 09 2018 at 13:29, on Zulip):

well wait

nikomatsakis (Aug 09 2018 at 13:30, on Zulip):

er, yes, I think it's legit, iiuc

nikomatsakis (Aug 09 2018 at 13:30, on Zulip):

an interesting example where enhanced error messages with more details would be quite nice

nikomatsakis (Aug 09 2018 at 13:30, on Zulip):

/me ponders trying to reduce it

Matthew Jasper (Aug 09 2018 at 13:35, on Zulip):

I think the function called where the borrow is created (which I can't see the name of, and I can't check easily on a phone) has too strict a lifetime annotation on it. But it's an extern function written in C++, so I'm not completely sure.

nikomatsakis (Aug 09 2018 at 13:36, on Zulip):

reduced: https://play.rust-lang.org/?gist=6e22b15051f9a108d09f28a0b657f717&version=nightly&mode=debug&edition=2015

nikomatsakis (Aug 09 2018 at 13:36, on Zulip):

yeah, I can't quite tell

nikomatsakis (Aug 09 2018 at 13:36, on Zulip):

there are Drops that free memory

nikomatsakis (Aug 09 2018 at 13:38, on Zulip):

also, the "in later iteration of loop" thing is totally wrong

nikomatsakis (Aug 09 2018 at 13:41, on Zulip):

adding a take() method would fix the error, I think, and seems safe...?

nikomatsakis (Aug 09 2018 at 13:41, on Zulip):

https://play.rust-lang.org/?gist=64c20d529ad49d5871417c391a8dbcde&version=nightly&mode=debug&edition=2015

nikomatsakis (Aug 09 2018 at 13:41, on Zulip):

like that :point_up:

nikomatsakis (Aug 09 2018 at 13:42, on Zulip):

I'm gonna file a bug

nikomatsakis (Aug 09 2018 at 13:44, on Zulip):

filed https://github.com/rust-lang/rust/issues/53221

memoryruins (Aug 09 2018 at 14:55, on Zulip):

update -- the following crates build: libcore, libprofiler_builtins, librustc, librustc_allocator, librustc_data_structures, libserialize, libsyntax, and libtest

memoryruins (Aug 09 2018 at 14:55, on Zulip):

libsyntax_ext hits the following error https://gist.github.com/memoryruins/b5a05efbd217608ca010c1f2252ef5c6

Jake Goulding (Aug 09 2018 at 14:59, on Zulip):

Does it need to be mutable? a.k.a. is it a NLL bug or NLL improvement?

simulacrum (Aug 09 2018 at 15:00, on Zulip):

(you can usually tell by just removing the mut keyword)

nikomatsakis (Aug 09 2018 at 15:00, on Zulip):

it looks correct to me

nikomatsakis (Aug 09 2018 at 15:01, on Zulip):

but yes @simulacrum's test sounds like the right thing to do :)

Jake Goulding (Aug 09 2018 at 15:01, on Zulip):

That's an ambiguous answer @nikomatsakis ;-)

nikomatsakis (Aug 09 2018 at 15:01, on Zulip):

sorry, I mean NLL's warning looks correct

nikomatsakis (Aug 09 2018 at 15:01, on Zulip):

the mut is not needed

Jake Goulding (Aug 09 2018 at 15:02, on Zulip):

Is it possible old borrowck might require mut, thus there's no code that works on both?

nikomatsakis (Aug 09 2018 at 15:06, on Zulip):

one way to find out!

nikomatsakis (Aug 09 2018 at 15:06, on Zulip):

@memoryruins you gonna try removing those annotations?

memoryruins (Aug 09 2018 at 15:09, on Zulip):

I'll remove them and try it out after this batch builds :)

memoryruins (Aug 09 2018 at 19:37, on Zulip):

Removing the unnecessary mut annotation did the trick ^^ https://github.com/rust-lang/rust/pull/53230

memoryruins (Aug 09 2018 at 19:41, on Zulip):

I think libstd_unicode can be removed from the bootstrap list since it's now in libcore

nikomatsakis (Aug 09 2018 at 20:40, on Zulip):

@memoryruins so @eddyb left a comment here suggesting where the problem may be with the LLVM crate

nikomatsakis (Aug 09 2018 at 20:41, on Zulip):

think you could try applying that diff and see if makes the LLVM crate build?

memoryruins (Aug 09 2018 at 21:33, on Zulip):

will do! If it builds, I'll add it to the part 4 PR

memoryruins (Aug 09 2018 at 21:33, on Zulip):

since travis caught an unused mut anyway

memoryruins (Aug 09 2018 at 21:33, on Zulip):

in one of the tests

simulacrum (Aug 09 2018 at 21:34, on Zulip):

I should really figure out how to get x.py check compiling tests too (--all-targets)

memoryruins (Aug 09 2018 at 23:55, on Zulip):

part 4 now passes on travis :crab:
codegen_llvm ran into an unused mut after eddy's suggestion (potentially progress)

error: variable does not need to be mutable
  --> librustc_codegen_llvm\type_of.rs:92:25
92 |                     let mut llty = Type::named_struct(cx, name);
   |                         ----^^^^

I'll try building it again without the mut annotation and look into the last few crates tomorrow, unless if anyone wants to have a go at it while I'm away

memoryruins (Aug 09 2018 at 23:57, on Zulip):

@Bernardo Meurer any luck with bootstrapping librustc_mir?

memoryruins (Aug 10 2018 at 08:12, on Zulip):

let's try all the remaining crates all at once

memoryruins (Aug 10 2018 at 08:17, on Zulip):

does the parent stdsimd have a lib.rs? only one I saw was in the stdsimd/crates/stdsimd/src directory

memoryruins (Aug 10 2018 at 08:18, on Zulip):

not seeing a crate/lib.rs for libbacktrace, the directory is all .c/.h files. is the crate located elsewhere or would it be considered part of libstd's sys?

nikomatsakis (Aug 10 2018 at 08:21, on Zulip):

I'm not really familiar with those crates :(

nikomatsakis (Aug 10 2018 at 08:22, on Zulip):

libbacktrace might be just C code

memoryruins (Aug 10 2018 at 08:26, on Zulip):

libc and compiler-builtins are both external crates, so I'm guessing we wouldnt be able to add the feature flag to those

memoryruins (Aug 10 2018 at 08:27, on Zulip):

that leaves codegen_llvm, librustc_mir, and libstd ^^ (building now)

memoryruins (Aug 10 2018 at 08:29, on Zulip):

no it looks like builtins has not(stage0) flags too

memoryruins (Aug 10 2018 at 08:29, on Zulip):

it would just need to be a separate PR right?

nikomatsakis (Aug 10 2018 at 08:37, on Zulip):

probably? builtins might be a bit special :)

nikomatsakis (Aug 10 2018 at 08:37, on Zulip):

it's probably good enough as is

nikomatsakis (Aug 10 2018 at 08:38, on Zulip):

at some point we'll swap the "default" flag for everything

nikomatsakis (Aug 10 2018 at 08:38, on Zulip):

the work you've already done gives me a lot of confidence this won't cause the compiler to fall over :)

memoryruins (Aug 10 2018 at 10:35, on Zulip):

pushed some commits to part 4. libstd, librustc_mir, and librustc_codegen_llvm build! eddyb's suggestion on changing the signature solved the error. just to check, builtins also successfully built locally

memoryruins (Aug 10 2018 at 10:35, on Zulip):

now to see if it passes on travis and it should be good to go

memoryruins (Aug 10 2018 at 17:31, on Zulip):

ayyy it passed

Last update: Nov 21 2019 at 13:50UTC