I'm opening this topic to discuss #53172. cc @Bernardo Meurer, who is interested in doing some of the crates there.
thanks @nikomatsakis ! I'm going to start with all the
liballoc crates and see how it goes
surely start with librustc_mir :mischievous:
Sure, but why do you say so?
because that is where the NLL implementation is, I suspect. =)
Because it's the one I work on most
I think between the ice and other fixes and (I think) Matthew's PR, bootstrapping should probably go smoothly :)
hum btw do we just want the nll feature flag here, or just on stage1 as we did last time we tried ?
(or say, not stage 0)
ah @lqd good point
@Bernardo Meurer you probably actually want
@Matthew Jasper aren't both of them closed?
or are you saying "with stage0 builds"
They didn't make beta
Just got the compiler to build :)
Hmm, do I need something else to run
x.py test? Mine is failing with some missing files
@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
./x.py check !
I have quite a lot of server power :P
./x.py check check also stage1 etc?
I updated https://github.com/rust-lang/rust/issues/53172#issue-348463765 with better instructions
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
RUSTFLAGS_STAGE_NOT_0='...' ./x.py build, not that I'd recommend it.
take a look at that, which explains the background :)
Woohoo, check pass
I guess to be safe we can do
./x.py build --stage 1
now that I think about it
oh yeah rustflags would fail for the dependencies
x.py check doesn't produce an executable
so how could it possibly check stage1 =)
All our dependencies worked when I tried.
@Matthew Jasper I was figuring we could add
#![feature] gates so that we can actually commit them
but it's probably worth just trying the
RUSTFLAGS thing and see what happens
maybe everything Just Works
don't need x.py test for sure though @Bernardo Meurer in case you need a bit quicker turnaround
./x.py build or
./x.py build --stage 1 ought to suffice
but at this point I myself would add the flag to half the crates in one go
instead of waiting 5-10 mins for each (but I had less server power)
I'm compiling on server with 2x Intel(R) Xeon(R) CPU E5-2630 v4
God bless companies that let you use stuff for personal projects!
that reminds me that I wanted to try enabling parallel compilation
I feel like I tried it without success once, but if so, I should try again and figure out why
We don't have parallel compilation already?
I mean across crates yes
but within one crate, not
there is experimental support for it though
That would be really, really nice
@Bernardo Meurer how'd it go?
Hey :wave: I'll build
librustc_metadata tonight and report back :)
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:
@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
Sounds good! stage1 successfully built with nll enabled on
For the kicks and to try multiple at once while I sleep, I'll see if it builds with nll enabled on
It hit an error while compiling stage1 of
Trying the set again without the above.
They built! Opened a PR https://github.com/rust-lang/rust/pull/53211
more crates are building, didn't list these beforehand in here https://github.com/rust-lang/rust/pull/53214
@memoryruins nice! I updated the tracking issue
another chunk successfully builds locally :) https://github.com/rust-lang/rust/pull/53219
now to try some of these larger crates: hello
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"
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.
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)
@memoryruins have you measured, or just "feels similar"?
still, even the latter is good =)
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
And it'll encourage us to optimize performance :)
feels similar. the difference might end up more apparent when all of the crates are enabled altogether
@memoryruins we should invetigate that one error
This was the gist (for reference): https://gist.github.com/memoryruins/14a2aad7fc85d0429ae9e4240ec0dacb
I think the error is legit... @eddyb, you around?
er, yes, I think it's legit, iiuc
an interesting example where enhanced error messages with more details would be quite nice
/me ponders trying to reduce it
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.
yeah, I can't quite tell
there are Drops that free memory
also, the "in later iteration of loop" thing is totally wrong
take() method would fix the error, I think, and seems safe...?
like that :point_up:
I'm gonna file a bug
update -- the following crates build:
libsyntax_ext hits the following error https://gist.github.com/memoryruins/b5a05efbd217608ca010c1f2252ef5c6
Does it need to be mutable? a.k.a. is it a NLL bug or NLL improvement?
(you can usually tell by just removing the mut keyword)
it looks correct to me
but yes @simulacrum's test sounds like the right thing to do :)
That's an ambiguous answer @nikomatsakis ;-)
sorry, I mean NLL's warning looks correct
the mut is not needed
Is it possible old borrowck might require mut, thus there's no code that works on both?
one way to find out!
@memoryruins you gonna try removing those annotations?
I'll remove them and try it out after this batch builds :)
Removing the unnecessary mut annotation did the trick ^^ https://github.com/rust-lang/rust/pull/53230
I think libstd_unicode can be removed from the bootstrap list since it's now in libcore
@memoryruins so @eddyb left a comment here suggesting where the problem may be with the LLVM crate
think you could try applying that diff and see if makes the LLVM crate build?
will do! If it builds, I'll add it to the part 4 PR
since travis caught an unused mut anyway
in one of the tests
I should really figure out how to get x.py check compiling tests too (--all-targets)
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
@Bernardo Meurer any luck with bootstrapping
let's try all the remaining crates all at once
does the parent stdsimd have a lib.rs? only one I saw was in the stdsimd/crates/stdsimd/src directory
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?
I'm not really familiar with those crates :(
libbacktrace might be just C code
that leaves codegen_llvm, librustc_mir, and libstd ^^ (building now)
no it looks like builtins has not(stage0) flags too
it would just need to be a separate PR right?
probably? builtins might be a bit special :)
it's probably good enough as is
at some point we'll swap the "default" flag for everything
the work you've already done gives me a lot of confidence this won't cause the compiler to fall over :)
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
now to see if it passes on travis and it should be good to go
ayyy it passed