Stream: t-compiler/wg-nll

Topic: crater-run-and-analysis


nikomatsakis (Jun 22 2018 at 15:06, on Zulip):

I was thinking that we ought to do a crater run with NLL enabled and then try to divvy up the work of analyzing the results -- I thought that e.g. folks like @Jake Goulding might be interested in looking at places where NLL reports new errors and trying to decide if the code was broken to start or not.

nikomatsakis (Jun 22 2018 at 15:07, on Zulip):

I am going to be away starting in about 22 minutes for the weekend, but maybe we can start in on this next week (or if people want to make steps in that direction independently, wfm).

I imagine we would do the following:

nikomatsakis (Jun 22 2018 at 15:07, on Zulip):

my goal here is to try and unearth bugs earlier rather than later

nikomatsakis (Jun 22 2018 at 15:07, on Zulip):

another similar goal would be trying to get rustc to bootstrap with NLL enabled and seeing what breaks

nikomatsakis (Jun 22 2018 at 15:07, on Zulip):

this could perhaps be done by adding #![feature(nll)] to every crate one by one

lqd (Jun 22 2018 at 15:40, on Zulip):

oh but for checking if bootstrapping works, only a x.py check would be enough right ? should go pretty fast

simulacrum (Jun 22 2018 at 15:43, on Zulip):

Yes, mostly -- there might be some parts (e.g., stdsimd/coresimd) that won't be checked via that approach, though. It's likely though that most issues can be resolved via x.py check.

lqd (Jun 22 2018 at 15:45, on Zulip):

@simulacrum great, thank you

simulacrum (Jun 22 2018 at 15:46, on Zulip):

we might not want to commit these into master since presumably they'd slow down bootstrap (quite yet) but it'd be good to do a survey

lqd (Jun 22 2018 at 16:13, on Zulip):

hehe MIR-borrowck borrowcking AST-borrowck AKA the passing of the torch

Jake Goulding (Jun 26 2018 at 16:58, on Zulip):

folks like @Jake Goulding might be interested in looking at places where NLL reports new errors

Sure, but don't let me get in y'all critical path ;-)

nikomatsakis (Jun 26 2018 at 17:44, on Zulip):

well the point is

nikomatsakis (Jun 26 2018 at 17:44, on Zulip):

this is not on the criticial path

nikomatsakis (Jun 26 2018 at 17:44, on Zulip):

and I'm looking to get a bit ahead of the game

nikomatsakis (Jun 26 2018 at 17:44, on Zulip):

and start to "parallelize" the effort of finding problems

Jake Goulding (Jun 26 2018 at 17:50, on Zulip):

then it might be a good fit for me

nikomatsakis (Jun 26 2018 at 22:34, on Zulip):

OK, I opened an issue about doing a crater run here: https://github.com/rust-lang/rust/issues/51827

I'll add some mentoring instructions for how to make the -Zborrowck=mir default change.

Now to find someone to run it. =)

simulacrum (Jun 26 2018 at 23:07, on Zulip):

try build and we should be able to schedule quickly -- we don't even need to run tests I believe so should be a 3-4 day operation :)

simulacrum (Jun 26 2018 at 23:08, on Zulip):

@lqd Also, I think it might be better to do #![cfg_attr(stage1, feature(nll))] for now

simulacrum (Jun 26 2018 at 23:09, on Zulip):

That we can make sure the compiler doesn't regress and land things incrementally

lqd (Jun 26 2018 at 23:10, on Zulip):

@simulacrum for bootstrap you mean ? rather than RUSTFLAGS_STAGE_NOT_0?

simulacrum (Jun 26 2018 at 23:10, on Zulip):

Yeah

lqd (Jun 26 2018 at 23:11, on Zulip):

/me hits ctrl and then C

lqd (Jun 26 2018 at 23:11, on Zulip):

alright, thank you :)

simulacrum (Jun 26 2018 at 23:11, on Zulip):

@lqd Well, if it works right away with RUSTFLAGS_STAGE_NOT_0, then great, but otherwise I would do this incrementally

lqd (Jun 26 2018 at 23:12, on Zulip):

absolutely

nikomatsakis (Jun 27 2018 at 00:02, on Zulip):

@simulacrum I was reluctant to add that if it will regress bors cycle time

nikomatsakis (Jun 27 2018 at 00:02, on Zulip):

we should measure

nikomatsakis (Jun 27 2018 at 00:02, on Zulip):

but I like the idea of "locking in" crate by crate

simulacrum (Jun 27 2018 at 01:36, on Zulip):

Yeah, but even for a PR that "gets us ready" it'll be nice to work through things

simulacrum (Jun 27 2018 at 01:36, on Zulip):

Plus NLL is looking faster by the day -- so it might not actually regress us all that much (i.e., be within margin of error anyway)

lqd (Jun 27 2018 at 17:43, on Zulip):

quick summary of background bootstrapping: 10-11 crates refuse to build, because of 1) a couple of known ICEs (incl the one that possibly stops futures from compiling), 2) I think that the "unused mut" lint is buggy in 1 or 2 usage patterns (eg w/ closures), 3) a simple but confusing (to me) piece of code which might be "this should have been a warning before and is now warned by MIR borrowck" 4) some correct new "unused mut" denied warnings 5) I'm still not done investigating, esp the errors (in 2 crates) actually about lifetimes and borrowing

nikomatsakis (Jun 27 2018 at 17:44, on Zulip):

nice!

simulacrum (Jun 27 2018 at 17:45, on Zulip):

(3) sounds somewhat concerning, is that warning wording correct? I can't recall warnings from borrowck...

lqd (Jun 27 2018 at 17:45, on Zulip):

WIP summary which I'll eventually turn into something more usable is here

lqd (Jun 27 2018 at 17:45, on Zulip):

@simulacrum let me get you the repro

lqd (Jun 27 2018 at 17:48, on Zulip):

https://play.rust-lang.org/?gist=bad0f50fef757aa9ba1499254b104fa2&version=nightly&mode=debug -> this gives an unused mut warning under NLL -- and the two things I noted were: 1) NLL vs AST, and 2) move/non-move closure

lqd (Jun 27 2018 at 17:50, on Zulip):

ie, with a move closure either AST or MIR is right that the mut is unused

nikomatsakis (Jun 27 2018 at 17:50, on Zulip):

yeah I think there are some other reported bugs about the "unused mut" lint

lqd (Jun 27 2018 at 17:51, on Zulip):

I've seem some in closures where the mut is inferred, and others with I think mutable references (and sometimes closures are involved as well)

simulacrum (Jun 27 2018 at 17:53, on Zulip):

ah, unused mut, okay

Jake Goulding (Jun 27 2018 at 17:57, on Zulip):

I've never thought about if an assigned-to variable is moved into a move closure

nikomatsakis (Jun 27 2018 at 18:04, on Zulip):

I think I'm going to have to do a more thorough re-read of the used-mut code — it was kind of tricky to do, iirc, and I think there's probably some ways to make it simpler

nikomatsakis (Jun 27 2018 at 18:05, on Zulip):

now that we have a first draft and some known flaws :)

lqd (Jun 27 2018 at 18:08, on Zulip):

I’m not sure I reproduced the behavior of the rustc code accurately, and not just the warning; I’ve had real bugs that the unused mut warning noticed so it caught my eye but might be nothing

lqd (Jun 27 2018 at 21:26, on Zulip):

would we want to document cases where unused muts warnings were missed before, similarly to how we track "fixed by MIR borrowck" cases ? I'm looking at this one specifically, and if I needed to reproduce it (extracting the code naively does not reproduce the missing unused mut warning without NLL) ?

nikomatsakis (Jun 27 2018 at 21:33, on Zulip):

if the warnings are legit -- that is, the mut really is unnecessary -- seems ok?

lqd (Jun 27 2018 at 21:34, on Zulip):

it is unnecessary yeah AFAICT

nikomatsakis (Jun 27 2018 at 21:36, on Zulip):

ok, I'm not sure if we need to document it per se

lqd (Jun 27 2018 at 21:38, on Zulip):

servo also builds with deny warnings, and they've been surprised about new warnings since it broke their build

lqd (Jun 27 2018 at 21:39, on Zulip):

but for something as big as NLL, this new warning seems relatively unimportant for sure

simulacrum (Jun 27 2018 at 21:40, on Zulip):

We make essentially zero guarantees about deny warnings; we try to stem breakage but I don't see this as unexpected

nikomatsakis (Jun 27 2018 at 21:40, on Zulip):

my opinion is that if you build with deny(warnings)...well :) then you want warnings to braek your build.

nikomatsakis (Jun 27 2018 at 21:40, on Zulip):

but we should offer nice ways to handle it in CI with lint capping

nikomatsakis (Jun 27 2018 at 21:40, on Zulip):

in any case, we do aggressively fix and add lints...

lqd (Jun 27 2018 at 21:41, on Zulip):

oh agreed, I was just checking :)

lqd (Jun 28 2018 at 00:01, on Zulip):

(updated the WIP gist, there's only the 4-5 lifetime errors to check/repro left to do tomorrow — and writeup, issues, etc)

lqd (Jun 28 2018 at 11:40, on Zulip):

@nikomatsakis I'm having trouble reproducing the liballoc bootstrapping errors: extracting the 10-15 minimal types only yields warnings and not those errors. Specifically, this code seems to cause these errors apparently because of the interaction between the inner fn anonymous lifetime and its generic parameters. To clarify the error messages, since K & V are also generic parameters in the outer fn, the errors point to the _inner_ fn's K & V, clone_subtree

nikomatsakis (Jun 28 2018 at 12:30, on Zulip):

hmm

nikomatsakis (Jun 28 2018 at 12:31, on Zulip):

these errors are caused by adding #![feature(nll)] to liballoc?

nikomatsakis (Jun 28 2018 at 12:31, on Zulip):

well, probably #![cfg_attr(not(stage0), feature(nll))]?

nikomatsakis (Jun 28 2018 at 12:31, on Zulip):

I can try to take a look

lqd (Jun 28 2018 at 12:31, on Zulip):

yes, lemme get you the cfg

lqd (Jun 28 2018 at 12:32, on Zulip):

#![cfg_attr(stage1, feature(nll))]

simulacrum (Jun 28 2018 at 12:34, on Zulip):

Oh, yeah, we'll want to change stage1 to not(stage0). Minor nit though :)

lqd (Jun 28 2018 at 12:37, on Zulip):

yeah when we enable it in the future indeed, as I was just looking for bootstrapping issues for now :)

lqd (Jun 28 2018 at 17:09, on Zulip):

I managed to minimize the bootstrap failure on librustc_passes though, it looks cute but probably we're already tracking something like it ?

nikomatsakis (Jun 28 2018 at 17:09, on Zulip):

well

nikomatsakis (Jun 28 2018 at 17:09, on Zulip):

I do recall seing that

nikomatsakis (Jun 28 2018 at 17:10, on Zulip):

I think that it is correct for us to reject it

nikomatsakis (Jun 28 2018 at 17:10, on Zulip):

unless we choose to expand 2-phase borrows

nikomatsakis (Jun 28 2018 at 17:10, on Zulip):

I think I proposed it somewhere, motivated by a similar example

lqd (Jun 28 2018 at 17:10, on Zulip):

there were similar examples with "self / nested calls" and others in our "fixed by NLL" category

lqd (Jun 28 2018 at 17:13, on Zulip):

in any case this one is super easy to fix in rustc (like all the valid unused mut lint warnings)

lqd (Jun 29 2018 at 07:29, on Zulip):

interesting, the liballoc errors I mentioned earlier could actually be linked to temporary lifetimes and tail expressions cases mentioned on discord (it's not the same error message and involves anonymous lifetimes as well :thinking_face:)

nikomatsakis (Jun 29 2018 at 09:16, on Zulip):

hmm :(

nikomatsakis (Jun 29 2018 at 09:16, on Zulip):

we may have to see if we can sneak in some changes here

nikomatsakis (Jun 29 2018 at 09:16, on Zulip):

I've not seriously thought about it

lqd (Jun 29 2018 at 09:17, on Zulip):

(I could be very wrong ofc :3)

nikomatsakis (Jun 29 2018 at 09:18, on Zulip):

well I do notice it coming up a lot more now

nikomatsakis (Jun 29 2018 at 09:18, on Zulip):

presumably the AST borrowck was just buggy :/

lqd (Jun 29 2018 at 09:37, on Zulip):

yeah :/

lqd (Jun 29 2018 at 09:41, on Zulip):

apart from these still a bit unclear, it seems all the other things were either known issues (some with fixes) or correct warnings, so still good news I think

lqd (Jun 29 2018 at 09:50, on Zulip):

since some of those ICEs apparently don't appear on master anymore, I'll update and try again

nikomatsakis (Jun 29 2018 at 09:56, on Zulip):

sounds good

lqd (Jun 29 2018 at 15:18, on Zulip):

update: with some of the previous ICEs removed, I just found other cases of both correct and incorrect mut warnings; also got a weird broken MIR ICE which I had missed before but was already known from previous NLL bootstrapping attempts.

nikomatsakis (Jun 29 2018 at 15:25, on Zulip):

Are you filing (or going to file) issues on these things?

lqd (Jun 29 2018 at 15:25, on Zulip):

yeah

lqd (Jun 29 2018 at 15:26, on Zulip):

most of them are already filed (some multiple times) so I've been tagging a couple and adding minimized repros

lqd (Jun 29 2018 at 15:26, on Zulip):

wrt the liballoc errors you might have had a patch fixing them before @nikomatsakis ? https://github.com/rust-lang/rust/issues/48224#issuecomment-365952037

lqd (Jun 29 2018 at 15:27, on Zulip):

I'll summarize all this and report now on the issue we created at the last meeting

nikomatsakis (Jun 29 2018 at 17:03, on Zulip):

@lqd thank you so much for taking this on <3 =)

lqd (Jun 29 2018 at 17:04, on Zulip):

:) no problem!

lqd (Jun 29 2018 at 17:04, on Zulip):

I'm 99% done writing the "report", just trying to repro another unused mut warning, in case it's not one we already know

lqd (Jun 29 2018 at 17:05, on Zulip):

(I can publish right now and finish later if need be)

nikomatsakis (Jun 29 2018 at 17:11, on Zulip):

it's fine

nikomatsakis (Jun 29 2018 at 17:11, on Zulip):

I did have some patches for some crates

lqd (Jun 29 2018 at 17:39, on Zulip):

ok I'm finally done https://github.com/rust-lang/rust/issues/51823#issuecomment-401423108

nikomatsakis (Jun 29 2018 at 17:50, on Zulip):

seems like we should prioritize some of those problems (e.g., https://github.com/rust-lang/rust/issues/51351)

lqd (Jun 29 2018 at 18:04, on Zulip):

yeah it looks interesting

nikomatsakis (Jun 29 2018 at 19:10, on Zulip):

ok @lqd I found my fixes for https://github.com/rust-lang/rust/issues/48224

nikomatsakis (Jun 29 2018 at 19:10, on Zulip):

I will open a PR

lqd (Jun 29 2018 at 19:13, on Zulip):

sweet :)

lqd (Jun 29 2018 at 19:13, on Zulip):

so the problem was in the borrowck and not liballoc ?

nikomatsakis (Jun 29 2018 at 19:18, on Zulip):

no

nikomatsakis (Jun 29 2018 at 19:18, on Zulip):

https://github.com/rust-lang/rust/pull/51914

nikomatsakis (Jun 29 2018 at 19:20, on Zulip):

I'm trying to find the issue where we were talking about expanding two-phase to cover that scenario you highlighted

lqd (Jun 29 2018 at 19:20, on Zulip):

oh great

lqd (Jun 29 2018 at 19:21, on Zulip):

I did try adding such lifetimes to liballoc but I missed adding them to marker::Immut

nikomatsakis (Jun 29 2018 at 19:22, on Zulip):

that code is doing some complex stuff :)

nikomatsakis (Jun 29 2018 at 19:24, on Zulip):

so @simulacrum, @lqd, @Jake Goulding — just to be sure, nobody made any movement towards a crater run yet, right?

lqd (Jun 29 2018 at 19:24, on Zulip):

(:) even extracting it didn't repro the error so I gave up)

lqd (Jun 29 2018 at 19:24, on Zulip):

I myself didn't no

nikomatsakis (Jun 29 2018 at 19:24, on Zulip):

also, @lqd would you consider opening a PR adding #![feature(nll)] to the crates that worked to prevent further regressions?

nikomatsakis (Jun 29 2018 at 19:24, on Zulip):

not sure if we should or not

nikomatsakis (Jun 29 2018 at 19:24, on Zulip):

but might be a good idea

lqd (Jun 29 2018 at 19:25, on Zulip):

this would slow down the build a bit I guess

nikomatsakis (Jun 29 2018 at 19:26, on Zulip):

that's the concern, yeah

nikomatsakis (Jun 29 2018 at 19:26, on Zulip):

not sure how much

nikomatsakis (Jun 29 2018 at 19:26, on Zulip):

@lqd do you know what the original source was from which you reduced this example?

lqd (Jun 29 2018 at 19:27, on Zulip):

I think so yes, I'll look at my notes

lqd (Jun 29 2018 at 19:29, on Zulip):
- librustc_passes: 1 error
error[E0502]: cannot borrow `*self.tables` as immutable because it is also borrowed as mutable
   --> librustc_passes/rvalue_promotion.rs:206:76
    |
206 |         euv::ExprUseVisitor::new(self, tcx, param_env, &region_scope_tree, self.tables, None)
    |         -------------------------------------------------------------------^^^^^^^^^^^-------
    |         |                        |                                         |
    |         |                        |                                         immutable borrow occurs here
    |         |                        mutable borrow occurs here
| borrow later used here

from here

nikomatsakis (Jun 29 2018 at 19:30, on Zulip):

thanks

nikomatsakis (Jun 29 2018 at 19:30, on Zulip):

I filed https://github.com/rust-lang/rust/issues/51915

Jake Goulding (Jun 29 2018 at 19:31, on Zulip):

I have not done anything towards crater; I expected I'd just help read through results

nikomatsakis (Jun 29 2018 at 19:31, on Zulip):

that sounds right

nikomatsakis (Jun 29 2018 at 19:31, on Zulip):

I may try to make that branch then

Jake Goulding (Jun 29 2018 at 19:37, on Zulip):

Re: adding the outlives to BTreeMap — I wonder if anyone adds the outlives annotations by default, or if we all wait for the compiler error

nikomatsakis (Jun 29 2018 at 19:37, on Zulip):

I certainly wait for the error

nikomatsakis (Jun 29 2018 at 19:37, on Zulip):

BUT in Rust 2018

nikomatsakis (Jun 29 2018 at 19:37, on Zulip):

they will be inferred

nikomatsakis (Jun 29 2018 at 19:37, on Zulip):

=)

nikomatsakis (Jun 29 2018 at 19:37, on Zulip):

#![feature(infer_struct_outlives)] or something like that -- mostly works, but for a cross-crate issue

lqd (Jun 29 2018 at 19:37, on Zulip):

oh nice

Jake Goulding (Jun 29 2018 at 19:38, on Zulip):

infer_struct_outlives — but this isn't on a struct, it's on a function... ?

nikomatsakis (Jun 29 2018 at 19:40, on Zulip):

I don't know if it would have gotten this case

nikomatsakis (Jun 29 2018 at 19:40, on Zulip):

don't remember the details

nikomatsakis (Jun 29 2018 at 19:40, on Zulip):

we already infer on functions anyway much of the time

simulacrum (Jun 29 2018 at 20:53, on Zulip):

@nikomatsakis Confirming that I have made no crater movement

simulacrum (Jun 30 2018 at 12:42, on Zulip):

@lqd Was enabling more benchmarks on perf.rlo -- the html5ever benchmark seems to exit with status code 1 with --Zborrowck=mir but no errors are reported, have you seen something like this before?

lqd (Jun 30 2018 at 12:46, on Zulip):

oh wow, no I don't think I have, however the broken MIR one could have a similar behaviour

lqd (Jun 30 2018 at 12:47, on Zulip):

even though it did have an error printed hum

lqd (Jun 30 2018 at 12:50, on Zulip):

I'll see if the old-ish build I have here has the same behaviour on html5ever

lqd (Jun 30 2018 at 13:02, on Zulip):

at the moment it's just killing my computer :) could it have been the OS killing the process, à la OOM-killer ?

simulacrum (Jun 30 2018 at 13:04, on Zulip):

hm, maybe

simulacrum (Jun 30 2018 at 13:05, on Zulip):

yeah I think that's what happened

lqd (Jun 30 2018 at 13:25, on Zulip):

we're going to have to look into that :)

lqd (Jun 30 2018 at 13:26, on Zulip):

after one or two minutes, rustc is using 10+ GB of memory (even on a month-old nightly)

Jake Goulding (Jun 30 2018 at 14:19, on Zulip):

using 10+ GB of memory

simulacrum (Jul 01 2018 at 16:52, on Zulip):

@lqd Have you done any work on the crater run? If not I can probably get a run going...

lqd (Jul 01 2018 at 16:54, on Zulip):

no I haven't done anything

simulacrum (Jul 01 2018 at 16:56, on Zulip):

Okay, I'll try to get something going

lqd (Jul 01 2018 at 16:56, on Zulip):

awesome, thank you :thumbs_up:

nikomatsakis (Jul 02 2018 at 15:37, on Zulip):

I didn't either, didn't quite have time

nikomatsakis (Jul 02 2018 at 15:57, on Zulip):

@simulacrum did you end up doing anything here?

simulacrum (Jul 02 2018 at 15:57, on Zulip):

Yes, crater run is started -- ETA around 3 days, I think

nikomatsakis (Jul 02 2018 at 15:58, on Zulip):

awesome!

nikomatsakis (Jul 02 2018 at 15:58, on Zulip):

can you link the PR in https://github.com/rust-lang/rust/issues/51827 ?

nikomatsakis (Jul 02 2018 at 15:58, on Zulip):

or whatever the basis for the crater run is

simulacrum (Jul 02 2018 at 15:58, on Zulip):

It's not any specific PR, just a commit on master

nikomatsakis (Jul 02 2018 at 15:59, on Zulip):

ok; link to commit? :)

simulacrum (Jul 02 2018 at 15:59, on Zulip):

Yeah, sure

nikomatsakis (Jul 02 2018 at 15:59, on Zulip):

ty

nikomatsakis (Jul 02 2018 at 15:59, on Zulip):

or maybe give the diff

nikomatsakis (Jul 02 2018 at 15:59, on Zulip):

I'd just like some record for posterity

simulacrum (Jul 02 2018 at 16:01, on Zulip):

(this is recorded in the crater spreadsheet, too); this run had 0 diff with rust-lang/rust because we ended up just using RUSTFLAGS instead of changing rustc itself to pass the borrowck arg

simulacrum (Jul 03 2018 at 16:20, on Zulip):

@nikomatsakis fyi, crater run failed with an out of memory error

simulacrum (Jul 03 2018 at 16:20, on Zulip):

looking into it now

nikomatsakis (Jul 03 2018 at 16:21, on Zulip):

oh, hmm, didn't we find something that consumed tons of memory? I should investigate those extreme results...

nikomatsakis (Jul 03 2018 at 16:21, on Zulip):

ah, yes, @lqd mentioned html5ever

simulacrum (Jul 03 2018 at 16:22, on Zulip):

anyway, I'm going to see if I can just restart it -- if that works, great, if not I might be able to generate a report based on what happened already

nikomatsakis (Jul 03 2018 at 16:23, on Zulip):

@simulacrum for tuple-stress, do we do --lib or --bin ? I can't tell

lqd (Jul 03 2018 at 16:23, on Zulip):

since html5ever is on crates.io it might even be the cause of the OOM

lqd (Jul 03 2018 at 16:24, on Zulip):

for tuple stress I don't use either lib or bin

simulacrum (Jul 03 2018 at 16:24, on Zulip):

@nikomatsakis There is only bin?

nikomatsakis (Jul 03 2018 at 16:24, on Zulip):

no, there is both

simulacrum (Jul 03 2018 at 16:24, on Zulip):

i.e. it's just one file?

nikomatsakis (Jul 03 2018 at 16:24, on Zulip):

at least in my directory

nikomatsakis (Jul 03 2018 at 16:24, on Zulip):

src/lib.rs exists

nikomatsakis (Jul 03 2018 at 16:24, on Zulip):

maybe I touched it

nikomatsakis (Jul 03 2018 at 16:24, on Zulip):

oh, I did :)

lqd (Jul 03 2018 at 16:25, on Zulip):

only main tho

lqd (Jul 03 2018 at 16:25, on Zulip):

right phew :)

nikomatsakis (Jul 03 2018 at 16:25, on Zulip):

/me remembers this dang test

lqd (Jul 03 2018 at 16:26, on Zulip):

can we do something special for constants/statics ? this thing is huge

nikomatsakis (Jul 03 2018 at 16:27, on Zulip):

yes, that's why it's in the test suite :)

nikomatsakis (Jul 03 2018 at 16:27, on Zulip):

well, we've thought about specializing these exact sorts of huge constants

nikomatsakis (Jul 03 2018 at 16:27, on Zulip):

but that's fairly orthogonal from NLL

nikomatsakis (Jul 03 2018 at 16:27, on Zulip):

mostly we wanted to do that for memory consumption

nikomatsakis (Jul 03 2018 at 16:27, on Zulip):

(in principle we can do it, now that miri is all integrated etc)

nikomatsakis (Jul 03 2018 at 16:28, on Zulip):

anyway, I am going to guess that the problem is the "for each block, iterate over each live variable" setup

nikomatsakis (Jul 03 2018 at 16:28, on Zulip):

in particular, we keep the live variables in a bit set

lqd (Jul 03 2018 at 16:28, on Zulip):

(the MIR is huge I meant)

nikomatsakis (Jul 03 2018 at 16:28, on Zulip):

which does not support O(n) iteration really

nikomatsakis (Jul 03 2018 at 16:28, on Zulip):

well, point is, iteration is proportional to the total number of locals

nikomatsakis (Jul 03 2018 at 16:28, on Zulip):

not the number live locals

nikomatsakis (Jul 03 2018 at 16:28, on Zulip):

but let me do my profile and see

lqd (Jul 03 2018 at 16:29, on Zulip):

oh, and there are 65k*2 locals at least

nikomatsakis (Jul 03 2018 at 16:30, on Zulip):

not sure if that hypothesis holds up

nikomatsakis (Jul 03 2018 at 16:30, on Zulip):

given the time spent in folding

lqd (Jul 03 2018 at 16:30, on Zulip):

I was wondering whether the SCC would impact here

nikomatsakis (Jul 03 2018 at 16:30, on Zulip):

not the PR I made I don't think

nikomatsakis (Jul 03 2018 at 16:30, on Zulip):

doesn't affect liveness

lqd (Jul 03 2018 at 16:31, on Zulip):

oh yeah it was about regions

simulacrum (Jul 03 2018 at 16:31, on Zulip):

SCC seems to make the bad cases worse, loosely

simulacrum (Jul 03 2018 at 16:32, on Zulip):

https://perf.rust-lang.org/nll-dashboard.html?commit=3fcccbf34a7af46ec467dc21c321db20994357ea&stat=max-rss vs https://perf.rust-lang.org/nll-dashboard.html?commit=&stat=max-rss

lqd (Jul 03 2018 at 16:33, on Zulip):

this url's "from" commit is not the one we should be using to compare to the previous merge ? http://perf.rust-lang.org/compare.html?start=4faaf7e3359fa78bad2e8c54011e94ce8ac078c6&end=3fcccbf34a7af46ec467dc21c321db20994357ea&stat=instructions%3Au

simulacrum (Jul 03 2018 at 16:34, on Zulip):

I loosely compared to master, but that might be wrong

nikomatsakis (Jul 03 2018 at 16:34, on Zulip):

I observed some similar things locally. Seems surprising. I'll have to investigate a bit more.

nikomatsakis (Jul 03 2018 at 16:35, on Zulip):

but also: the master status is looking pretty decent

simulacrum (Jul 03 2018 at 16:35, on Zulip):

yeah, memory is not bad

nikomatsakis (Jul 03 2018 at 16:35, on Zulip):

or, I see, that was just memory

nikomatsakis (Jul 03 2018 at 16:35, on Zulip):

:)

nikomatsakis (Jul 03 2018 at 16:36, on Zulip):

I was like "man when did we get to 105%!"

lqd (Jul 03 2018 at 16:36, on Zulip):

:tada: oh noes

lqd (Jul 03 2018 at 16:37, on Zulip):

SCCs have positive impacts on inflate

lqd (Jul 03 2018 at 16:37, on Zulip):

but it seems like it doesn't for most of the others

nikomatsakis (Jul 03 2018 at 16:38, on Zulip):

yeah. I can't really see why that would be, I'll have to do more profiling locally

simulacrum (Jul 03 2018 at 16:44, on Zulip):

crater run seems to have restarted successfully -- about 1/2 done loosely speaking

simulacrum (Jul 04 2018 at 12:01, on Zulip):

Crater run is done, currently uploading logs

simulacrum (Jul 04 2018 at 12:02, on Zulip):

However it looks like I messed up a little when hacking crater since both toolchains appear to have ran with NLL enabled

simulacrum (Jul 04 2018 at 12:03, on Zulip):

though that might be an artifact of reporting -- I'll try to determine that

simulacrum (Jul 04 2018 at 12:05, on Zulip):

Yeah okay crater isn't properly working with the same master commit it looks like

lqd (Jul 04 2018 at 12:10, on Zulip):

oh; can we salvage something out of the run or would we have to do another one ? (eg can we compare errors to another recent run)

simulacrum (Jul 04 2018 at 12:11, on Zulip):

I'm looking into it

simulacrum (Jul 04 2018 at 12:11, on Zulip):

Technically the run is quite good -- sort of

simulacrum (Jul 04 2018 at 12:12, on Zulip):

There's no regressions but many of the build failures are likely NLL related, though it's hard to tell. https://cargobomb-reports.s3.amazonaws.com/nll-1/index.html is the index page -- some logs won't be uploaded yet though

simulacrum (Jul 04 2018 at 12:13, on Zulip):

I think I can salvage the run successfully

simulacrum (Jul 04 2018 at 12:14, on Zulip):

But I'll wait for these logs to upload just in case

lqd (Jul 04 2018 at 12:14, on Zulip):

the regressions would only be noticed if crater could compare to the other commit right ?

simulacrum (Jul 04 2018 at 12:15, on Zulip):

Yeah -- I think I can convince crater that we actually ran against a different commit; should be relatively easy to point it at a past PR run (well, master from that run)

lqd (Jul 04 2018 at 12:15, on Zulip):

do you by any chance remember if 3432 failures is in the ballpark of recent master runs ?

simulacrum (Jul 04 2018 at 12:15, on Zulip):

I can look... I think.

lqd (Jul 04 2018 at 12:16, on Zulip):

if not no worries, tricking crater into thinking we used another commit will take of it

simulacrum (Jul 04 2018 at 12:16, on Zulip):

last PR run has ~2000 build-fails https://cargobomb-reports.s3.amazonaws.com/pr-51762/index.html

lqd (Jul 04 2018 at 12:17, on Zulip):

awesome thank you

simulacrum (Jul 04 2018 at 12:17, on Zulip):

okay all logs uploaded, I'll try to hack crater now

lqd (Jul 04 2018 at 12:19, on Zulip):

until then I'll be looking at all the crates starting with an "A" -- there are "NLL errors" from the couple I've looked at, so likely I don't need to wait for the crater hack :)

lqd (Jul 04 2018 at 12:26, on Zulip):

@simulacrum wait, was the run using 2 phase borrows ?

simulacrum (Jul 04 2018 at 12:26, on Zulip):

Unfortunately, no, because I only found out yesterday that's part of what we're shipping

lqd (Jul 04 2018 at 12:27, on Zulip):

:) ok good to know for everyone that some of the errors we can see in https://github.com/rust-lang/rust/issues/51372 esp cannot borrow "_" as mutable because it is also borrowed as immutable are normally no big deal

simulacrum (Jul 04 2018 at 12:31, on Zulip):

@lqd Should I kick off another run with two phase borrows immediately?

lqd (Jul 04 2018 at 12:33, on Zulip):

probably better to ask @nikomatsakis :) I myself guess it couldn't hurt

lqd (Jul 04 2018 at 12:34, on Zulip):

I don't know what errors not having them could make, and I only saw the "cannot borrow _" once in the 10-20 I just opened now (but ofc they may exist here, I just can't tell from a glance)

nikomatsakis (Jul 04 2018 at 12:43, on Zulip):

wait, was the run using 2 phase borrows ?

argh

nikomatsakis (Jul 04 2018 at 12:43, on Zulip):

@simulacrum I think you should probably kick off another run, yes.

nikomatsakis (Jul 04 2018 at 12:43, on Zulip):

that sucks :(

nikomatsakis (Jul 04 2018 at 12:43, on Zulip):

I've been wanting to change the sense of that flag forever

nikomatsakis (Jul 04 2018 at 12:43, on Zulip):

i.e., to make -Zno-two-phase-borrows =)

nikomatsakis (Jul 04 2018 at 12:43, on Zulip):

since we want them on by default

nikomatsakis (Jul 04 2018 at 12:43, on Zulip):

/me wishes he had done that now :)

simulacrum (Jul 04 2018 at 12:43, on Zulip):

I'm still slightly confused -- I thought two phase borrows didn't exist with old borrowck?

nikomatsakis (Jul 04 2018 at 12:43, on Zulip):

they don't

nikomatsakis (Jul 04 2018 at 12:44, on Zulip):

the flag has no effect if you are not using MIR borrowck

simulacrum (Jul 04 2018 at 12:44, on Zulip):

hm, so we shouldn't see regressions because of it?

nikomatsakis (Jul 04 2018 at 12:44, on Zulip):

well MIR borrowck fixes a lot of bugs

nikomatsakis (Jul 04 2018 at 12:44, on Zulip):

however, a non-trivial percentage of those bugs

simulacrum (Jul 04 2018 at 12:44, on Zulip):

because of two phase borrows, I mean?

nikomatsakis (Jul 04 2018 at 12:44, on Zulip):

are STILL not errors because of 2-phase borrows

simulacrum (Jul 04 2018 at 12:45, on Zulip):

ah, hm, okay, that makes sense

nikomatsakis (Jul 04 2018 at 12:45, on Zulip):

no I mean that there are cases that -- without 2-phase borrows -- ought to be rejected (but we fail to do so)

nikomatsakis (Jul 04 2018 at 12:45, on Zulip):

but with 2-phase borrows ought to be accepted

simulacrum (Jul 04 2018 at 12:45, on Zulip):

Right, so we'll have "false" regressions

nikomatsakis (Jul 04 2018 at 12:45, on Zulip):

yeah

nikomatsakis (Jul 04 2018 at 12:45, on Zulip):

still actually it's kind of interesting to have data on that :)

nikomatsakis (Jul 04 2018 at 12:45, on Zulip):

silver lining

nikomatsakis (Jul 04 2018 at 12:46, on Zulip):

I wonder if we can start by screening for ICEs?

nikomatsakis (Jul 04 2018 at 12:46, on Zulip):

is that something we can automate?

nikomatsakis (Jul 04 2018 at 12:46, on Zulip):

(what format are crater results stored in...?)

simulacrum (Jul 04 2018 at 12:46, on Zulip):

S3

simulacrum (Jul 04 2018 at 12:47, on Zulip):

I'll look into hunting for ICEs once I try to get regressions actually working and start the two phase borrows run

lqd (Jul 04 2018 at 13:00, on Zulip):

in the crater report, do we have the tested crate's commit in addition to its version, or if I needed to look at the code I should the get commit from ... somewhere on crates.io if the GH doesn't have it tagged ?

lqd (Jul 04 2018 at 13:01, on Zulip):

or to ask differently, how can I reproduce the errors crater sees ? :)

simulacrum (Jul 04 2018 at 13:01, on Zulip):

It's just the crates.io upload, so you can get the precise source code via https://gist.github.com/nikomatsakis/75e09ed314bea715e192ce226127d3c6

lqd (Jul 04 2018 at 13:02, on Zulip):

@simulacrum sweet, thanks a lot

simulacrum (Jul 04 2018 at 13:02, on Zulip):

Also, proper regression report is ready; should be able to extract and open index.html report.tar.gz

simulacrum (Jul 04 2018 at 13:02, on Zulip):

(file is tiny)

simulacrum (Jul 04 2018 at 13:03, on Zulip):

@nikomatsakis Okay, so to be sure, I need to pass -Zborrowck=mir -Ztwo-phase-borrows, and that's it, right?

nikomatsakis (Jul 04 2018 at 13:04, on Zulip):

let me double check but I believe that is true :)

nikomatsakis (Jul 04 2018 at 13:04, on Zulip):

correct

simulacrum (Jul 04 2018 at 13:04, on Zulip):

okay, then I'll kick off a run that should hopefully not need fixing post run this time :)

simulacrum (Jul 04 2018 at 13:06, on Zulip):

@nikomatsakis Hm, do you happen to have an existing try build you think we could use for this? (I just need it to not cause regressions of its own)

simulacrum (Jul 04 2018 at 13:07, on Zulip):

i.e. I need two commits that will be equivalent on master or on try

nikomatsakis (Jul 04 2018 at 13:07, on Zulip):

@simulacrum yeah, I suppose so

nikomatsakis (Jul 04 2018 at 13:07, on Zulip):

one of the recent PRs would probably do?

nikomatsakis (Jul 04 2018 at 13:07, on Zulip):

they are a bit outdated but...

simulacrum (Jul 04 2018 at 13:07, on Zulip):

I guess I can look ...

nikomatsakis (Jul 04 2018 at 13:08, on Zulip):

maybe the build from here https://github.com/rust-lang/rust/pull/52007 ?

nikomatsakis (Jul 04 2018 at 13:08, on Zulip):

https://github.com/rust-lang/rust/commit/00bcc44fb92c28465c727881355c3235a56a4045

nikomatsakis (Jul 04 2018 at 13:08, on Zulip):

parent is 1 day old: https://github.com/rust-lang/rust/commit/860d169474acabdc53b9a698f8ce02eba7e0daeb

simulacrum (Jul 04 2018 at 13:09, on Zulip):

I can take two master commits too, if that helps

nikomatsakis (Jul 04 2018 at 13:10, on Zulip):

well... I suppose any master commit is fine?

nikomatsakis (Jul 04 2018 at 13:10, on Zulip):

I'm not really clear on what you need :)

simulacrum (Jul 04 2018 at 13:10, on Zulip):

hm, I'll just use that PR I think

simulacrum (Jul 04 2018 at 13:10, on Zulip):

I need two separate SHAs but the same underlying code, basically -- well, as close as "same"

simulacrum (Jul 04 2018 at 13:10, on Zulip):

anyway, that PR's try build should be fine

nikomatsakis (Jul 04 2018 at 13:10, on Zulip):

yeah

nikomatsakis (Jul 04 2018 at 13:10, on Zulip):

should be ok

simulacrum (Jul 04 2018 at 13:11, on Zulip):

Time to look for ICEs

simulacrum (Jul 04 2018 at 13:54, on Zulip):

Okay, looks like 276 ICEs

simulacrum (Jul 04 2018 at 13:54, on Zulip):

(some might be duplicates)

simulacrum (Jul 04 2018 at 13:55, on Zulip):

@nikomatsakis What's the best way to report these? Somehow deduplicate and open issues?

nikomatsakis (Jul 04 2018 at 13:56, on Zulip):

good question :)

nikomatsakis (Jul 04 2018 at 13:57, on Zulip):

I guess trying to deduplicate somehow would be a start -- maybe just group them by the panic message?

nikomatsakis (Jul 04 2018 at 13:57, on Zulip):

and/or location of the panic

nikomatsakis (Jul 04 2018 at 13:57, on Zulip):

it seems like opening 276 ICEs is a bit overkill — maybe opening up one issue with a list...?

simulacrum (Jul 04 2018 at 13:57, on Zulip):

hm, okay, location should work

lqd (Jul 04 2018 at 13:57, on Zulip):

as I'm going through the As, some of their dependencies fail, so anything else depending on:
- num-bigint-0.1.44
- pulldown-cmark-0.1.2
- crossbeam-channel-0.1.3
will fail because of 2 phase borrows (bit of a PSA, and I'll try and doing this again if I stumble upon more)

nikomatsakis (Jul 04 2018 at 13:57, on Zulip):

depends how much we can group them I guess

simulacrum (Jul 04 2018 at 13:57, on Zulip):

I think many are duplicates

simulacrum (Jul 04 2018 at 14:33, on Zulip):

https://github.com/rust-lang/rust/issues/52054 -- @nikomatsakis

simulacrum (Jul 04 2018 at 14:33, on Zulip):

Looks like ~3 categories

lqd (Jul 04 2018 at 14:35, on Zulip):

@simulacrum are there false positives ? there is no error here ?

simulacrum (Jul 04 2018 at 14:36, on Zulip):

@lqd Less so false positive and more so probably an error in which directory the log was copied from -- you can see that log doesn't have MIR borrowck enabled

lqd (Jul 04 2018 at 14:38, on Zulip):

I also built that crate with NLL to make sure

simulacrum (Jul 04 2018 at 14:38, on Zulip):

hm, no idea then -- might be a false positive

lqd (Jul 04 2018 at 14:39, on Zulip):

yeah maybe, I'll just note it for later if we want/need to look more into it

lqd (Jul 04 2018 at 14:40, on Zulip):

oh it's some leftover artefacts that it builds here, the crate builds but one of its deps doesn't

lqd (Jul 04 2018 at 14:41, on Zulip):

so likely just wrong log indeed

nikomatsakis (Jul 04 2018 at 15:05, on Zulip):

@simulacrum interesting, thanks! <3

lqd (Jul 04 2018 at 16:16, on Zulip):

/me sees the light at the end of the tunnel of As

lqd (Jul 04 2018 at 16:16, on Zulip):

quick analysis of those <50 crates https://hackmd.io/s/H1x-c18cGm

lqd (Jul 04 2018 at 16:17, on Zulip):

only 1442 to go :D

simulacrum (Jul 04 2018 at 16:18, on Zulip):

@lqd It might be faster if you write a script or something to analyze them if the results are fairly methodical... I can give you all the regressed logs if you want

simulacrum (Jul 04 2018 at 16:18, on Zulip):

in a tarball, that is

lqd (Jul 04 2018 at 16:18, on Zulip):

interesting thought for sure

simulacrum (Jul 04 2018 at 16:19, on Zulip):

I think pietroalbini had some script too...

lqd (Jul 04 2018 at 16:19, on Zulip):

some of the logs have the problems we mentioned

lqd (Jul 04 2018 at 16:20, on Zulip):

yeah I bet it must be very helpful to the infra-team and thus would be helpful to me :D

simulacrum (Jul 04 2018 at 16:21, on Zulip):

this is the script I used for the ICE hunt: https://gist.github.com/Mark-Simulacrum/2a3cf12527d541a29f66316c55ec54f8

simulacrum (Jul 04 2018 at 16:21, on Zulip):

If it looks helpful I can help guide you through setup; it basically just downloads the logs and does "things" to them

lqd (Jul 04 2018 at 16:22, on Zulip):

even though the most mechanical task I had to do was filtering out the 2 phase borrow errors, as I have not really started trying to reproduce/minimize/understand the real errors I saw

lqd (Jul 04 2018 at 16:23, on Zulip):

oooh shiny

lqd (Jul 04 2018 at 16:25, on Zulip):

yeah looks helpful

simulacrum (Jul 04 2018 at 16:28, on Zulip):

@lqd Okay, so you'll want to download the inputs to it

lqd (Jul 04 2018 at 16:28, on Zulip):

the 2 json files, which I'm guessing are different from the ones in your report tarball ?

simulacrum (Jul 04 2018 at 16:29, on Zulip):
wget https://cargobomb-reports.s3.amazonaws.com/nll-1/results.json -O results-nll-1.json
wget http://cargobomb-reports.s3.amazonaws.com/pr-51762/results.json -O results-pr-51762.json
simulacrum (Jul 04 2018 at 16:29, on Zulip):

I think that should be all you need to run it... but if it breaks, I'll investigate

lqd (Jul 04 2018 at 16:31, on Zulip):

@simulacrum thanks a lot! I've downloaded the inputs, I'll ping you if I have a problem but agreed seems like all I'd need. I might not do it right now :)

lqd (Jul 04 2018 at 17:33, on Zulip):

hum there are errors I saw in these crates' dependencies, but they themselves don't all appear as regressions

lqd (Jul 04 2018 at 17:33, on Zulip):

I think I'll continue by analyzing/minimizing more of the errors in the "A" crates I did earlier, before looking at more crates from the crater report (this can serve as a reminder for others, if they look at this, start from the crates beginning with a B -- even though most of the errors are probably in their dependencies unfortunately)

nikomatsakis (Jul 04 2018 at 17:34, on Zulip):

I'm gonna spend an hour looking at those ICEs

nikomatsakis (Jul 04 2018 at 17:37, on Zulip):

broken MIR in DefId(0/0:2416 ~ syntex_syntax[45c6]::ext[0]::

that is so curious

nikomatsakis (Jul 04 2018 at 17:37, on Zulip):

why on earth are we borrow checking syntex

nikomatsakis (Jul 04 2018 at 17:37, on Zulip):

oh, I know what that is

nikomatsakis (Jul 04 2018 at 17:37, on Zulip):

it's a known bug

nikomatsakis (Jul 04 2018 at 17:37, on Zulip):

assigned to me, in fact :)

nikomatsakis (Jul 04 2018 at 17:53, on Zulip):

ok, categorized the ICEs and linked them to open issues. One was not previously known.

lqd (Jul 04 2018 at 18:53, on Zulip):

@nikomatsakis this is a legit error right ? it's from url 1.7.0 and prevents building 10-15 out of the 50 crates I've looked at

nikomatsakis (Jul 04 2018 at 18:55, on Zulip):

hmm

nikomatsakis (Jul 04 2018 at 18:55, on Zulip):

what do you mean by "legit"?

lqd (Jul 04 2018 at 18:55, on Zulip):

a valid error

nikomatsakis (Jul 04 2018 at 18:55, on Zulip):

like, the code should not compile?

lqd (Jul 04 2018 at 18:55, on Zulip):

I think yeah ?

nikomatsakis (Jul 04 2018 at 18:56, on Zulip):

it does seem correct

nikomatsakis (Jul 04 2018 at 18:56, on Zulip):

thanks to the drop, yes

nikomatsakis (Jul 04 2018 at 18:56, on Zulip):

took me a second to see that

nikomatsakis (Jul 04 2018 at 18:56, on Zulip):

I feel like there is a patched version of url that solves this?

nikomatsakis (Jul 04 2018 at 18:56, on Zulip):

but I am wrong :)

lqd (Jul 04 2018 at 18:56, on Zulip):

really ? oh lol

nikomatsakis (Jul 04 2018 at 18:56, on Zulip):

no, I am mistaken, 1.7.0 seems to be the newest

nikomatsakis (Jul 04 2018 at 18:57, on Zulip):

there was some other crate with (iirc) a similar thing going on

nikomatsakis (Jul 04 2018 at 18:57, on Zulip):

the error is pretty bad :(

nikomatsakis (Jul 04 2018 at 18:57, on Zulip):

we should file a diagnostic bug on that

lqd (Jul 04 2018 at 18:57, on Zulip):

it's still like this in rust-url master btw

lqd (Jul 04 2018 at 18:58, on Zulip):

it is a bit obscure :)

nikomatsakis (Jul 04 2018 at 18:58, on Zulip):

it should say something about Drop, clearly

lqd (Jul 04 2018 at 18:58, on Zulip):

I'll file an issue

nikomatsakis (Jul 04 2018 at 18:58, on Zulip):

the easiest fix would probably be to make it an Option<&'a mut> in the crate

nikomatsakis (Jul 04 2018 at 18:58, on Zulip):

so you can take() it

nikomatsakis (Jul 04 2018 at 18:59, on Zulip):

(presuming of course that the Drop is needed)

lqd (Jul 04 2018 at 18:59, on Zulip):

I'll tell that to Simon :p

nikomatsakis (Jul 04 2018 at 19:00, on Zulip):

if you do:

impl<'a> S<'a> {
    fn finish(self) -> &'a mut String {
        let p = self.url;
        p
    }
}

you get a better error...but anyway.

lqd (Jul 04 2018 at 19:01, on Zulip):

at least it mentions Drop :thumbs_up:

nikomatsakis (Jul 04 2018 at 19:01, on Zulip):

the reason is that just returning self.url implicitly reborrows

nikomatsakis (Jul 04 2018 at 19:01, on Zulip):

so you get &mut self.url

nikomatsakis (Jul 04 2018 at 19:01, on Zulip):

I'm not sure why this leads to that precise error about temporaries

nikomatsakis (Jul 04 2018 at 19:01, on Zulip):

seems like a bug

lqd (Jul 04 2018 at 19:02, on Zulip):

oh but maybe we're already tracking this diagnostics issue, I'll need to check

nikomatsakis (Jul 04 2018 at 19:02, on Zulip):

oh, I guess it doesn't talk about temporaries

nikomatsakis (Jul 04 2018 at 19:02, on Zulip):

there may well be an issue, yes.

lqd (Jul 04 2018 at 19:09, on Zulip):

the title of https://github.com/rust-lang/rust/issues/46634 was promising

nikomatsakis (Jul 04 2018 at 19:10, on Zulip):

yeah, I thought of that one, but it seems to be distinct issue

lqd (Jul 04 2018 at 19:14, on Zulip):

yeah, and I can't find a specific issue so I'll file it as "Implicit reborrow hides error Cannot move out of Struct with destructor" if that sounds good

nikomatsakis (Jul 04 2018 at 19:17, on Zulip):

sure

lqd (Jul 04 2018 at 19:34, on Zulip):

(filed as https://github.com/rust-lang/rust/issues/52059) now to talk to Simon about servo's crate

lqd (Jul 04 2018 at 20:53, on Zulip):

fixed_by_nll += 1; https://github.com/servo/rust-url/pull/454

lqd (Jul 05 2018 at 13:13, on Zulip):

this feels like a workaround for the lack of GATs but the error is a bit obscure again https://play.rust-lang.org/?gist=ce6ac6b0e7db4be75fc3bcac57c6ba9c&version=nightly&mode=debug&edition=2015 (extracted from specs 0.10) :confused:

lqd (Jul 05 2018 at 13:35, on Zulip):

also here's an alternative to niko's cargo-curl which also accepts the crate-version format that the crater report uses: https://gist.github.com/lqd/4a8af10389d10840d90655c109df5eac

nikomatsakis (Jul 05 2018 at 13:38, on Zulip):

hmm, that seems like a bug to me @lqd

nikomatsakis (Jul 05 2018 at 13:38, on Zulip):

specifically, I think the implied bounds computation may be messing up somehow

nikomatsakis (Jul 05 2018 at 13:39, on Zulip):

this variant has a clearer error message

nikomatsakis (Jul 05 2018 at 13:40, on Zulip):

but I think we should infer a relationship between 'a and 'b from the argument type

lqd (Jul 05 2018 at 13:40, on Zulip):

I forgot to mention there was something related to the bounds, that is specs does not have the T: 'a, but didn't show an error while this minimized version actually did

lqd (Jul 05 2018 at 13:41, on Zulip):

yes indeed a bit clearer, I also tried this

lqd (Jul 05 2018 at 13:41, on Zulip):

the empty named region was cute though

lqd (Jul 05 2018 at 13:42, on Zulip):

(so there might still be something else to extract from specs, more scaffolding causing some implied bounds to be missed)

lqd (Jul 05 2018 at 13:44, on Zulip):

this crater run is :thumbs_up:

nikomatsakis (Jul 05 2018 at 13:46, on Zulip):

this crater run is :thumbs_up:

in what sense? like, finding bugs?

nikomatsakis (Jul 05 2018 at 13:46, on Zulip):

are you filing bugs or taking notes?

nikomatsakis (Jul 05 2018 at 13:46, on Zulip):

ps thank you one million times :)

nikomatsakis (Jul 05 2018 at 13:46, on Zulip):

(also, I think we should think about trying to advertise this as a wider effort? maybe?)

lqd (Jul 05 2018 at 13:47, on Zulip):

finding bugs and issues both in borrowck and existing crates

lqd (Jul 05 2018 at 13:47, on Zulip):

I'm doing both, notes from yesterday are here https://hackmd.io/s/H1x-c18cGm, and I'll file this new issue soon

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

I just did a first pass on the "As" yesterday, and now sifting through the interesting errors

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

(and seeing that in those 50 crates, the dependencies were the source of errors, it made me worried that we might duplicate triaging efforts if we shard work by the first letter, and that realization also made me hope crater uses sccache or similar, otherwise it'll be compiling the same dependencies a lot ;)

nikomatsakis (Jul 05 2018 at 13:54, on Zulip):

I think it compiles things into a shared workspace, but I'm not sure. @simulacrum might know.

simulacrum (Jul 05 2018 at 13:55, on Zulip):

It does, but the order of compilation isn't the best I believe

simulacrum (Jul 05 2018 at 13:55, on Zulip):

We probably are also making Cargo somewhat unhappy since we put ~20,000 crates into the same target dir

lqd (Jul 05 2018 at 13:55, on Zulip):

sweet! (it was just a random thought anyway)

lqd (Jul 05 2018 at 13:56, on Zulip):

a good cargo stress test

simulacrum (Jul 05 2018 at 13:56, on Zulip):

@lqd I think we can take the crates and generate a graph from it if you want, should be able to help with the duplicated work problem -- at least for crates.io crates that's easy

Jake Goulding (Jul 05 2018 at 13:56, on Zulip):

@simulacrum roughly, how many GB is that target dir?

simulacrum (Jul 05 2018 at 13:57, on Zulip):

hm, the docs suggest that after a full run it's ~600 GB but I'm looking how the nll run is doing now

lqd (Jul 05 2018 at 13:58, on Zulip):

@simulacrum could be interesting to graph out the regressions yeah; however: the errors I'm seeing only appear as regressions in dependent crates, they themselves don't seem to be in the report ? it could be because we massaged the report, or does that sound familiar ?

simulacrum (Jul 05 2018 at 13:58, on Zulip):

@lqd It's possible we're skipping the dependent crates or something like that -- you can try and see if the dependent crate exists at all (e.g., enable all the tabs at the top of index.html)

simulacrum (Jul 05 2018 at 13:59, on Zulip):

@Jake Goulding 35 GB at ~8000 crates tested out of the 20,000 we look at

simulacrum (Jul 05 2018 at 13:59, on Zulip):

(this is for a check run, though)

lqd (Jul 05 2018 at 14:00, on Zulip):

@simulacrum for example url 1.7.0 is indeed in build-fails but not in regressions

lqd (Jul 05 2018 at 14:01, on Zulip):

oh but it could be because of warnings

simulacrum (Jul 05 2018 at 14:01, on Zulip):

The report shows that in both the original run and later run it failed to build

lqd (Jul 05 2018 at 14:01, on Zulip):

yeah and one of those 2 denies lints

lqd (Jul 05 2018 at 14:02, on Zulip):

right ?

simulacrum (Jul 05 2018 at 14:02, on Zulip):

Hm, no we cap lints at warn; the errors look like missing functions which feels odd...

lqd (Jul 05 2018 at 14:02, on Zulip):

in the "new toolchain" but not the old one

simulacrum (Jul 05 2018 at 14:03, on Zulip):

url-1.7.0 is build fail in the old one too, https://cargobomb-reports.s3.amazonaws.com/pr-51762/01cc982e936120acb0424e41de14e42ba2d88c6f/reg/url-1.7.0

simulacrum (Jul 05 2018 at 14:03, on Zulip):

er, https://cargobomb-reports.s3.amazonaws.com/pr-51762/01cc982e936120acb0424e41de14e42ba2d88c6f/reg/url-1.7.0/log.txt

lqd (Jul 05 2018 at 14:04, on Zulip):

what I'm saying is, url-1.7.0 fails in the non-nll toolchain because of --cap-lints=forbid, while it fails in the NLL toolchain because of its lifetime bug

lqd (Jul 05 2018 at 14:04, on Zulip):

is that correct ?

simulacrum (Jul 05 2018 at 14:04, on Zulip):

no

lqd (Jul 05 2018 at 14:04, on Zulip):

good :D

simulacrum (Jul 05 2018 at 14:04, on Zulip):

the log I linked is from the non-nll toolchain and has the assert function not found errors

simulacrum (Jul 05 2018 at 14:05, on Zulip):

e.g. Jun 27 14:06:10.335 INFO kablam! error[E0425]: cannot find function assert_test_result in module self::test`

lqd (Jul 05 2018 at 14:05, on Zulip):

how are those found only enabling NLL on the same master commit

simulacrum (Jul 05 2018 at 14:05, on Zulip):

it's possible the crate's tests are just failing in general

lqd (Jul 05 2018 at 14:05, on Zulip):

spuriously maybe ?

simulacrum (Jul 05 2018 at 14:06, on Zulip):

This does not look like a spurious failure

lqd (Jul 05 2018 at 14:06, on Zulip):

this seems strange

simulacrum (Jul 05 2018 at 14:08, on Zulip):

The assert_test_result function comes from libtest, I think

lqd (Jul 05 2018 at 14:08, on Zulip):

I'll look at this crate again because I can't repro these errors with NLL disabled

simulacrum (Jul 05 2018 at 14:09, on Zulip):

I can reproduce locally too

simulacrum (Jul 05 2018 at 14:09, on Zulip):

cargo check --all-targets in the tarball reproduces

lqd (Jul 05 2018 at 14:09, on Zulip):

so it is the cfgs that I'm missing while reproducing

lqd (Jul 05 2018 at 14:12, on Zulip):

cargo test indeed fails to compile like expected

lqd (Jul 05 2018 at 14:13, on Zulip):

thankfully some other crate depended on its default target or I would have missed it

simulacrum (Jul 05 2018 at 14:17, on Zulip):

@lqd You generally want to do --all-targets.

simulacrum (Jul 05 2018 at 14:17, on Zulip):

Anyway, did you want that crate graph so you can eliminate children?

lqd (Jul 05 2018 at 14:18, on Zulip):

if you have one available sure :)

lqd (Jul 05 2018 at 14:18, on Zulip):

(but don't go out of your way to build it just for me)

simulacrum (Jul 05 2018 at 14:19, on Zulip):

I think I have some code that does it anyway, just need to pull it in for crater's regressions

simulacrum (Jul 05 2018 at 15:27, on Zulip):

@lqd https://github.com/Mark-Simulacrum/crater-report-merge -- https://github.com/Mark-Simulacrum/crater-report-merge/blob/master/src/main.rs#L222-L225 -- not sure how to format it better, right now it just dumps a wall of text at you

simulacrum (Jul 05 2018 at 15:28, on Zulip):

which for URL is quite long: dependents on url 1.7.0: 11498 crates, 11237 versions

lqd (Jul 05 2018 at 15:30, on Zulip):

@simulacrum oh thanks a lot for this :)

lqd (Jul 05 2018 at 15:33, on Zulip):

(if anyone was interested about url's test story: https://github.com/servo/rust-url/pull/439)

lqd (Jul 05 2018 at 15:40, on Zulip):

filed https://github.com/rust-lang/rust/issues/52078 for the specs issue, I don't know what tags I should add so just A-NLL for now

simulacrum (Jul 05 2018 at 17:00, on Zulip):

@nikomatsakis fyi, until we have a fix for the memory issue I think it will be somewhat painful to run crater

nikomatsakis (Jul 05 2018 at 17:02, on Zulip):

do you mean the one that effects html5ever?

simulacrum (Jul 05 2018 at 17:08, on Zulip):

Yeah, there's a few other crates it affects (well, perhaps one)

nikomatsakis (Jul 05 2018 at 17:17, on Zulip):

do we have any idea what they are?

nikomatsakis (Jul 05 2018 at 17:17, on Zulip):

I should probably prioritize that

nikomatsakis (Jul 05 2018 at 17:17, on Zulip):

I have at least one idea that should ameliorate it

nikomatsakis (Jul 05 2018 at 17:17, on Zulip):

I was hoping to mentor it out but maybe should just try to do it

nikomatsakis (Jul 05 2018 at 17:18, on Zulip):

I'd probably want to build on https://github.com/rust-lang/rust/pull/51987

simulacrum (Jul 05 2018 at 17:27, on Zulip):

marksman_escape

simulacrum (Jul 05 2018 at 17:27, on Zulip):

14GB usage

nikomatsakis (Jul 05 2018 at 18:02, on Zulip):

I was hoping to mentor it out but maybe should just try to do it

so @David Wood — sorry I got distracted — actually maybe you want to work on trying to address https://github.com/rust-lang/rust/issues/52028 ?

nikomatsakis (Jul 05 2018 at 18:03, on Zulip):

that said, we should at least discuss the other thing

nikomatsakis (Jul 05 2018 at 18:03, on Zulip):

let me go leave a few comments there, maybe file an issue

davidtwco (Jul 05 2018 at 18:03, on Zulip):

Is @nnethercote not working on that?

nikomatsakis (Jul 05 2018 at 18:04, on Zulip):

I don't think so...they were looking at unroll_place...

nikomatsakis (Jul 05 2018 at 18:05, on Zulip):

I was thinking @David Wood specifically of this idea

nikomatsakis (Jul 05 2018 at 18:05, on Zulip):

let's open a thread I guess

lqd (Jul 05 2018 at 20:19, on Zulip):

I'm working on triaging the run with a script — thanks to the awesome @simulacrum — but in the meantime, here's another crate which takes a lot of RAM in my tests: isolang-0.2.1(again, 10G in a minute or so)

lqd (Jul 05 2018 at 22:22, on Zulip):

alrighty, progress update, I'm not 100% sure about all this but here goes — 1) I first separated in the report, the regressions with local errors from the ones whose errors are only in dependencies, here 2) out of those, I cratesio-curleded my way to checking each of them for 2-phase-borrows (2PB in my linked notes) false positives (I'm not typing ariel's 2ΦB ever again :p), 3) I then did a first quick pass on those to clear the ones which didn't look like NLL-related (some liballoc shenanigans, etc) which you can see here 4) there are some crates with errors marked interesting to investigate later which I will do ... later ... before extracting from the report the regressions located in dependencies only

lqd (Jul 05 2018 at 22:31, on Zulip):

(we could make NLLbot with this: it downloads all crates and if one doesn't build with NLLs it opens an issue in its repo :)

simulacrum (Jul 06 2018 at 01:13, on Zulip):

@lqd https://github.com/pietroalbini/crater-tree is probably much better than my hacky tool

simulacrum (Jul 06 2018 at 03:34, on Zulip):

Crater run should be done tomorrow morning I think

lqd (Jul 06 2018 at 05:45, on Zulip):

oh thanks, yours worked fine

lqd (Jul 06 2018 at 09:06, on Zulip):

ok I'm see a 2PB ICE when trying to filter them from the report; it's likely we'll see it again in the new crater run

lqd (Jul 06 2018 at 09:15, on Zulip):

(it's the one @David Wood fixed but the run won't have this PR ofc)

lqd (Jul 06 2018 at 11:48, on Zulip):

am I misunderstanding this piece of code (extracted from the fscmp crater failures) ? is it normal that the second line compiles and the first doesn't ?

nikomatsakis (Jul 06 2018 at 12:05, on Zulip):

seems wacky to me:)

lqd (Jul 06 2018 at 12:17, on Zulip):

same :)

lqd (Jul 06 2018 at 13:25, on Zulip):

the 1s line failing looks like a "fixed by NLL" case though (in the real crate it's not as useless as the version I minimized of course)

lqd (Jul 06 2018 at 14:12, on Zulip):

err, looking at it more, I'm confused and the 1st line should probably compile ?!

nikomatsakis (Jul 06 2018 at 14:14, on Zulip):

I think it should compile

nikomatsakis (Jul 06 2018 at 14:14, on Zulip):

that's what I meant before :)

lqd (Jul 06 2018 at 14:15, on Zulip):

right I confused myself :/

lqd (Jul 06 2018 at 15:05, on Zulip):

(as I couldn't find a similar issue, filed as https://github.com/rust-lang/rust/issues/52111)

nikomatsakis (Jul 06 2018 at 15:19, on Zulip):

I'm going to throw that on the Edition Preview 2 milestone for now

nikomatsakis (Jul 06 2018 at 15:20, on Zulip):

maybe we need an "needs diagnosis" label...

lqd (Jul 06 2018 at 15:24, on Zulip):

as in "we need to diagnose these issues" ?

nikomatsakis (Jul 06 2018 at 15:24, on Zulip):

yes e.g. there are a few ICEs whose cause I do not know

nikomatsakis (Jul 06 2018 at 15:24, on Zulip):

similarly I have no idea what is leading to that particular bug

nikomatsakis (Jul 06 2018 at 15:24, on Zulip):

I feel like it's important to "classify" the error in order to assess how hard it will be to fix :)

lqd (Jul 06 2018 at 15:25, on Zulip):

very true, as of now we don't have a whole lot of information on a small number of issues

lqd (Jul 06 2018 at 15:30, on Zulip):

and the results of the new crater run with 2PB should be ready soon, so maybe new issues we'll need to diagnose as well

Jake Goulding (Jul 06 2018 at 15:46, on Zulip):

I keep thinking "lead" and "peanut butter" for "2PB"

nikomatsakis (Jul 06 2018 at 15:46, on Zulip):

I definitely think of PBJ

Jake Goulding (Jul 06 2018 at 15:46, on Zulip):

makes it more amusing

lqd (Jul 06 2018 at 15:47, on Zulip):

btw is there a better/official acronym ?

simulacrum (Jul 06 2018 at 16:28, on Zulip):

@lqd New crater run done, 2342 regressions (this one will 2PB enabled) https://cargobomb-reports.s3.amazonaws.com/nll-3/index.html -- not sure why the number is higher than before this time around :/

Jake Goulding (Jul 06 2018 at 16:29, on Zulip):

That's progress of a sort ;-)

lqd (Jul 06 2018 at 16:29, on Zulip):

@simulacrum thank you

lqd (Jul 06 2018 at 16:30, on Zulip):

indeed an interesting question

simulacrum (Jul 06 2018 at 16:30, on Zulip):

It might be that we previously didn't detect all regressions or something like that

lqd (Jul 06 2018 at 16:31, on Zulip):

could be

Jake Goulding (Jul 06 2018 at 16:31, on Zulip):

@simulacrum how can I determine why a crate is skipped?

simulacrum (Jul 06 2018 at 16:31, on Zulip):

@Jake Goulding https://github.com/rust-lang-nursery/crater/blob/master/config.toml -- search for the name

lqd (Jul 06 2018 at 16:31, on Zulip):

master wasn't especially different from the previous run ?

lqd (Jul 06 2018 at 16:32, on Zulip):

ah skipped is the blacklist maybe ?

simulacrum (Jul 06 2018 at 16:32, on Zulip):

@lqd Hm, not sure -- you can compare the commits, it might have been more recent

Jake Goulding (Jul 06 2018 at 16:32, on Zulip):

@simulacrum twox-hash = { skip = true } #automatic — so... what does "automatic" mean? :innocent:

simulacrum (Jul 06 2018 at 16:32, on Zulip):

previous run was somewhat faulty since I think there may have been race conditions

lqd (Jul 06 2018 at 16:32, on Zulip):

and the OOMs were probably not helping

simulacrum (Jul 06 2018 at 16:33, on Zulip):

@Jake Goulding Not sure. Maybe https://github.com/rust-lang-nursery/crater/blob/master/find-bad-crates.py?

Jake Goulding (Jul 06 2018 at 16:34, on Zulip):

I'd like to make sure all of my crates can be tested, so I'm trying to figure out if I screwed something up

simulacrum (Jul 06 2018 at 16:35, on Zulip):

@Jake Goulding It looks like it was added after it didn't pass 8 runs in https://github.com/rust-lang-nursery/crater/commit/cf95b1063aacc71ddd6af7386b658240720f5013

You can fairly easily run crater yourself on just that crate by adding it to the demo-crates list I think, I'd check if that works

lqd (Jul 06 2018 at 16:35, on Zulip):

could I ask what those are at the bottom of the regressions, they look like weird versions ?

simulacrum (Jul 06 2018 at 16:35, on Zulip):

Generally it can be that crates with odd build scripts (e.g., writing files) are not quite right

simulacrum (Jul 06 2018 at 16:35, on Zulip):

er, don't build

simulacrum (Jul 06 2018 at 16:35, on Zulip):

@lqd You mean "ubnt-intrepid.hyper-web-server.ac6165a6a2f30cb5cf6dc"?

simulacrum (Jul 06 2018 at 16:36, on Zulip):

That's a GitHub repo, ubnt-intrepid/hyper-web-server at that commit hash

lqd (Jul 06 2018 at 16:36, on Zulip):

yeah, alexcrichton.cargo-edit-locally.28605545a3ddf9b164d5 etc

lqd (Jul 06 2018 at 16:36, on Zulip):

searching crates.io didn't seem to know about them

simulacrum (Jul 06 2018 at 16:37, on Zulip):

GH repos

lqd (Jul 06 2018 at 16:37, on Zulip):

so coming from crates depending on those specific git commits ?

simulacrum (Jul 06 2018 at 16:38, on Zulip):

no, we scan github too, and find rust projects there

lqd (Jul 06 2018 at 16:38, on Zulip):

oh ok I didn't know that

Jake Goulding (Jul 06 2018 at 16:41, on Zulip):

Yeah, that's pretty wild.

davidtwco (Jul 06 2018 at 16:42, on Zulip):

We need a mind blown emoji.

Pietro Albini (Jul 06 2018 at 17:14, on Zulip):

the github repos list is pretty old though...

Pietro Albini (Jul 06 2018 at 17:15, on Zulip):

and good luck triaging 2300+ regressions ;-)

lqd (Jul 06 2018 at 17:15, on Zulip):

thanks :scream:

simulacrum (Jul 06 2018 at 17:15, on Zulip):

I made the perhaps not-so-good decision of running crater-tree locally and am now waiting for good old fake binary cargos to be created

simulacrum (Jul 06 2018 at 17:16, on Zulip):

I might go back to my approach which generates the graph and update it to work better

Pietro Albini (Jul 06 2018 at 17:16, on Zulip):

LOL, crater-tree is everything but optimized

simulacrum (Jul 06 2018 at 17:17, on Zulip):

@lqd https://gist.github.com/Mark-Simulacrum/6a8c998ba184dcf0849863864aa3ac8b

lqd (Jul 06 2018 at 17:17, on Zulip):

569 ICEs https://gist.github.com/lqd/e9ef65a3f5eacdafdc3f0e16bf6bbbd2

lqd (Jul 06 2018 at 17:18, on Zulip):

@simulacrum nice, thanks! :)

simulacrum (Jul 06 2018 at 17:19, on Zulip):

Looks like ~445 root regressions in the crates.io part of the regressions

lqd (Jul 06 2018 at 17:19, on Zulip):

is it sorted in a special way ?

simulacrum (Jul 06 2018 at 17:19, on Zulip):

no idea, this is crater-tree

Pietro Albini (Jul 06 2018 at 17:19, on Zulip):

nope, it's not sorted

lqd (Jul 06 2018 at 17:19, on Zulip):

:smile:

simulacrum (Jul 06 2018 at 17:19, on Zulip):

grep -v '^|' for the roots

simulacrum (Jul 06 2018 at 17:19, on Zulip):

(I think)

lqd (Jul 06 2018 at 17:20, on Zulip):

oh this is crater-tree returning regressions ?

Pietro Albini (Jul 06 2018 at 17:23, on Zulip):

yeah, that should be the crater-tree output

lqd (Jul 06 2018 at 17:26, on Zulip):

assuming all the plugin_registrar ICEs are the same, I don't _think_ there are new ones

lqd (Jul 06 2018 at 17:33, on Zulip):

@Pietro Albini could you explain crater-tree's output a bit please ? (I have virtually no experience with crater or its results) -- here's how I understand it, it is the dependency tree of the crates which regressed, in order to differentiate the regressions which happened because of a crate's code vs the ones which happened because of an upstream dependency ?

Pietro Albini (Jul 06 2018 at 17:34, on Zulip):

@lqd exactly

lqd (Jul 06 2018 at 17:35, on Zulip):

awesome thank you :thumbs_up:

lqd (Jul 06 2018 at 18:39, on Zulip):

@Pietro Albini the report mentions 2342 regressions, and the crater-tree output simulacrum posted has 1995, is that expected ?

Pietro Albini (Jul 06 2018 at 18:40, on Zulip):

crater-tree doesn't support GitHub repos, so there are also 347 broken repos

nikomatsakis (Jul 06 2018 at 19:21, on Zulip):

hey all -- can I get a kind of update here on the status?

nikomatsakis (Jul 06 2018 at 19:21, on Zulip):

seems like we got some results

nikomatsakis (Jul 06 2018 at 19:21, on Zulip):

did we break down the ICEs like we did before?

lqd (Jul 06 2018 at 19:25, on Zulip):

I looked at them, they look the same, and tagged the "unexpected panics" with the known issue they were

lqd (Jul 06 2018 at 19:25, on Zulip):

one of which has a fix in queue

lqd (Jul 06 2018 at 19:26, on Zulip):

but have not categorized the same way yet

lqd (Jul 06 2018 at 20:05, on Zulip):

@nikomatsakis https://gist.github.com/lqd/e9ef65a3f5eacdafdc3f0e16bf6bbbd2

nikomatsakis (Jul 06 2018 at 20:05, on Zulip):

wow, thanks

lqd (Jul 06 2018 at 20:06, on Zulip):

double check it of course ;)

nikomatsakis (Jul 06 2018 at 20:10, on Zulip):

looks about right

nikomatsakis (Jul 06 2018 at 20:10, on Zulip):

no new ICEs, seems good

lqd (Jul 06 2018 at 20:11, on Zulip):

yup that was a bit of a relief earlier

lqd (Jul 06 2018 at 20:12, on Zulip):

I'm currently trying to regenerate a root regression report (without GH repos for now) with links to logs and all that

simulacrum (Jul 06 2018 at 20:27, on Zulip):

@lqd Let me know if I can help

lqd (Jul 06 2018 at 20:28, on Zulip):

I should be good, you have already helped a lot :)

lqd (Jul 06 2018 at 20:54, on Zulip):

@simulacrum on second thought :) it's not a huge deal, but the crate versions are missing from the crater-tree output, so there are duplicates I cannot distinguish

simulacrum (Jul 06 2018 at 20:55, on Zulip):

Hm, that might be easy to add. I don't really want to rerun crater tree though -- I think I'll finish up my version :)

lqd (Jul 06 2018 at 20:55, on Zulip):

:D

Pietro Albini (Jul 06 2018 at 20:57, on Zulip):

what do you need the crate version though? if a crate failed multiple times you can just collapse them into one failure

lqd (Jul 06 2018 at 20:57, on Zulip):

I'll continue on my side, outputting the logs of all the versions of a root regression

lqd (Jul 06 2018 at 20:58, on Zulip):

@Pietro Albini I would have liked to distinguish the errors between versions, but as I said it's not a huge deal

lqd (Jul 06 2018 at 20:58, on Zulip):

I will do so manually after outputting all versions regardless :)

lqd (Jul 06 2018 at 21:01, on Zulip):

so ofc @simulacrum don't worry too much about it vs other things to do, enjoying life and so on

simulacrum (Jul 06 2018 at 21:01, on Zulip):

Somehow loading and working with Crater results is quite fun

Pietro Albini (Jul 06 2018 at 21:03, on Zulip):

@lqd also, we really need a tool faster than crater-tree for this kind of stuff
another run just finished, and that one has 7k regressions

lqd (Jul 06 2018 at 21:04, on Zulip):

@Pietro Albini to be precise I wanted to also dinstiguish root regressions from regressions, so "fusing" part of the crater-tree output to the crater report. without versions I just know that a version that regressed has a crate in root regressions

Pietro Albini (Jul 06 2018 at 21:04, on Zulip):

and sorry, NLL is not the thing that broke the highest number of crates :P

lqd (Jul 06 2018 at 21:05, on Zulip):

oh wow, yeah the code simulacrum showed me was quite fast (can I call them @simulacrum 's simulacra ?)

simulacrum (Jul 06 2018 at 21:05, on Zulip):

crater-tree is fairly slow IMO -- it does a lot of extra work I think

lqd (Jul 06 2018 at 21:06, on Zulip):

and you're most of the way there already

lqd (Jul 06 2018 at 21:07, on Zulip):

should soon become a cool new crater tool

Pietro Albini (Jul 06 2018 at 21:09, on Zulip):

yeah, with crater-tree I just chose the faster way to implement it, and that worked fine until today :P

lqd (Jul 06 2018 at 21:10, on Zulip):

for sure, I'm grateful to already have the root regressions it gave me @Pietro Albini :thumbs_up:

Pietro Albini (Jul 06 2018 at 21:10, on Zulip):

by the way, we should have proper support for NLL runs next time, the PR for it is up

lqd (Jul 06 2018 at 21:11, on Zulip):

7k regressions is fun though :)

lqd (Jul 06 2018 at 21:17, on Zulip):

@Pietro Albini quick question, an example 'ammonia' is a root regression: https://gist.github.com/Mark-Simulacrum/6a8c998ba184dcf0849863864aa3ac8b#file-gistfile1-txt-L1713 or am I misunderstanding the output ? but here's its log: https://cargobomb-reports.s3.amazonaws.com/nll-3/00bcc44fb92c28465c727881355c3235a56a4045/reg/ammonia-1.1.0/log.txt, it fails because of url, so I might have misunderstood if it was a root ?

Pietro Albini (Jul 06 2018 at 21:21, on Zulip):

yeah, the root regression for that is the url crate

Pietro Albini (Jul 06 2018 at 21:22, on Zulip):

it's not marked as such because the url crate is technically not a regression: it was already failing to build on the first toolchain

lqd (Jul 06 2018 at 21:23, on Zulip):

ok understood thank you, so I will refilter those logs again to remove the ones without local errors

lqd (Jul 06 2018 at 21:24, on Zulip):

it might only happen because of url, but since it's a popular crate I suspect it will be necessary to filter them out

simulacrum (Jul 06 2018 at 21:25, on Zulip):

it's not marked as such because the url crate is technically not a regression: it was already failing to build on the first toolchain

I wonder if we can instead say "root build failure causes"

simulacrum (Jul 06 2018 at 21:25, on Zulip):

i.e. generate a report that looks just at whether we failed to build

simulacrum (Jul 06 2018 at 21:26, on Zulip):

ultimately crate X only regressed because crate Y failed to build; fixing crate Y is the only way to fix crate X in this situation

Pietro Albini (Jul 06 2018 at 21:26, on Zulip):

what I'd really like to do is to do a diff of the errors instead of just checking if it builds, so we can catch regressions even on same-build-fail crates

Pietro Albini (Jul 06 2018 at 21:27, on Zulip):

but that's a bit longer term than this

simulacrum (Jul 06 2018 at 21:27, on Zulip):

Sure, that'd be better -- but it would make logs ~unreadable since we need json error then

simulacrum (Jul 06 2018 at 21:27, on Zulip):

And there's not currently a good way to go from JSON to human :/

Pietro Albini (Jul 06 2018 at 21:27, on Zulip):

sure there is, the json output includes the rendered field last time I checked

simulacrum (Jul 06 2018 at 21:28, on Zulip):

ah, interesting

Pietro Albini (Jul 06 2018 at 21:28, on Zulip):

we can replace the json in the logs with the rendered field while we catch those errors

Pietro Albini (Jul 06 2018 at 21:29, on Zulip):

but before there is craterbot to finish...

lqd (Jul 06 2018 at 21:30, on Zulip):

how close is craterbot btw ? looked almost ready, and/or in review

Pietro Albini (Jul 06 2018 at 21:31, on Zulip):

it just needs the last PR to be merged (better-run-graph) before we can start trying it live, and aidanhs promised to review it before monday

Pietro Albini (Jul 06 2018 at 21:31, on Zulip):

and that PR alone turns a 2 days check only run into only one day

lqd (Jul 06 2018 at 21:32, on Zulip):

niiiiice

lqd (Jul 06 2018 at 21:34, on Zulip):

ok now we're talking

lqd (Jul 06 2018 at 21:35, on Zulip):

in fact it is _so_ reduced I'm worried it's wrong :p https://hackmd.io/s/Bkqcxvazm

Pietro Albini (Jul 06 2018 at 21:36, on Zulip):

well, just fix those and see if that did something in the next run

Pietro Albini (Jul 06 2018 at 21:37, on Zulip):

check-only runs will be really fast soon, so doing one or two extra won't be a problem ;)

lqd (Jul 06 2018 at 21:37, on Zulip):

lol if only I could "just fix" them, I'm not niko :p

Pietro Albini (Jul 06 2018 at 21:38, on Zulip):

:D

lqd (Jul 06 2018 at 21:39, on Zulip):

(some of these errors are from the previously triaged ICEs so I could filter them out as well; and some of the errors here were already triaged from the previous nll-1 run)

simulacrum (Jul 06 2018 at 21:40, on Zulip):

@Pietro Albini Do you think the "tree" aspect of the root regression finder is important? It's simpler to just say how many dependent crates there are...

Pietro Albini (Jul 06 2018 at 21:42, on Zulip):

well, yeah, the thing that matters is the root regressions, not the reverse deps, so the tree is not that important

Pietro Albini (Jul 06 2018 at 21:43, on Zulip):

(by the way, it's fun how we moved the crater dev discussions from nowhere to the nll chan :D)

nikomatsakis (Jul 06 2018 at 21:44, on Zulip):

it is sometimes useful to know the direct dependencies

nikomatsakis (Jul 06 2018 at 21:44, on Zulip):

because sometimes the only fix is to update those

nikomatsakis (Jul 06 2018 at 21:44, on Zulip):

e.g. to use a newer version

simulacrum (Jul 06 2018 at 21:44, on Zulip):

@nikomatsakis Hm, because the root isn't responding?

nikomatsakis (Jul 06 2018 at 21:44, on Zulip):

or because fixing required a breaking change to API

simulacrum (Jul 06 2018 at 21:44, on Zulip):

ah, okay

nikomatsakis (Jul 06 2018 at 21:45, on Zulip):

often people don't backport fixes, too

simulacrum (Jul 06 2018 at 21:54, on Zulip):

Maybe a O(n^3) algorithm isn't the right way to find root crates...

lqd (Jul 06 2018 at 22:16, on Zulip):

(slight aside: with the unused mut lint and liballoc PRs merged, I think we're 2 ICEs fixed away from being able to bootstrap with NLL! and both of those ICEs show up in the crater runs)

simulacrum (Jul 06 2018 at 22:48, on Zulip):

@lqd Could be broken, but https://gist.github.com/Mark-Simulacrum/5dada20317f73d4cf4dd71b8ae87de81

lqd (Jul 06 2018 at 22:50, on Zulip):

oh already nice :)

lqd (Jul 06 2018 at 22:50, on Zulip):

should the logs point at the craterbomb-reports site ?

simulacrum (Jul 06 2018 at 22:52, on Zulip):

uh, they were supposed to, but I think I forgot that bit

simulacrum (Jul 06 2018 at 22:54, on Zulip):

regenerating...

lqd (Jul 06 2018 at 22:55, on Zulip):

btw I'm getting S3 access denied errors on some of the logs

lqd (Jul 06 2018 at 22:55, on Zulip):

to be precise, all of the ones with + in the crate names ;)

lqd (Jul 06 2018 at 22:56, on Zulip):

which should be escaped as %2B

lqd (Jul 06 2018 at 22:57, on Zulip):

example: google-drive3-cli-1.0.7+20171201, crater link https://cargobomb-reports.s3.amazonaws.com/nll-3/00bcc44fb92c28465c727881355c3235a56a4045/reg/google-drive3-cli-1.0.7+20171201/log.txt but should be https://cargobomb-reports.s3.amazonaws.com/nll-3/00bcc44fb92c28465c727881355c3235a56a4045/reg/google-drive3-cli-1.0.7%2B20171201/log.txt

lqd (Jul 06 2018 at 22:59, on Zulip):

note that your crater-merge-report downloaded them fine, it's just in the html crater report that they fail

simulacrum (Jul 06 2018 at 23:12, on Zulip):

@lqd

simulacrum (Jul 06 2018 at 23:12, on Zulip):

Need to replace + with %2B

lqd (Jul 06 2018 at 23:17, on Zulip):

right :)

simulacrum (Jul 06 2018 at 23:18, on Zulip):

@lqd https://gist.github.com/Mark-Simulacrum/22ee460c5ff2a6f64ee04b41fe638bc6

lqd (Jul 06 2018 at 23:19, on Zulip):

niiice!

lqd (Jul 06 2018 at 23:20, on Zulip):

there are so many crates broken by url 1.7.0

lqd (Jul 06 2018 at 23:23, on Zulip):

680 crates depend on it, and those are super popular as well

nikomatsakis (Jul 07 2018 at 01:39, on Zulip):

d'oh

Jake Goulding (Jul 07 2018 at 15:26, on Zulip):

lol if only I could "just fix" them, I'm not niko :p

Not yet, maybe...

lqd (Jul 08 2018 at 11:45, on Zulip):

so update on crater:

This is all I have. Enjoy your Sunday everyone (and/or flights to Greece) :)

lqd (Jul 16 2018 at 15:44, on Zulip):

lol at minimizing errors in crates with dozens of

impl<'v, 'i, 'c, V, I, C, G, W> Apply<'c, Vec<W>> for GroupBy<'i, Block<'v, 'i, 'c, V, I, C>, G>
    where V: Clone,
          I: Clone + Eq + Hash,
          C: Clone + Eq + Hash,
          G: 'c + Clone + Eq + Hash + Ord,
          W: 'c + Clone
{
    type In = Block<'v, 'i, 'c, V, I, C>;
    type FOut = Vec<W>;
    type Out = Block<'c, 'c, 'c, W, G, C>;
...
Jake Goulding (Jul 16 2018 at 15:45, on Zulip):

these are the SO questions I hate the most

lqd (Jul 16 2018 at 16:54, on Zulip):

and then there's the error hidden in layers of macros

lqd (Jul 16 2018 at 16:54, on Zulip):

I did leave the best ones for the end :3

lqd (Jul 16 2018 at 17:27, on Zulip):

"Pokémon Gen3 save data" :joy:

lqd (Jul 16 2018 at 17:28, on Zulip):

I don't what this is doing but it's awesome

nikomatsakis (Jul 16 2018 at 17:31, on Zulip):

funny:)

lqd (Jul 16 2018 at 17:57, on Zulip):

ok so I think they're "all" done bar one with thousands of lines of macros I couldn't untangle in an hour (and also the ones crater-tree ignores: the GH repos). Since some of the errors are weird or in a grey area between bugs and feature requests, it would be awesome if someone more knowledgeable about all these could do a pass over the results and reduced repros to correct my mistakes and things I didn't know or missed :)

nikomatsakis (Jul 16 2018 at 18:01, on Zulip):

amazing! awesome

Jake Goulding (Jul 16 2018 at 18:19, on Zulip):

Hopefully you fix all those and all the github ones get fixed too ;-)

lqd (Jul 16 2018 at 18:21, on Zulip):

@Jake Goulding don't forget some of those are definitely bugs in crates that AST borrowck let slip through :p

lqd (Jul 16 2018 at 18:22, on Zulip):

which was in a sense lucky for me; "yay 300 regressions off the list" ;)

Jake Goulding (Jul 16 2018 at 18:37, on Zulip):

The AST borrow-sometimes-checker

lqd (Jul 17 2018 at 12:15, on Zulip):

so there are 2 main "reports", I'll first start with the one about the root regressions' dependencies here. These results contain both ICEs and errors spanning multiple versions of a crate (marked "same errors as XXXX"), so we can ignore all those. Then there are little bits of text for the others, some are "this looks like the 2-phase-borrows expansion feature" we just talked about, but others are less clear cut to me, so I've added a repro as well as why they seemed interesting/weird. For example "this might be fixed-by-NLL" or "some outlives annotations are missing but I'm not sure if rustc should/will infer them" or "is this for NLL2/polonius rather than today". The page might look big but that's like 25 crates, with the ICEs, multiple versions, known issues etc we mentioned, the ones needing a quick pass are maybe 8 total.

lqd (Jul 17 2018 at 12:24, on Zulip):

The second report is about the errors in root regressions which were really in the crates and not their dependencies here. 31 in total, <10 to be ignored because of ICEs, known errors, and a couple of errors happening in multiple versions of crate, etc and is basically the same story as the previous report: a little blurb explaining what I thought the error was when I could, and repros. All in all mostly similar cases to the previous report, not exactly the same but where I wasn't sure, especially around the ones which feel correct to reject, or we're currently rejecting but maybe not in the future expanded 2ΦB, and the likes.

Eh2406 (Jul 18 2018 at 17:27, on Zulip):

We probably are also making Cargo somewhat unhappy since we put ~20,000 crates into the same target dir

I got into dev for Cargo in order to make crater faster... It was a furry yak. What percentage of a crater run is spent in cargo?

lqd (Jul 31 2018 at 08:29, on Zulip):

with Felix landing Niko's #52731 yesterday, it looks to me like all the ICEs encountered in the crater runs are fixed :tada: so we could do another one soon (but we might hit another recent ICE that Oli and Matthew have been working on if we do it before their fix lands)

Matthew Jasper (Jul 31 2018 at 11:31, on Zulip):

The ICE is fixed, but there will still be regressions from the additional errors.

lqd (Aug 16 2018 at 12:35, on Zulip):

@simulacrum #53426 should be sufficient for another crater run right ?

simulacrum (Aug 16 2018 at 12:45, on Zulip):

I think so, kicked off a try build

lqd (Aug 16 2018 at 12:47, on Zulip):

awesome, thank you

lqd (Aug 16 2018 at 12:49, on Zulip):

I'm not sure this is the correct patch either btw, but it did seem to enable the MIR borrowck and 2PB in my tests

nikomatsakis (Aug 16 2018 at 14:01, on Zulip):

nice

lqd (Aug 16 2018 at 14:25, on Zulip):

mostly I'm wondering about tests, as they will not pass by just enabling these default borrowck/2PB options, but I don't know if the try builds or crater require them ?

lqd (Aug 16 2018 at 15:36, on Zulip):

(the try build for the crater PR succeeded btw, it appears to match the instructions for #53171 -- I guess we'll need an final ok from @pnkfelix or @nikomatsakis and it should be good to go :)

nikomatsakis (Aug 16 2018 at 16:05, on Zulip):

I think it looks great

nikomatsakis (Aug 16 2018 at 16:05, on Zulip):

I don't know how to start crater though I've heard of things like craterbot

lqd (Aug 16 2018 at 16:08, on Zulip):

oh yeah I forgot about that :) it'll be for whenever the lovely people in the infra team have time :) (there might not even be a crater box available rn who knows)

lqd (Aug 18 2018 at 09:02, on Zulip):

the crater run is finished! 962 "regressions" — I'll start triaging later but just looking at 50 random results there's this very widespread ICE scrape_region_constraints which seems new to me (but then might be already tracked since I was away :)

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

there appear to be handful of recent issues with that ICE

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

crash: checking hyper with borrowck-migrate #52992 https://github.com/rust-lang/rust/issues/52992

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

Failed to compile hyper with NLL feature enabled #53182

lqd (Aug 18 2018 at 09:23, on Zulip):

it's good that we already track it, less surprises :) thank you

lqd (Aug 18 2018 at 14:31, on Zulip):

oh there's a 2nd ICE, unresolved inference variable in outliveson a couple crates; the rest (568 out of 962) are the scrape_region_constraints one

lqd (Aug 18 2018 at 14:55, on Zulip):

looks to me like the return of #52057 but in an even less probable situation than before — which I'll minimize next

lqd (Aug 19 2018 at 00:14, on Zulip):

here are the ICEs from the crater run #53482

memoryruins (Aug 19 2018 at 01:11, on Zulip):

good analysis :)

lqd (Aug 19 2018 at 16:44, on Zulip):

So between what crater reports as a "regression" and the ones which can appear in other crate's dependencies or crater "errors" or the blacklist etc, I think I have a tally that makes sense to me.

982 regressions, 591 ICEs. 357 crates failing because of a dependency (grouped to 30 root regression crates, even though crater doesn't see all of them as regressions per se), 24 crates failing by themselves. (The astute calculist will notice it doesn't add up to 982 but 972). Compared to the previous reports, I have incorporated more errors from the github projects we track (but ofc those won't be available to cargo-curl or cratesio-curl).

Here is the 24 crates regression report and the 30/357 crate's dependencies regression report. Of course, we've seen a lot of those in the previous 2 runs, but I have not yet incorporated the previous triaging results into these new reports, which is what I will do next.

Matthew Jasper (Aug 19 2018 at 16:51, on Zulip):

A lot of those look like know soundness fixes.

lqd (Aug 19 2018 at 16:55, on Zulip):

yeah, it doesn't look that different from the previous run, a good sign (I think :)

lqd (Aug 19 2018 at 16:57, on Zulip):

the number of ICE occurrences has gone up (slightly) but the kinds of ICEs have gone down, there's way less "regressions", etc

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

to make it less confusing I will merge the "deps" report into the crates one somehow later today (eg by taking logs from dependent crates instead for the ones where we "don't have" them) so use only the second one ;)

lqd (Aug 20 2018 at 15:41, on Zulip):

alright I'm out of time, so, update: I've merged the 2 reports into the crates one, and maybe 40 out the 45/27 are now done (they don't all have repros as some errors look similar or are too time consuming to minimize) and ready to be double-checked

memoryruins (Aug 20 2018 at 17:09, on Zulip):

unable to help double-checking until back in town end of week. enjoying staying updated on the progress, well done with the round-up!

lqd (Aug 21 2018 at 11:18, on Zulip):

does this feel familiar to anyone or should I file an issue ? it's like 25% of the regressions in the crater run

Jake Goulding (Aug 21 2018 at 13:04, on Zulip):

Feels like that's the same as foo(&mut self, bar(&self))

Jake Goulding (Aug 21 2018 at 13:05, on Zulip):

Note it compiles with let mut w= [Gf256{}];

Jake Goulding (Aug 21 2018 at 13:06, on Zulip):

So I'm thinking it's some subtlety around Index desugaring or maybe the difference of "built in" indexing

lqd (Aug 21 2018 at 13:10, on Zulip):

that's basically what I thought as well, it seems close the the 2PB expansion feature request but not exactly the same, seems to involve indexing and operator overloading, so I don't know

lqd (Aug 21 2018 at 13:10, on Zulip):

in the meantime I'll leave it as is in the report and @nikomatsakis and @pnkfelix can weigh in when they have time to double check

Jake Goulding (Aug 21 2018 at 13:16, on Zulip):

*IndexMut::index_mut(&mut w, 0) -= *Index::index(&w, 0) fails; hmm

nikomatsakis (Aug 21 2018 at 16:23, on Zulip):

@lqd I'd like to update https://github.com/nikomatsakis/nll-crater-run-results with the latest crater run results... is there something I can read over to see the classifications you've done so far etc?

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

oops I had forgotten about that or could have a made a PR :/

nikomatsakis (Aug 21 2018 at 16:25, on Zulip):

no big thing

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

besides the ICE issue, there is https://hackmd.io/yK9YhE8-TGGA79tqOpkoCA?both

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

does this feel familiar to anyone or should I file an issue ? it's like 25% of the regressions in the crater run

I believe this may be https://github.com/rust-lang/rust/issues/47349 (which is categorized as WONTFIX afaik)

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

there may be some kind of 2PB expansion we can do but that's not clear

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

like https://github.com/rust-lang/rust/issues/51915

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

yeah but that alone wouldn't suffice

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

yeah

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

I think what happens here is that we first deref the Vec<T> to [T]

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

and right there we do a mutable borrow

nikomatsakis (Aug 21 2018 at 16:28, on Zulip):

and then we index into the [T] (mutably)

nikomatsakis (Aug 21 2018 at 16:28, on Zulip):

so there is a whole bunch of stuff happening before we get to the reads

nikomatsakis (Aug 21 2018 at 16:28, on Zulip):

I think it's beyond what 2PB can achieve

nikomatsakis (Aug 21 2018 at 16:28, on Zulip):

perhaps some more advanced variant could do it

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

e.g., the "phi" borrows that arielby was proposing

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

gluon_base and try_transform_mut seem fixed compared to before

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

the ICEs are here btw

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

@lqd ok so the hackmd document contains the rest, and the ICEs are in #53482?

lqd (Aug 21 2018 at 17:21, on Zulip):

yeah, and I did https://github.com/nikomatsakis/nll-crater-run-results/pull/1 to start

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

oh nice!

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

is that complete?

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

i.e., should I merge?

lqd (Aug 21 2018 at 17:22, on Zulip):

it's just updating a couple crates

lqd (Aug 21 2018 at 17:22, on Zulip):

not complete unfortunately, I was working on adding more which you could then double check and review

lqd (Aug 21 2018 at 17:24, on Zulip):

ie a lot of the crates in the hackmd doc are not in the repo yet, so I started by updating the ones which were in both

lqd (Aug 21 2018 at 17:28, on Zulip):

but it's mergeable I mean, those 3-4 crates are complete, it's just not the full run yet :)

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

would you rather I wait?

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

I can add what I did from the hackmd to this PR, I'd just need to know how you'd want to review/modify the results that wouldn't be 100% sure

lqd (Aug 21 2018 at 17:31, on Zulip):

for example the case we talked about earlier, maybe a different commit for those, and so on ?

nikomatsakis (Aug 21 2018 at 17:32, on Zulip):

I'm confused by the attr case

nikomatsakis (Aug 21 2018 at 17:32, on Zulip):

I thought I filed a bug for that...

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

I was confused as well :)

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

/me feels disorganized

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

I searched for an issue mentioning attr while doing the doc and didn't seem to find any

lqd (Aug 21 2018 at 17:35, on Zulip):

(but travis messages contain "attr" so it makes search a bit hit and miss in this case :)

nikomatsakis (Aug 21 2018 at 17:35, on Zulip):

maybe I just forgot to file?

nikomatsakis (Aug 21 2018 at 17:35, on Zulip):

possible

lqd (Aug 21 2018 at 17:35, on Zulip):

possible :)

lqd (Aug 21 2018 at 17:36, on Zulip):

or maybe tried it on a branch where it worked

nikomatsakis (Aug 21 2018 at 17:40, on Zulip):

I guess regarding this ICE: "2. unresolved inference variable in outlives: _#3t"

nikomatsakis (Aug 21 2018 at 17:40, on Zulip):

we should open a new issue

nikomatsakis (Aug 21 2018 at 17:40, on Zulip):

I hate to reopen existing issues, too depressing

lqd (Aug 21 2018 at 17:41, on Zulip):

plus it looks a bit more involved (eg, possible impl Trait interaction) even if it manifests with the same ICE

nikomatsakis (Aug 21 2018 at 17:43, on Zulip):

right

nikomatsakis (Aug 21 2018 at 17:43, on Zulip):

ok, I opened another issue

lqd (Aug 21 2018 at 17:45, on Zulip):

I'll put the ones where I'm not sure in the top table, and feel free to move them down as "correctly failing by MIR borrowck"

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

@lqd can you link attr to https://github.com/rust-lang/rust/issues/53569 in https://github.com/nikomatsakis/nll-crater-run-results/pull/1/files ?

lqd (Aug 21 2018 at 17:46, on Zulip):

sure

nikomatsakis (Aug 21 2018 at 17:47, on Zulip):

I guess let me ask a different question :)

nikomatsakis (Aug 21 2018 at 17:47, on Zulip):

I want to go over these results

nikomatsakis (Aug 21 2018 at 17:47, on Zulip):

would you rather I go through the hackmd comment and leave comments?

nikomatsakis (Aug 21 2018 at 17:47, on Zulip):

I remember last time feeling a need to organize differently later on

nikomatsakis (Aug 21 2018 at 17:47, on Zulip):

but it is a handy way to do a first pass, I suppose

lqd (Aug 21 2018 at 17:48, on Zulip):

yeah maybe it can be a nice first pass, so that I can then incorporate those comments into the PR

nikomatsakis (Aug 21 2018 at 17:48, on Zulip):

k

lqd (Aug 21 2018 at 17:49, on Zulip):

all the cases similar to sprs-0.6.2 are what we talked about before and can be all marked as #47349 I think

nikomatsakis (Aug 21 2018 at 17:50, on Zulip):

another question that is worth thinking about

nikomatsakis (Aug 21 2018 at 17:50, on Zulip):

is whether we want to make an effort to track WONTFIX so that later we can have some idea

nikomatsakis (Aug 21 2018 at 17:50, on Zulip):

seems like yes

nikomatsakis (Aug 21 2018 at 17:50, on Zulip):

e.g., it'd be very useful to know the "classes" of fixes we saw in the wild

nikomatsakis (Aug 21 2018 at 17:50, on Zulip):

so e.g. there is #47349

nikomatsakis (Aug 21 2018 at 17:51, on Zulip):

there is foo(&mut bar, ...), which you have been tagging as #51915 -- though I might draw finer distinctions

nikomatsakis (Aug 21 2018 at 17:51, on Zulip):

in particular between e.g. foo(bar, ...) (with an implicit reborrow) and the explicit form

nikomatsakis (Aug 21 2018 at 17:51, on Zulip):

seems like it would be good to classify those

nikomatsakis (Aug 21 2018 at 17:51, on Zulip):

I guess by tagging with issues

nikomatsakis (Aug 21 2018 at 17:52, on Zulip):

(if nothing else, these might be cases where we also want specialized diagnostics)

lqd (Aug 21 2018 at 17:52, on Zulip):

agreed; (and maybe in the same vein we still have the run results without 2PB in case we want to see what the different "levels" of 2PB handling look like in the wild)

nikomatsakis (Aug 21 2018 at 18:04, on Zulip):

@lqd maybe I'll leave FIXME tags for things where it seems like some further work could be done (e.g., filing or finding a tracking issue for Fixed By NLL etc)

lqd (Aug 21 2018 at 18:04, on Zulip):

@nikomatsakis yeah I think liner is weird

nikomatsakis (Aug 21 2018 at 18:06, on Zulip):

I think the error is probably legit

nikomatsakis (Aug 21 2018 at 18:06, on Zulip):

specifically I think there is a bug in AST borrowck .. I guess I should go see if I can find it

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

the repro surely was done for the previous run, I didn't notice the errors are probably different now for this crate

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

@pnkfelix may recall... I think that AST borrowck though doesn't consider match b { true, false } to read b, which is clearly false

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

or at least it doesn't under some circumstances

nikomatsakis (Aug 21 2018 at 18:09, on Zulip):

yep, example from playground:

enum Foo { V1, V2 }

fn main() {
  let mut foo = Foo::V1;
  let bar = &mut foo;
  match foo { Foo::V1 => (), Foo::V2 => () }
  drop(bar);
}
nikomatsakis (Aug 21 2018 at 18:09, on Zulip):

that compiles with AST borrowck (!)

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

but not with NLL

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

seems like https://github.com/rust-lang/rust/issues/27282 may be the right issue, but that's really for a different problem

lqd (Aug 21 2018 at 18:15, on Zulip):

@nikomatsakis in the repo you have "wants a more aggressive 2PB" is that the WONTFIX one like for capnp ?

nikomatsakis (Aug 21 2018 at 18:18, on Zulip):

probably... where did I write that?

lqd (Aug 21 2018 at 18:20, on Zulip):

lower table, extended-collections

lqd (Aug 21 2018 at 18:20, on Zulip):

maybe not since there is an issue hmm

lqd (Aug 21 2018 at 18:22, on Zulip):

maybe the WONTFIX for capnpin the hackmd doc might be one of those cases we mentioned where we can track them later

nikomatsakis (Aug 21 2018 at 18:24, on Zulip):

I guess I wish I had transcribed the log links =)

nikomatsakis (Aug 21 2018 at 18:25, on Zulip):

but yes looking at the log here, it looks like this comes from the foo(&mut bar, use(bar)) pattern

nikomatsakis (Aug 21 2018 at 18:25, on Zulip):

Really, I think we should file a tracking issue for .. hmm, what to call it

nikomatsakis (Aug 21 2018 at 18:25, on Zulip):

I think arielby's name was Ref2Φ

nikomatsakis (Aug 21 2018 at 18:25, on Zulip):

but I'd prefer something one can type in ASCII :P

lqd (Aug 21 2018 at 18:25, on Zulip):

:D

nikomatsakis (Aug 21 2018 at 18:25, on Zulip):

maybe just Ref2 :)

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

Ref2Phi2Furious

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

anyway, the point, a generalized form of 2PB that is integrated into the type system proper

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

it seems like the examples we see almost all fall into that category

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

rather than the actual expansion I had in mind for #51915

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

so maybe we should just repurpose #51915, I don't know

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

in particular here the extended-collections error in question:

Aug 17 23:13:06.434 INFO kablam! 155 |                       curr_node = &mut mem::replace(
Aug 17 23:13:06.434 INFO kablam!     |  ______________________________________-
Aug 17 23:13:06.434 INFO kablam! 156 | |                         &mut next_link,
Aug 17 23:13:06.434 INFO kablam!     | |                         -------------- first mutable borrow occurs here
Aug 17 23:13:06.434 INFO kablam! 157 | |                         (*next_link.next).get_pointer_mut(curr_height),
Aug 17 23:13:06.434 INFO kablam!     | |                         ^^^^^^^^^^^^^^^^^ second mutable borrow occurs here
Aug 17 23:13:06.434 INFO kablam! 158 | |                     ).next;
Aug 17 23:13:06.434 INFO kablam!     | |_____________________- borrow used here in later iteration of loop
lqd (Aug 21 2018 at 18:28, on Zulip):

(maybe we can link all the hackmd we've made in the repo, to at least have those until all the FIXMEs are done)

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

it seems like we basically have two common regressions—

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

this one (#51915) and the fact that we correctly check += now

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

interestingly, both of those might be fixed by the same 2-phase borrow sort of thing

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

hmm @lqd this example may be a bug:

minimized

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

in particular because of this:

    parse_sess: &'a ParseSess,
nikomatsakis (Aug 21 2018 at 18:31, on Zulip):

this implies that e.g. you could rewrite that code too

nikomatsakis (Aug 21 2018 at 18:32, on Zulip):

oh...

nikomatsakis (Aug 21 2018 at 18:32, on Zulip):

never mind :)

nikomatsakis (Aug 21 2018 at 18:32, on Zulip):

I was going to say you could rewrite to let parse_sess = cx.parse_sess;

nikomatsakis (Aug 21 2018 at 18:32, on Zulip):

but of course that itself would be an illegal access

nikomatsakis (Aug 21 2018 at 18:32, on Zulip):

presumably this same confusion is why the AST-based checker accepted it

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

ok I went through the whole hackmd doc

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

thanks a million times for doing this initial triage work @lqd :crown:

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

so, so helpful

lqd (Aug 21 2018 at 18:36, on Zulip):

awesome thank you! I'll incorporate the hackmd comments now

lqd (Aug 21 2018 at 18:40, on Zulip):

oh but I can link the WONTFIX cases to https://github.com/rust-lang/rust/issues/47349 as it's "the diagnostics to guide users" for cases which used to work IIUC

nikomatsakis (Aug 21 2018 at 18:43, on Zulip):

yes, seems good

nikomatsakis (Aug 21 2018 at 18:43, on Zulip):

it's worth separating the cases anyway

nikomatsakis (Aug 21 2018 at 18:44, on Zulip):

as there might be some targeted fix for one and not the other

lqd (Aug 21 2018 at 18:55, on Zulip):

I'll reminimize rs-graph-0.14.0 to show more specifically where the 2 borrowcks differ, otherwise it'll be tough to check if it's #53121

nikomatsakis (Aug 21 2018 at 18:56, on Zulip):

yep

nikomatsakis (Aug 21 2018 at 18:56, on Zulip):

that'd be awesome

Jake Goulding (Aug 21 2018 at 18:56, on Zulip):

need to write that auto minimization tool, like bugpoint ;-)

lqd (Aug 21 2018 at 18:58, on Zulip):

creduce works "fine"

Jake Goulding (Aug 21 2018 at 18:59, on Zulip):

Oh?

lqd (Aug 21 2018 at 18:59, on Zulip):

but it's super aggressive in reducing the names, making it a bit harder to link to the original crate

Jake Goulding (Aug 21 2018 at 18:59, on Zulip):

C-Reduce happens to do a pretty good job reducing the size of programs in languages other than C/C++, such as JavaScript and Rust.

Jake Goulding (Aug 21 2018 at 18:59, on Zulip):

huh, TIL

Jake Goulding (Aug 21 2018 at 19:00, on Zulip):

I wonder how useful reducing names actually is to finding bugs. Be nice if you could disable that

lqd (Aug 21 2018 at 19:00, on Zulip):

I did use it for the one crate linking super generic code from hyper, futures, and tokio

lqd (Aug 21 2018 at 19:00, on Zulip):

but then renamed everything so it's easier to understand and closer to the crate

lqd (Aug 21 2018 at 19:01, on Zulip):

that was after a couple hours of reducing this case manually :)

lqd (Aug 21 2018 at 19:02, on Zulip):

(also it uses clang-format so it looks weird, maybe we can hook up rustfmt for it)

lqd (Aug 21 2018 at 19:03, on Zulip):

I was confused by segment-tree as it built fine on nightly

lqd (Aug 21 2018 at 19:04, on Zulip):

but probably I did something wrong instead

lqd (Aug 21 2018 at 19:31, on Zulip):

just in time for the meeting, I've pushed the missing results to https://github.com/nikomatsakis/nll-crater-run-results/pull/1 for all except rs-graph which I'll re-minimize before either opening a new pr or pushing to this one

lqd (Aug 22 2018 at 11:28, on Zulip):

@nikomatsakis added a couple things, including the new rs-graph repros where the 2 borrowcks disagree https://github.com/nikomatsakis/nll-crater-run-results/pull/2

lqd (Aug 22 2018 at 11:29, on Zulip):

that was quick :)

nikomatsakis (Aug 22 2018 at 11:29, on Zulip):

/me looks

lqd (Aug 22 2018 at 11:30, on Zulip):

living on the edge, merging before looking :)

lqd (Aug 22 2018 at 11:32, on Zulip):

I'll reminimize liner now

lqd (Aug 22 2018 at 11:51, on Zulip):

I think last time I minimized liner, using a _ pattern might have made things confusing (rather than contributing to my goal of not focusing on the match arms): that is, the errors didn't change between the 2 crater runs, and the 2 borrowcks differed on the minimal repro.

lqd (Aug 22 2018 at 11:51, on Zulip):

This repro is more truthful to the crate

lqd (Aug 22 2018 at 11:52, on Zulip):

but this one with the _ pattern was tagged as #53114

lqd (Aug 22 2018 at 11:52, on Zulip):

(while I thought the errors looked legit)

nikomatsakis (Aug 22 2018 at 11:59, on Zulip):

thanks :)

nikomatsakis (Aug 22 2018 at 11:59, on Zulip):

my take would be that the "truthful" repro is "not a bug"

nikomatsakis (Aug 22 2018 at 12:00, on Zulip):

(though, incidentally, I think that the Ref2 proposal might accept it)

nikomatsakis (Aug 22 2018 at 12:00, on Zulip):

(or some variants thereof)

nikomatsakis (Aug 22 2018 at 12:00, on Zulip):

but #53114 may well be a bug :P

nikomatsakis (Aug 22 2018 at 12:00, on Zulip):

sometimes lies are useful I guess

lqd (Aug 22 2018 at 12:00, on Zulip):

I'll open another PR on your repo for the repro and moving them in the "correct" table

lqd (Aug 22 2018 at 12:10, on Zulip):

and here it is https://github.com/nikomatsakis/nll-crater-run-results/pull/3

nikomatsakis (Aug 22 2018 at 13:11, on Zulip):

ty <3

lqd (Aug 22 2018 at 13:24, on Zulip):

np :)

lqd (Aug 28 2018 at 16:59, on Zulip):

/me realizes he still hasn't minimized the 2 remaining crates from the crater run :scream:

nikomatsakis (Aug 28 2018 at 16:59, on Zulip):

eek

nikomatsakis (Aug 28 2018 at 16:59, on Zulip):

I didn't realize there were any left :)

lqd (Aug 28 2018 at 16:59, on Zulip):

/me shouldn't have said anything

lqd (Aug 28 2018 at 17:02, on Zulip):

for reals centril's proptest-arbitrary + proptest is tough to do, I spent a "couple" hours on it and came up short so with new hope and energy and c-reduce I'll try to do so before the meeting (I was finishing automating the timing of the "modern crate versions")

lqd (Aug 28 2018 at 18:15, on Zulip):

this is what I mean, repro, not minimized https://play.rust-lang.org/?gist=c990af931ab56d93207efcade0ffd01b&version=nightly&mode=debug&edition=2015

lqd (Aug 28 2018 at 18:16, on Zulip):

:scream:

nikomatsakis (Aug 28 2018 at 18:17, on Zulip):

well.. that's not terrible

nikomatsakis (Aug 28 2018 at 18:17, on Zulip):

can we run the expanded form instead?

lqd (Aug 28 2018 at 18:17, on Zulip):

the destination was not terrible but the journey was :p

nikomatsakis (Aug 28 2018 at 18:18, on Zulip):

heh

lqd (Aug 28 2018 at 18:19, on Zulip):

by now probably yeah, I'll just throw it at c-reduce you never know :)

lqd (Aug 28 2018 at 18:19, on Zulip):

(c-reduce works on a single file)

pnkfelix (Aug 28 2018 at 18:29, on Zulip):

@lqd ping me if you want to bounce around ideas on how to reduce. (As Niko says, the first step is often to work on an expanded form. My second step is often, or at least used to be, to employ “everybody loops”.)

lqd (Aug 28 2018 at 18:31, on Zulip):

@pnkfelix cool, thank you :) thankfully in my cases it's mostly about traits and bounds so I can remove all the methods and/or employ "everybody-unimplementeds"

pnkfelix (Aug 28 2018 at 18:32, on Zulip):

Bah; why panic when you can diverge? ;)

lqd (Aug 29 2018 at 12:22, on Zulip):

the proptest-arbitrary minimized results in https://github.com/nikomatsakis/nll-crater-run-results/pull/4 look like #53570

nikomatsakis (Aug 29 2018 at 12:57, on Zulip):

ok, I should test if my fix fixes it

lqd (Aug 29 2018 at 13:00, on Zulip):

ah yes, there's already a PR fixing this issue, nice

nikomatsakis (Aug 29 2018 at 14:06, on Zulip):

my branch does not fix the examples

nikomatsakis (Aug 29 2018 at 14:06, on Zulip):

did we file a bug for them?

nikomatsakis (Aug 29 2018 at 14:06, on Zulip):

I think they are a separate — but perhaps similar — problem

nikomatsakis (Aug 29 2018 at 14:06, on Zulip):

I've not dug into it more deeply yet

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

I don't think we did no

lqd (Aug 29 2018 at 14:08, on Zulip):

I can do it and ref it in the PR if you want ?

nikomatsakis (Aug 29 2018 at 14:15, on Zulip):

would be good, thanks

lqd (Aug 29 2018 at 14:42, on Zulip):

:face_palm: #53570 is about closures and there were no closures in the minimized versions, sorry. I filed #53789 and updated the nll-crater-run-results PR

lqd (Aug 29 2018 at 16:39, on Zulip):

@nikomatsakis if those proptest-arbitrary cases are a duplicate of #53121 it'll be interesting to see why they differ wrt crate-type=lib ?

nikomatsakis (Aug 29 2018 at 16:50, on Zulip):

yeah, I forgot about that

lqd (Sep 04 2018 at 11:56, on Zulip):

I finalized minimized pom from the crater run: is this playground example an instance of #53040 ? and is this issue tracking the fact that the error "returning this value requires that '1 must outlive '2" doesn't contain a note with the '2 lifetime here ?

lqd (Sep 13 2018 at 15:34, on Zulip):

also I was wondering, now that type ascription support is more advanced, and the previous run's ICEs are fixed, if we'll need another crater run soon, maybe wait until more of the type ascription work lands ? (and since I'm guessing the RC is super soon, it could be for some time during RC2 ?)

nikomatsakis (Sep 13 2018 at 15:56, on Zulip):

I'm mildly nervous about ascription causing ICEs or errors

nikomatsakis (Sep 13 2018 at 15:56, on Zulip):

so it seems good to run it

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

but it may indeed be that we can piggy back on the RC run

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

although

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

I think that may not work for us

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

beacuse it will be in migration mode

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

and we want to see errors, right?

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

seems like we should do our own run

lqd (Sep 13 2018 at 15:57, on Zulip):

yeah good point

simulacrum (Sep 13 2018 at 15:57, on Zulip):

Craterbot allows anyone on the compiler team (which it gets through, I think, the GH team) to queue runs now :)

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

nice

simulacrum (Sep 13 2018 at 15:59, on Zulip):

https://crater.rust-lang.org/ is also a wonderful UI now

lqd (Sep 13 2018 at 16:00, on Zulip):

and wonderfully devoid of running experiments which we could take advantage of at any time :)

memoryruins (Sep 13 2018 at 16:09, on Zulip):

perfect timing :) would you mind opening the PR for a crater run again @lqd ?

lqd (Sep 13 2018 at 16:11, on Zulip):

yeah I can take care of that, do we want it like soon or would we want more of the remaining ascription work to land first ?

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

I thnk all the ascription work that is going to land in near future has landed

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

the rest isn't even started yet

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

also, we believe it's mostly "the details"

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

but not the common cases

lqd (Sep 13 2018 at 16:14, on Zulip):

oh ok I'll take care of it now then

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

https://github.com/rust-lang/rust/pull/53426 is updated, and should be ready to be try-ed and then craterbot-ted

memoryruins (Sep 13 2018 at 23:49, on Zulip):

screenshot.3561.png
The actual stderr differed from the expected stderr.

lqd (Sep 14 2018 at 03:04, on Zulip):

(if that's a screenshot of CI errors, the failing tests are expected, but the try run will succeed)

memoryruins (Sep 14 2018 at 03:23, on Zulip):

ah yea, it is haha (good to know!)

lqd (Sep 14 2018 at 10:49, on Zulip):

@pnkfelix salut Felix :) could you ask bors for a try run on this PR so we can later do the crater run ?

pnkfelix (Sep 14 2018 at 10:49, on Zulip):

done

lqd (Sep 14 2018 at 10:50, on Zulip):

@pnkfelix merci

lqd (Sep 14 2018 at 13:25, on Zulip):

btw we're good to go for the crater run on #53426. However, since a run has already been made for this PR, it's likely the name parameter needs to be different for craterbot this time, I guess "pr-53426-2" or similar would be enough

pnkfelix (Sep 14 2018 at 13:29, on Zulip):

okay lets see

pnkfelix (Sep 14 2018 at 13:31, on Zulip):

can you just save me time and tell me what to type into the comment? I haven't used craterbot before and don't want to thrash trying to identify the correct values for e.g. start=master#f00ba5

pnkfelix (Sep 14 2018 at 13:31, on Zulip):

or should it be literally the same except for the name field?

pnkfelix (Sep 14 2018 at 13:31, on Zulip):

(same compared to https://github.com/rust-lang/rust/pull/53426#issuecomment-413696519 that is ...)

lqd (Sep 14 2018 at 13:31, on Zulip):

I don't know either but can look it up :)

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

ah wait the end needs to point to the new try commit probably

lqd (Sep 14 2018 at 13:32, on Zulip):

since I rebased over a more recent master I think it's going to be a different sha

lqd (Sep 14 2018 at 13:32, on Zulip):

yup

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

okay well if you give me a comment to write, I'll sanity-check it and then paste in.

lqd (Sep 14 2018 at 13:33, on Zulip):

yeah I'm trying to understand how it works (I've never used it not have access) but I think I got it, just a sec

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

right, I understand that you and I are both thrashing

lqd (Sep 14 2018 at 13:35, on Zulip):

my understanding is that it's @craterbot run start=master#fccde0018a618eb6f45d2a3c97f629809994dff6 end=try#e3ede4ae5297558caacf160ecf523f3a5759f682 mode=check-only name=pr-53426-2 what do you think ?

lqd (Sep 14 2018 at 13:35, on Zulip):

e3ed being the merge of the try run, and fcc the parent master commit IIUC

lqd (Sep 14 2018 at 13:36, on Zulip):

we can also run that by the infra team for extra verification :)

pnkfelix (Sep 14 2018 at 13:41, on Zulip):

lets give it a whirl

pnkfelix (Sep 14 2018 at 13:42, on Zulip):

though you left off the caps-lint=warn that was in @simulacrum 's run

lqd (Sep 14 2018 at 13:43, on Zulip):

interesting

lqd (Sep 14 2018 at 13:43, on Zulip):

I don't remember if it's needed this time

pnkfelix (Sep 14 2018 at 13:43, on Zulip):

oh

pnkfelix (Sep 14 2018 at 13:44, on Zulip):

would we be better off leaving that off for this run?

lqd (Sep 14 2018 at 13:44, on Zulip):

I think it's likely you're right and we need it

lqd (Sep 14 2018 at 13:44, on Zulip):

@kennytm if you have a second ^

pnkfelix (Sep 14 2018 at 13:45, on Zulip):

default would be forbid

simulacrum (Sep 14 2018 at 13:45, on Zulip):

I would cap-lints to warn by default

simulacrum (Sep 14 2018 at 13:45, on Zulip):

unless you have some reason not to (though I don't know what that would be)

lqd (Sep 14 2018 at 13:45, on Zulip):

it's likely to not prevent the crates denying all warnings

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

thanks @simulacrum

lqd (Sep 14 2018 at 13:48, on Zulip):

sweet

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

thanks @pnkfelix @simulacrum and @kennytm

lqd (Sep 15 2018 at 10:35, on Zulip):

♪ the number of ICEs is zero ♫

nikomatsakis (Sep 15 2018 at 10:36, on Zulip):

woohoo!

nikomatsakis (Sep 15 2018 at 10:36, on Zulip):

I was nervous about ascriptions adding more ICEs

lqd (Sep 15 2018 at 10:36, on Zulip):

:)

lqd (Sep 15 2018 at 10:36, on Zulip):

and out of the previous 500-600 ICE instances, only 4 crates are unknown/new to us

lqd (Sep 15 2018 at 10:37, on Zulip):

I haven't yet looked at the errors in detail or tried to minimize (WIP doc is here until then and the nll-crater-results repo PRs which will follow)

lqd (Sep 15 2018 at 11:31, on Zulip):

I updated https://github.com/nikomatsakis/nll-crater-run-results/pull/4 and will work through the 4 new crates by next meeting

lqd (Sep 15 2018 at 11:48, on Zulip):

@Pietro Albini hey :) have you seen things like this or this: they are categorized "SameBuildFail", but there are no errors in the logs, is that familiar to you ?

Pietro Albini (Sep 15 2018 at 11:48, on Zulip):

wut

Pietro Albini (Sep 15 2018 at 11:48, on Zulip):

rustc returns that there is an error... let me try those locally

lqd (Sep 15 2018 at 11:48, on Zulip):

that is weird indeed

lqd (Sep 15 2018 at 11:49, on Zulip):

I'm trying locally as well

lqd (Sep 15 2018 at 11:51, on Zulip):

opcua-server 0.3.0 seems to build on a nightly from last week :thinking:

Pietro Albini (Sep 15 2018 at 11:56, on Zulip):

wtf, it's building fine even with the run's toolchain

lqd (Sep 15 2018 at 11:56, on Zulip):

the plot thickens

Pietro Albini (Sep 15 2018 at 11:58, on Zulip):

let me start a crater run locally

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

maybe a spurious weirdness

Pietro Albini (Sep 15 2018 at 11:59, on Zulip):

it's strange if this is spurious and happened on both toolchains

Pietro Albini (Sep 15 2018 at 11:59, on Zulip):

but rustc reported some errors :/

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

rustc didn't want to share those errors with us

Pietro Albini (Sep 15 2018 at 12:03, on Zulip):

wtf, this is happening in my local crater run

lqd (Sep 15 2018 at 12:03, on Zulip):

progress :)

lqd (Sep 15 2018 at 12:11, on Zulip):

hyper doesn't build on crater but that seems normal I guess ? it's missing examples in the crates.io package but they are referenced in Cargo.toml, we can see the failure just because of our --all-targets config

Pietro Albini (Sep 15 2018 at 12:16, on Zulip):

hmm, maybe we should directly strip missing examples

Pietro Albini (Sep 15 2018 at 12:17, on Zulip):

now, it would be really interesting to manage to reproduce the opcua failure outside of the docker container

lqd (Sep 15 2018 at 12:18, on Zulip):

true

lqd (Sep 15 2018 at 12:22, on Zulip):

does crates.io remove parts of your crate when you publish (examples, benchmarks, etc) or this hyper case is probably an author error (cargo.toml "include" doesn't include examples) ? or maybe this is something that can just happen and we may need to be more resilient of this kinds of cases in crater ?

Pietro Albini (Sep 15 2018 at 12:26, on Zulip):

don't know

Jake Goulding (Sep 15 2018 at 12:26, on Zulip):

This may actually be my fault... :embarrassed:

lqd (Sep 15 2018 at 12:26, on Zulip):

wow, so many tests, examples, benches that don't build

Jake Goulding (Sep 15 2018 at 12:27, on Zulip):

I know one of these crates used to ship many MB of test data

Jake Goulding (Sep 15 2018 at 12:27, on Zulip):

And it annoyed me so I removed it from the cargo package

lqd (Sep 15 2018 at 12:27, on Zulip):

I'm wondering whether having both check and check --all-targets separately would slow down things in crater

Jake Goulding (Sep 15 2018 at 12:28, on Zulip):

Since most people never want to run the tests / examples / etc. from the package

lqd (Sep 15 2018 at 12:29, on Zulip):

it makes sense that you did it, and only can impact us I think, eg on crater

lqd (Sep 15 2018 at 12:32, on Zulip):

@Pietro Albini I was also wondering before, what kinds of cases were the "Errors" ? docker problems, OOMs, cargo having problems with lockfiles for some reason, things like that ? for instance this I don't know what to think

Jake Goulding (Sep 15 2018 at 12:32, on Zulip):

It also affects distro packagers who would run tests

lqd (Sep 15 2018 at 12:33, on Zulip):

@Jake Goulding the examples you mean ?

Jake Goulding (Sep 15 2018 at 12:34, on Zulip):

lemme see if I can find my commit, It might have been something else ;-)

lqd (Sep 15 2018 at 12:34, on Zulip):

(from hyper?)

Jake Goulding (Sep 15 2018 at 12:36, on Zulip):

Ah, no, h2

### Before

Unpacked: 59M
Compressed: 5.4M

### After

Unpacked: 788K
Compressed: 133K

Jake Goulding (Sep 15 2018 at 12:37, on Zulip):

This will make people emo.

That's you! :wink:

lqd (Sep 15 2018 at 12:38, on Zulip):

lol

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

I was mostly wondering, 10% of the crates don't build for various reasons; if they don't break downstream that's good enough for me :)

lqd (Sep 15 2018 at 12:40, on Zulip):

like the futures crates don't build either, etc ¯\_(ツ)_/¯

Pietro Albini (Sep 15 2018 at 15:04, on Zulip):

@lqd you shouldn't care about the "errors" section

Pietro Albini (Sep 15 2018 at 15:04, on Zulip):

those are crater errors I need to triage and fix, so they don't depend on the experiment

Pietro Albini (Sep 15 2018 at 15:05, on Zulip):

I just need the time to look into them a bit more :P

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

oh ok, thank you! :)

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

it was mostly out of curiosity

Pietro Albini (Sep 15 2018 at 15:07, on Zulip):

the problem with that is, I need to improve the logging to include in the report more stuff

Pietro Albini (Sep 15 2018 at 15:07, on Zulip):

at the moment just the error is not useful, and I need to log into the agent machines and grep the logs :P

lqd (Sep 15 2018 at 15:10, on Zulip):

understandable :) @Pietro Albini thanks for the awesome job on crater, the bot, the UI, and more that you do

lqd (Sep 16 2018 at 18:51, on Zulip):

(update: the crater run was restarted as some, less than 200, logs have failed uploading)

lqd (Sep 17 2018 at 09:36, on Zulip):

the new run is finished, from a cursory glance I don't think these missing logs were NLL-related, but I'll check in more detail later

Pietro Albini (Sep 17 2018 at 09:39, on Zulip):

looked at it

Pietro Albini (Sep 17 2018 at 09:39, on Zulip):

the missing logs were too big to be uploaded, so they were missing before

lqd (Sep 17 2018 at 09:40, on Zulip):

probably in the samebuildfail or sametestpass categories

Pietro Albini (Sep 17 2018 at 09:40, on Zulip):

the only one that seems a "regression" is cluLamansh, but that's just a lot of warnings

Pietro Albini (Sep 17 2018 at 09:41, on Zulip):

(which means, it was the only one with test-pass on the first toolchain and error on the second one)

lqd (Sep 17 2018 at 09:41, on Zulip):

yeah, no errors

Pietro Albini (Sep 17 2018 at 09:42, on Zulip):

well, great :D

lqd (Sep 17 2018 at 09:42, on Zulip):

:)

lqd (Sep 17 2018 at 09:43, on Zulip):

it could be interesting to look at these warnings and diagnostics in case there are weird cases we don't know about, but today is not that day for me :p

nikomatsakis (Sep 17 2018 at 15:03, on Zulip):

wow, so many tests, examples, benches that don't build

say more?

nikomatsakis (Sep 17 2018 at 15:03, on Zulip):

understandable :) @Pietro Albini thanks for the awesome job on crater, the bot, the UI, and more that you do

also, yes, :clap:

lqd (Sep 17 2018 at 15:17, on Zulip):

if you randomly look at the logs in the "SameBuildFail" category of the full report you'll see a number of crates where tests, benchmarks, examples, don't build on crater, example: this one, or that one

lqd (Sep 17 2018 at 19:39, on Zulip):

two of the new crates in the crater run are of this form, was it a bug in AST borrowck that it used to compile, or is it a bug in MIR borrowck that it does not ?

lqd (Sep 17 2018 at 19:39, on Zulip):

probably fixed-by-nll right?

Matthew Jasper (Sep 17 2018 at 19:42, on Zulip):

Yes it's an AST borrowck bug. goes to find issue

lqd (Sep 17 2018 at 19:43, on Zulip):

/me starts humming Foo Fighters — My Hero

Matthew Jasper (Sep 17 2018 at 19:50, on Zulip):

#53432

lqd (Sep 17 2018 at 19:51, on Zulip):

yussss @Matthew Jasper thanks!!!

lqd (Sep 17 2018 at 19:51, on Zulip):

@Matthew Jasper too bad it seems that a lot of hyper-based crates rely on it :)

Matthew Jasper (Sep 17 2018 at 19:53, on Zulip):

That's seems to be true of all soundness bugs that are in the compiler long enough

Matthew Jasper (Sep 17 2018 at 19:54, on Zulip):

Especially when people have limited institution of when things are valid

lqd (Sep 17 2018 at 21:46, on Zulip):

alright I think I'm basically done @nikomatsakis: I updated https://github.com/nikomatsakis/nll-crater-run-results/pull/4 for the 4th run repros, and also added all the previous runs failing crates' logs (so this one commit kinda messes up the diff, do you want to review/check the other commits individually or want me to make a separate PR for these logs ?) I'll do one final pass over the results now to check I haven't forgotten anything

lqd (Sep 17 2018 at 23:40, on Zulip):

I dislike this very much: 22 crates marked as Regressed don't have logs, I think it's safe to say that at lest some of those are OOMs

lqd (Sep 17 2018 at 23:49, on Zulip):

eg unic-ucd-name-0.7.0 == 12GB ram usage after 63s

lqd (Sep 18 2018 at 00:03, on Zulip):

(but njn has some in-progress work to reduce NLL memory consumption in many perf.rlo benchmarks, and will be running some tests with e.g. this crate by tomorrow — it's likely that it will help, but maybe not completely fix these potential new issues, so we'll see how it goes :)

Jake Goulding (Sep 18 2018 at 01:04, on Zulip):

A person on SO was trying to build Rust code on a VPS with 256MB of RAM. lol 12G

lqd (Sep 18 2018 at 07:11, on Zulip):

update on njn's work

reduces its max-rss by 96%, from 29.1GB to 1.2GB
:tada:

Pietro Albini (Sep 18 2018 at 10:29, on Zulip):

@lqd there is currently a 1.5gb memory limit for builds

Pietro Albini (Sep 18 2018 at 10:29, on Zulip):

also, which are the flags you're using for NLL runs?

lqd (Sep 18 2018 at 10:31, on Zulip):

@Pietro Albini I've also seen some not that big builds, but my thinking was that if crater runs multiple crates at once, the OOM killer might randomly kill one of the smaller processes first ? this could explain the non-huge crates from my list. But a 1.5GB limit explains the other more "mundane" ones at 1-2GB failures

Pietro Albini (Sep 18 2018 at 10:32, on Zulip):

the machine shouldn't run out of memory, we have 16gb and the sum of the limits for the 8 parallel containers is 12gb...

lqd (Sep 18 2018 at 10:32, on Zulip):

for the runs, the defaults are changed to automatically flip on -Zborrowck=mir and -Ztwo-phase-borrows

Pietro Albini (Sep 18 2018 at 10:33, on Zulip):

for the runs, the defaults are changed to automatically flip on -Zborrowck=mir and -Ztwo-phase-borrows

great...

lqd (Sep 18 2018 at 10:33, on Zulip):

maybe the couple small ones from the list are random stuff, as these build locally without issues or high memory usage anyways; most of the others however are likely killed because of the limits

nikomatsakis (Sep 18 2018 at 14:16, on Zulip):

@lqd so how should I review this PR?

nikomatsakis (Sep 18 2018 at 14:16, on Zulip):

commit by commit?

nikomatsakis (Sep 18 2018 at 14:16, on Zulip):

also, thanks again for your work on this. invaluable.

lqd (Sep 18 2018 at 14:17, on Zulip):

yeah, commit by commit should work better, or I can remove the big logs one into another PR if need be

nikomatsakis (Sep 18 2018 at 14:18, on Zulip):

also...dear god...

struct Chain<P, F>(P, F);
impl<'a, I, O, P: Parser<'a, I, Output=O>, U, Q: Parser<'a, I, Output=U>, F: Fn(O) -> Combinator<Q>> Parser<'a, I> for Chain<P, F> {
    type Output = U;
    fn parse(&self, input: &'a [I], start: usize) -> Result<(U, usize)> {
        self.0.parse(input, start).and_then(|(out, pos)|
            self.1(out).0.parse(input, pos)
        )
    }
}
nikomatsakis (Sep 18 2018 at 14:18, on Zulip):

from one of your repros :)

nikomatsakis (Sep 18 2018 at 14:18, on Zulip):

there is at minimum a diagnostics issue there

lqd (Sep 18 2018 at 14:18, on Zulip):

that was "fun" to repro :p

lqd (Sep 18 2018 at 14:20, on Zulip):

yeah I wanted to check whether the slight missing diagnostics was known to anyone or if we needed a new issue

nikomatsakis (Sep 18 2018 at 14:22, on Zulip):

well

nikomatsakis (Sep 18 2018 at 14:22, on Zulip):

ok so I looked at it more closely

nikomatsakis (Sep 18 2018 at 14:23, on Zulip):

the diagnostics issue, I presume, is that the error just gives '_ but we talk about a '2?

nikomatsakis (Sep 18 2018 at 14:23, on Zulip):

that seems clearly like a problem

lqd (Sep 18 2018 at 14:23, on Zulip):

yeah

nikomatsakis (Sep 18 2018 at 14:23, on Zulip):

I believe that the error itself is legitimate but unfortunate

nikomatsakis (Sep 18 2018 at 14:23, on Zulip):

what's happening here is that, when you write the closure |x: &u8| { .. }

nikomatsakis (Sep 18 2018 at 14:23, on Zulip):

given that there is no "expected type" from context

nikomatsakis (Sep 18 2018 at 14:23, on Zulip):

we infer that the lifetime of x is local to the closure itself

nikomatsakis (Sep 18 2018 at 14:23, on Zulip):

and hence that the data cannot escape

nikomatsakis (Sep 18 2018 at 14:24, on Zulip):

(this error needs more improvement, but @davidtwco has been iterating some on that...)

nikomatsakis (Sep 18 2018 at 14:24, on Zulip):

at least, I think this is the problem

nikomatsakis (Sep 18 2018 at 14:24, on Zulip):

we actually don't have a good way to annotate this type afaik

nikomatsakis (Sep 18 2018 at 14:24, on Zulip):

I think the bottom line is that using >> for this is a bad choice in rust :)

nikomatsakis (Sep 18 2018 at 14:25, on Zulip):

in any case, if you modify the example to have a named lifetime that we can talk about, then it passes

nikomatsakis (Sep 18 2018 at 14:25, on Zulip):

sorry, wrong link

nikomatsakis (Sep 18 2018 at 14:25, on Zulip):

passing version

nikomatsakis (Sep 18 2018 at 14:25, on Zulip):

that version contains

let parser = take(0) >> |v:&'a [u8]| {
        take(0).map(move |_| {
            v.to_vec()
        })
    };
nikomatsakis (Sep 18 2018 at 14:25, on Zulip):

note the explicit 'a

nikomatsakis (Sep 18 2018 at 14:26, on Zulip):

I'm not sure why the old borrow checker didn't error out here

Jake Goulding (Sep 18 2018 at 14:26, on Zulip):

we actually don't have a good way to annotate this type afaik

I've seen hacks like fn enforce<T: FnOnce(...)>(f: T) -> T to apply complicated rules to closures

nikomatsakis (Sep 18 2018 at 14:26, on Zulip):

yes, you can do that

nikomatsakis (Sep 18 2018 at 14:27, on Zulip):

note that I said a good way :)

nikomatsakis (Sep 18 2018 at 14:27, on Zulip):

in any case, if you are doing that, you might as well make a method

nikomatsakis (Sep 18 2018 at 14:27, on Zulip):

so e.g. instead of foo >> bar something like foo.and_then(bar)

Jake Goulding (Sep 18 2018 at 14:27, on Zulip):

https://stackoverflow.com/a/46198877/155423

lqd (Sep 18 2018 at 14:27, on Zulip):

there might be a fixed-by-nll issue we're tracking for this ?

nikomatsakis (Sep 18 2018 at 14:28, on Zulip):

I feel like we can probably minimize this further

nikomatsakis (Sep 18 2018 at 14:28, on Zulip):

let me see...

nikomatsakis (Sep 18 2018 at 14:28, on Zulip):

I don't know if there's an issue or not

lqd (Sep 18 2018 at 14:30, on Zulip):

as you can see the repros are not always minimal

nikomatsakis (Sep 18 2018 at 14:48, on Zulip):

ok I went in detail over everything

nikomatsakis (Sep 18 2018 at 14:48, on Zulip):

this is encouraging

nikomatsakis (Sep 18 2018 at 14:48, on Zulip):

seems like no real "new" bugs?

nikomatsakis (Sep 18 2018 at 14:49, on Zulip):

(but for the diagnostic issue)

lqd (Sep 18 2018 at 14:50, on Zulip):

I think yeah

lqd (Sep 18 2018 at 14:50, on Zulip):

I've also added the doc links to the PR desc if you wanted to check these notes

lqd (Sep 18 2018 at 14:50, on Zulip):

that is, apart from the couple crates which OOM, if you've been following this

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

hmm yes I saw that

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

so we have to figure those out

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

have you tried building them locally?

lqd (Sep 18 2018 at 14:52, on Zulip):

yeah, I've all run them locally, the 20 or so

lqd (Sep 18 2018 at 14:52, on Zulip):

and njn's PR with a -96% max-rss is ready to r+

lqd (Sep 18 2018 at 14:53, on Zulip):

I don't know if they tested the second crate I saw yet, but we'll find out as soon as it lands I will recheck

lqd (Sep 18 2018 at 14:54, on Zulip):

that is #54318 which @pnkfelix reviewed earlier, but not r+ed yet, since those results were not available yet

lqd (Sep 18 2018 at 14:55, on Zulip):

I have not yet timed the only one of these 2 I can build, to see how longer it takes with NLL on, but will do so before tonight's meeting

lqd (Sep 18 2018 at 14:56, on Zulip):

(the other wants 29GB of ram ;)

nikomatsakis (Sep 18 2018 at 15:04, on Zulip):

bumped to p=1

davidtwco (Sep 18 2018 at 15:04, on Zulip):

(if there are crates that we want to find out how much RAM they use; and you don't have a machine with sufficient RAM to find that out; I have a 64GB machine that I can build stuff on to find out where they max out)

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

@lqd I guess I will file those diagnostic issues

lqd (Sep 18 2018 at 15:06, on Zulip):

(@davidtwco good to know for the future; thankfully it was only this one crate locally, and njn had found out this number. on crater the limit is 1.5GB so we're cutting it close often there)

Pietro Albini (Sep 18 2018 at 20:22, on Zulip):

@lqd nll runs will be a lot easier soon :)

Pietro Albini (Sep 18 2018 at 20:23, on Zulip):

@craterbot run start=nightly-1970-01-01 end=nightly-1970-01-01+rustflags="-Zborrowck=mir -Ztwo-phase-borrows"

lqd (Sep 18 2018 at 20:23, on Zulip):

ah yes you were probably working on flag support :)

lqd (Sep 18 2018 at 20:23, on Zulip):

haha nice

lqd (Sep 18 2018 at 20:23, on Zulip):

@Pietro Albini nicely done :thumbs_up:

Last update: Nov 21 2019 at 23:25UTC