Stream: t-compiler

Topic: weekly meeting 2019-11-14 #54818


pnkfelix (Nov 14 2019 at 10:04, on Zulip):

Hi @T-compiler/meeting ; the triage meeting will be starting in 4 hours 56 minutes

pnkfelix (Nov 14 2019 at 10:04, on Zulip):

I will be doing pre-triage in a parallel topic

pnkfelix (Nov 14 2019 at 10:05, on Zulip):

I forgot to do the Polonius WG check-in last week, so I'll be posting that shortly. (If @lqd is around then maybe they can answer any Q's you all have.)

pnkfelix (Nov 14 2019 at 10:06, on Zulip):

the scheduled WG checkin is for RLS 2.0 and self-profile

pnkfelix (Nov 14 2019 at 10:06, on Zulip):

regarding RLS 2.0: @matklad , you around?

pnkfelix (Nov 14 2019 at 10:06, on Zulip):

(or rather: Will you be available at the time of the meeting)

pnkfelix (Nov 14 2019 at 10:07, on Zulip):

and likewise for self-profile: is @mw or @Wesley Wiser available to give an update at the meeting?

pnkfelix (Nov 14 2019 at 10:09, on Zulip):

regarding the Polonius checkin, here is @lqd 's update (hackmd):

Since the last compiler meeting WG check-in, we:

- made a lot of progress on our completeness goals with the work on move/initialization errors, and subset errors, both getting close to completion.
- fixed the last failure in the rustc test suite (but there are still the same 2 OOMs as last time, we haven't had much time to look at those yet).
- made diagnostics output match NLL in a lot more cases (there are less .polonius.stderrs in-tree).
- did some cleanup in our terminology by picking better names for our atoms (for example, origin instead of region, path instead of MovePath) hopefully making it clearer in the process, and more work is planned here.
- last time we mentioned we'd like to work on a polonius book, so we've done that. It's sparse at the moment but online, at https://rust-lang.github.io/polonius/, and more documentation work is in-flight and planned.
- the exploration and prototype on the rules offering more flow-sensitive precision for the analysis has also progressed a lot.
- also did some refactoring, and work quite a bit on performance. And since the latter can step on the other work and vice-versa, we decided to focus on completeness first, and then after that has been achieved, re-adapt and land the optimization work.

Some more news:
. - Niko has done a presentation on Polonius at RustBelt Rust. Slides are available at https://nikomatsakis.github.io/rust-belt-rust-2019/
- Albin has finished their master's thesis, and is currently rewriting most of the draft (which was available at https://rust-lang.zulipchat.com/user_uploads/4715/ufu5BGNrkzVbV8FtkK3Tco6M/Albins-Thesis-draft-version.pdf).
- we hope to have a "polonius work week" at the end of November to push the in-progress work over the finish line together.

matklad (Nov 14 2019 at 10:10, on Zulip):

Yep, will be there, will do status report!

lqd (Nov 14 2019 at 10:27, on Zulip):

I may also be around to answer questions -- depending on $dayjob work I might have to answer async. Niko will surely attend and can also answer any such questions of course

Wesley Wiser (Nov 14 2019 at 11:29, on Zulip):

I should be around for the checkin

pnkfelix (Nov 14 2019 at 12:19, on Zulip):

:construction_worker: request for assistance: Anyone want to try to look into "
Rustc panics (NoSolution): could not prove Binder(projection soup)" #65581 ?

pnkfelix (Nov 14 2019 at 12:20, on Zulip):

(might be missing normalize call. might be yet another "needs lazy-normalization" ...)

pnkfelix (Nov 14 2019 at 12:43, on Zulip):

:construction_worker: request for assistance: "Rust 1.38 regressions weren't fully triaged" #65577. There are five cases left to look at, see @Matthew Jasper 's comment here

pnkfelix (Nov 14 2019 at 13:16, on Zulip):

:construction_worker: request for assistance: "Miscompilation with target-cpu=znver1 (AMD Ryzen 1000/2000 series) on Windows + LLVM 9." #63959: @eddyb has done epic reduction. But we need someone else, with ready access to windows-development as a target (even via Wine on Linux, apparently) to pick up the torch

pnkfelix (Nov 14 2019 at 13:26, on Zulip):

:exclamation: one item on the nominations list is "Some features can no longer be controlled by conditional compilation" #65860 . I want to make sure we resolve today who and where we are going to resolve the questions here: Q1: Is it T-lang or T-compiler or both? Q2: Is asynchronous discussion sufficient (with or without an rfcbot poll), or do we need a synchronized meeting (maybe one of the Friday steering meetings)?

eddyb (Nov 14 2019 at 13:51, on Zulip):

:construction_worker: request for assistance: "Miscompilation with target-cpu=znver1 (AMD Ryzen 1000/2000 series) on Windows + LLVM 9." #63959: eddyb has done epic reduction. But we need someone else, with ready access to windows-development as a target (even via Wine on Linux, apparently) to pick up the torch

I'm not really sure about that call for action, tbh

At this point you can look at the IR/assembly on godbolt (e.g. I got this from @mati865: https://godbolt.org/z/esAhng) and try to deduce what's happening from there

I found it hard to reduce further, so unless someone has ideas they want me to try, it's probably better to dig into where in LLVM it goes wrong

Also, I haven't tested the final reproduction on wine again, it might not work now (or trigger SIGILL on my older CPU), but I think @mati865 was able to repro with cross-compilation + wine (on the right hardware), so that's likely more useful if we can streamline it

And you need a 1st generation Ryzen CPU, that's the hard part in all of this. I have access to one and I (or one of my colleagues) can help test things out on it, I just don't have the time to think about it or stare at LLVM IR

eddyb (Nov 14 2019 at 13:51, on Zulip):

(sorry for the wall of text)

eddyb (Nov 14 2019 at 13:53, on Zulip):

(arguably one thing that could be done semi-automatedly is bisect LLVM to find the point between 8.0 and 9.0 where the resulting assembly changed - I don't even think this is a high-level LLVM optimization btw. then again, maybe it was just the znver1 target CPU, or its cost tables, being added at all, so no useful information would be gained)

rkruppe (Nov 14 2019 at 13:54, on Zulip):

might be a good ICE-breakers-llvm issue, since at this point it's pretty likely to be an LLVM bug and as eddyb said further insight will most likely come from digging into LLVM

eddyb (Nov 14 2019 at 13:56, on Zulip):

something we could've done in retrospect is replace the cost tables with those from another μarch and see if that fixes it, we could've probably easily avoided getting this bug into stable without reverting the 9.0 upgrade, but I guess none of us expected it to be that far down the stack

mati865 (Nov 14 2019 at 14:12, on Zulip):

something we could've done in retrospect is replace the cost tables with those from another μarch and see if that fixes it, we could've probably easily avoided getting this bug into stable without reverting the 9.0 upgrade, but I guess none of us expected it to be that far down the stack

I think replacing cost tables would be fairly easy but that would kill the point of targetting znver1 because the binary won't be optimal.

pnkfelix (Nov 14 2019 at 14:33, on Zulip):

lets tag it as ICE-breakers-llvm, in any case

eddyb (Nov 14 2019 at 14:46, on Zulip):

@mati865 yeah but it would make -Ctarget-cpu=native work, even if people can avoid targetting znver1 themselves

pnkfelix (Nov 14 2019 at 15:03, on Zulip):

Hi @T-compiler/meeting! Add a :wave: emoji to show you're here :)

pnkfelix (Nov 14 2019 at 15:03, on Zulip):

First five minutes are for...

pnkfelix (Nov 14 2019 at 15:03, on Zulip):

Announcements

pnkfelix (Nov 14 2019 at 15:04, on Zulip):

my announcement: please do skim up above for the :construction_worker: :construction_worker: :construction_worker: who are asking for your help

eddyb (Nov 14 2019 at 15:04, on Zulip):

someone did this and it looks really easy https://github.com/rust-lang/rust/pull/66384

pnkfelix (Nov 14 2019 at 15:05, on Zulip):

Polytyptic programming for the win :banana: :glasses: :envelope:

eddyb (Nov 14 2019 at 15:05, on Zulip):

I wish I realized how straight-forward it is, maybe we should have E-easy issues for some of these?

Santiago Pastorino (Nov 14 2019 at 15:06, on Zulip):

that's very cool

Pietro Albini (Nov 14 2019 at 15:06, on Zulip):

it's finally public :tada: https://blog.rust-lang.org/inside-rust/2019/11/14/evaluating-github-actions.html

Santiago Pastorino (Nov 14 2019 at 15:06, on Zulip):

we've talked with @oli about some code around interning

Pietro Albini (Nov 14 2019 at 15:07, on Zulip):

we still need to do a lot more testing, but I hope we can get sub-2hr builds on bors with actions

eddyb (Nov 14 2019 at 15:07, on Zulip):

@Pietro Albini it feels like the move to Azure was a couple weeks ago :P

pnkfelix (Nov 14 2019 at 15:07, on Zulip):

wait, whoa

Pietro Albini (Nov 14 2019 at 15:07, on Zulip):

heh

pnkfelix (Nov 14 2019 at 15:08, on Zulip):

that's really wild

pnkfelix (Nov 14 2019 at 15:08, on Zulip):

get the exploding brain memes ready

Pietro Albini (Nov 14 2019 at 15:08, on Zulip):

the data point we have at the moment is for PR builds

Pietro Albini (Nov 14 2019 at 15:08, on Zulip):

we went from 1hr 56min to 42min

eddyb (Nov 14 2019 at 15:09, on Zulip):

wat

eddyb (Nov 14 2019 at 15:09, on Zulip):

you can't just say that without at least giving some semblance of an explanation

centril (Nov 14 2019 at 15:09, on Zulip):

explanation: 8 cores

eddyb (Nov 14 2019 at 15:09, on Zulip):

I thought there were fundamental limits to how fast we- oh

pnkfelix (Nov 14 2019 at 15:09, on Zulip):

yeah the blog post does say "increased core count"

pnkfelix (Nov 14 2019 at 15:10, on Zulip):

clearly this is a way for all of us to justify buying 8-core laptops

eddyb (Nov 14 2019 at 15:10, on Zulip):

/me slaps the roof of 24-core 48-thread AMD EPYC server

nagisa (Nov 14 2019 at 15:11, on Zulip):

/me looks at that 128 core epyc server and transmutes into a jelly

centril (Nov 14 2019 at 15:11, on Zulip):

@eddyb I have a measly i7-7700K in my build-bot NAS

pnkfelix (Nov 14 2019 at 15:11, on Zulip):

okay. If there are any announcements we/you forgot, please PM me during the meeting and I'll allocate time at the end for them

pnkfelix (Nov 14 2019 at 15:11, on Zulip):

but lets get started

eddyb (Nov 14 2019 at 15:11, on Zulip):

oh and I tricked @centril into trying this out (we've been meaning to fix it for many years now): goodbye useless <std macros> spans https://github.com/rust-lang/rust/pull/66364

eddyb (Nov 14 2019 at 15:11, on Zulip):

(sorry, got distracted by the shiny numbers above)

centril (Nov 14 2019 at 15:12, on Zulip):

@eddyb speaking of... I hope not to rebase that PR too many times ^^

pnkfelix (Nov 14 2019 at 15:12, on Zulip):

so I went through all the P-high issues this week (finally, after two weeks of failing to do so)

pnkfelix (Nov 14 2019 at 15:12, on Zulip):

we have 30 open P-high issues, 15 of which are unassigned

centril (Nov 14 2019 at 15:13, on Zulip):

yikes

pnkfelix (Nov 14 2019 at 15:13, on Zulip):

I'd assign myself to those 15, but @centril advised against that :wink:

pnkfelix (Nov 14 2019 at 15:13, on Zulip):

but in any case, if you are looking for a bug, check out the :construction_worker: :construction_worker: :construction_worker: above

pnkfelix (Nov 14 2019 at 15:14, on Zulip):

next up, beta nominations

pnkfelix (Nov 14 2019 at 15:14, on Zulip):

there are 7

pnkfelix (Nov 14 2019 at 15:14, on Zulip):

beta-nom 1/7: "Fix two OOM issues related to ConstProp" #66394

pnkfelix (Nov 14 2019 at 15:15, on Zulip):

(this is a pretty recent PR; we can wait to evaluate it next week if people like)

centril (Nov 14 2019 at 15:15, on Zulip):

you said something about waiting a week? ^--

pnkfelix (Nov 14 2019 at 15:15, on Zulip):

figured its easiest to just post 'em all

pnkfelix (Nov 14 2019 at 15:16, on Zulip):

beta-nom 2/7: "Do not ICE on trait aliases with missing obligations" #66392

pnkfelix (Nov 14 2019 at 15:16, on Zulip):

same deal for this one; it was posted last night

pnkfelix (Nov 14 2019 at 15:16, on Zulip):

beta-nom 3/7: "Do not ICE in if without else in async fn" #66391

nikomatsakis (Nov 14 2019 at 15:16, on Zulip):

Re: #66392, I don't think we need to backport for unstable features

centril (Nov 14 2019 at 15:17, on Zulip):

@nikomatsakis it ICEs without the gates

nikomatsakis (Nov 14 2019 at 15:17, on Zulip):

(then perhaps we need another test?)

centril (Nov 14 2019 at 15:17, on Zulip):

good point (cc @Esteban Küber )

pnkfelix (Nov 14 2019 at 15:17, on Zulip):

yeah lets note that on the PR itself

nikomatsakis (Nov 14 2019 at 15:17, on Zulip):

I see, it ICEs if you use the syntax even w/ feature gate disabled, you're saying?

nikomatsakis (Nov 14 2019 at 15:17, on Zulip):

still, i'm unconvinced we need to backport.

nikomatsakis (Nov 14 2019 at 15:18, on Zulip):

but I'll move to shrug :)

centril (Nov 14 2019 at 15:18, on Zulip):

I'll join in with that sentiment ^^

pnkfelix (Nov 14 2019 at 15:18, on Zulip):

I'll finish posting all teh ones that were posted last night

pnkfelix (Nov 14 2019 at 15:18, on Zulip):

beta-nom 4/7: " Fix ICE when trying to suggest Type<> instead of Type()" #66390

pnkfelix (Nov 14 2019 at 15:19, on Zulip):

(this is another one from last night. i.e. it doesn't have to be decided this week)

pnkfelix (Nov 14 2019 at 15:19, on Zulip):

beta-nom 5/7: "Do not ICE on recovery from unmet associated type bound obligation" #66388

pnkfelix (Nov 14 2019 at 15:19, on Zulip):

(this is another one from last night. i.e. it doesn't have to be decided this week)

pnkfelix (Nov 14 2019 at 15:20, on Zulip):

beta-nom 6/9: "find_deprecation: deprecation attr may be ill-formed meta." #66381

pnkfelix (Nov 14 2019 at 15:21, on Zulip):

hmm i miscounted. :)

pnkfelix (Nov 14 2019 at 15:22, on Zulip):

sed -e s/7/9/g

pnkfelix (Nov 14 2019 at 15:22, on Zulip):

okay so let me take a pause here before we go to the final three

pnkfelix (Nov 14 2019 at 15:22, on Zulip):

The question of policy is perhaps interesting

pnkfelix (Nov 14 2019 at 15:23, on Zulip):

in terms of whether to backport #66392

pnkfelix (Nov 14 2019 at 15:23, on Zulip):

I suppose my vote would be influenced by what the ICE looks like, i.e. what kind of error one sees without the feature gate in place

pnkfelix (Nov 14 2019 at 15:24, on Zulip):

bug report says "error: internal compiler error: src/librustc_typeck/astconv.rs:1216: trait_ref_to_existential called on <OnSet as Callback0> with non-dummy Self"

nikomatsakis (Nov 14 2019 at 15:24, on Zulip):

Makes sense.

centril (Nov 14 2019 at 15:24, on Zulip):

probably the same as with the gate active

nikomatsakis (Nov 14 2019 at 15:24, on Zulip):

I think given that it is such a tiny PR, we might as well backport

Esteban Küber (Nov 14 2019 at 15:24, on Zulip):

It's the same error in both cases

pnkfelix (Nov 14 2019 at 15:24, on Zulip):

is that message enough for someone to work out a workaround?

nikomatsakis (Nov 14 2019 at 15:24, on Zulip):

If it were more complex, I would feel less motivated

Esteban Küber (Nov 14 2019 at 15:24, on Zulip):

I only enabled the hate in the test to reduce the output

pnkfelix (Nov 14 2019 at 15:25, on Zulip):

I only enabled the hate in the test to reduce the output

the point is that it wasn't clear, at least from the diff, that this was a non nightly-only bug

Esteban Küber (Nov 14 2019 at 15:25, on Zulip):

That's fair

centril (Nov 14 2019 at 15:25, on Zulip):

@Esteban Küber the only point I had is that I think if we think the PR is papering over something, then we should file a FIXME issue for the fix

centril (Nov 14 2019 at 15:25, on Zulip):

and label that with F-trait_alias

Esteban Küber (Nov 14 2019 at 15:25, on Zulip):

If it was nightly only I wouldn't have nominated it though :sweat_smile:

pnkfelix (Nov 14 2019 at 15:26, on Zulip):

If it were more complex, I would feel less motivated

specifically, this is "just" replacing a bug! with a delay_span_bug!, right?

pnkfelix (Nov 14 2019 at 15:26, on Zulip):

sounds like the kind of thing we can generally feel pretty safe backporting, yeah

pnkfelix (Nov 14 2019 at 15:27, on Zulip):

so it sounds like we can approve all six of the aforementioned PR's for beta backport then?

pnkfelix (Nov 14 2019 at 15:27, on Zulip):

I'll work on that assumption until someone says otherwise

pnkfelix (Nov 14 2019 at 15:27, on Zulip):

lets move on to the final three

pnkfelix (Nov 14 2019 at 15:27, on Zulip):

beta-nom 7/9: "parser: don't use unreachable!() in fn unexpected." #66361

pnkfelix (Nov 14 2019 at 15:29, on Zulip):

seems fine, beta-accepted

pnkfelix (Nov 14 2019 at 15:30, on Zulip):

beta-nom 8/9: "async fn presence affects an unrelated error message" #66312

pnkfelix (Nov 14 2019 at 15:30, on Zulip):

oh whoopse

pnkfelix (Nov 14 2019 at 15:30, on Zulip):

I tagged an issue instead of a PR

pnkfelix (Nov 14 2019 at 15:30, on Zulip):

or rather, @Pietro Albini did

pnkfelix (Nov 14 2019 at 15:30, on Zulip):

so ... never mind? I mean, we typically don't make decisions based on issues alone

pnkfelix (Nov 14 2019 at 15:31, on Zulip):

(I could imagine eagerly determining that a beta-backport wont be necessary)

Pietro Albini (Nov 14 2019 at 15:31, on Zulip):

yeah, that was both a "oops I didn't see this was an issue" on my end and a reminder to nominate the resulting PR

centril (Nov 14 2019 at 15:31, on Zulip):

yea and I think the problem is much less bad than I anticipated after Jonas's comment

pnkfelix (Nov 14 2019 at 15:31, on Zulip):

(but it just hasn't been our process to date)

pnkfelix (Nov 14 2019 at 15:31, on Zulip):

okay

pnkfelix (Nov 14 2019 at 15:31, on Zulip):

so we'll remove the labels

pnkfelix (Nov 14 2019 at 15:32, on Zulip):

beta-nom 9/9: "Undo an assert causing an ICE until we fix the underlying problem" #66250

centril (Nov 14 2019 at 15:32, on Zulip):

@pnkfelix my original fear was that the priorities in method selection was wrong on stable so that we'd have to point release

centril (Nov 14 2019 at 15:32, on Zulip):

but that was wrong

pnkfelix (Nov 14 2019 at 15:32, on Zulip):

yep, I understand. It was a scary looking bug. still sort of is, to me.

pnkfelix (Nov 14 2019 at 15:33, on Zulip):

beta-nom 9/9: "Undo an assert causing an ICE until we fix the underlying problem" #66250

by the way: Not your best title for a PR, @oli . :wink:

pnkfelix (Nov 14 2019 at 15:34, on Zulip):

the bug being resolved here is "ICE: "Tried to access field 0 of union with 0 fields"" #65462

pnkfelix (Nov 14 2019 at 15:34, on Zulip):

of note, this is a case where the error message, while pointing out the cause, completely lacks a span

pnkfelix (Nov 14 2019 at 15:35, on Zulip):

some of the other ICE's we've discussed earlier in the meeting always seemed to have error messages that had useful content and spans no not always spans

pnkfelix (Nov 14 2019 at 15:35, on Zulip):

so anyway that's what tips me over the edge for being pro-backport for PR #66250

pnkfelix (Nov 14 2019 at 15:36, on Zulip):

okay so lets accept it for beta backport. That's all the beta-nominations

centril (Nov 14 2019 at 15:36, on Zulip):

time check: 24 minutes left

pnkfelix (Nov 14 2019 at 15:37, on Zulip):

we have a bunch of stable nominations

nikomatsakis (Nov 14 2019 at 15:37, on Zulip):

so anyway that's what tips me over the edge for being pro-backport for PR #66250

(re this, maybe we want a FIXME comment + another bug, to track the underlying problem? cc @oli)

pnkfelix (Nov 14 2019 at 15:38, on Zulip):

my suggestion: lets not do the stable nominations of PR's that were posted in the last 24 hours

pnkfelix (Nov 14 2019 at 15:38, on Zulip):

those should wait until next week

pnkfelix (Nov 14 2019 at 15:38, on Zulip):

so that leaves us with two stable nominations to look at toda

pnkfelix (Nov 14 2019 at 15:39, on Zulip):

stable-nom 1/2: "async fn presence affects an unrelated error message" #66312

pnkfelix (Nov 14 2019 at 15:39, on Zulip):

darn it

pnkfelix (Nov 14 2019 at 15:39, on Zulip):

I can't believe i fell for it again

pnkfelix (Nov 14 2019 at 15:40, on Zulip):

shoud this be tagged a regressino though?

pnkfelix (Nov 14 2019 at 15:40, on Zulip):

okay well

centril (Nov 14 2019 at 15:40, on Zulip):

probably was always "broken"?

pnkfelix (Nov 14 2019 at 15:40, on Zulip):

stable nom 2/2: "Do not ICE with a precision flag in formatting str and no format arguments" #66093

centril (Nov 14 2019 at 15:40, on Zulip):

so it's not "good" -> "bad"

pnkfelix (Nov 14 2019 at 15:40, on Zulip):

probably was always "broken"?

ah that could be

pnkfelix (Nov 14 2019 at 15:41, on Zulip):

whoops again

pnkfelix (Nov 14 2019 at 15:41, on Zulip):

#66093 was already stable-accepted

pnkfelix (Nov 14 2019 at 15:41, on Zulip):

boy I just am off my game today

pnkfelix (Nov 14 2019 at 15:42, on Zulip):

alright, well, we have five nominations

pnkfelix (Nov 14 2019 at 15:42, on Zulip):

the first of which I'm ignoring

centril (Nov 14 2019 at 15:42, on Zulip):

ctypes and rustfmt look good to discuss

pnkfelix (Nov 14 2019 at 15:42, on Zulip):

(#66406 was nominated after I did pretriage and so I missed it earlier; it just needs prioiritzation)

pnkfelix (Nov 14 2019 at 15:43, on Zulip):

so, first up: "improper_ctypes fires for &mut T, &T, *const T and *mut T (when T: Sized)" #66220

pnkfelix (Nov 14 2019 at 15:43, on Zulip):

basically, PR #65134 changed the semantics of improper_ctypes lint

pnkfelix (Nov 14 2019 at 15:43, on Zulip):

and it gets a lot more false matches now

pnkfelix (Nov 14 2019 at 15:43, on Zulip):

there were a bunch of bugs filed

nikomatsakis (Nov 14 2019 at 15:44, on Zulip):

/me (afk for a few minutes btw)

pnkfelix (Nov 14 2019 at 15:44, on Zulip):

The first thing I wanted to discuss here: I want us to have some guidelines for when such a PR can be reverted

pnkfelix (Nov 14 2019 at 15:44, on Zulip):

as in, I want compiler-team members to feel free to use some judgement and not have to wait for a rfcbot or synchronous discussion with the whole team

davidtwco (Nov 14 2019 at 15:45, on Zulip):

basically, PR #65134 changed the semantics of improper_ctypes lint

To clarify, #65134 made the lint apply to extern "C" fns as well as functions within extern "C" { } blocks. The actual things that get linted haven't changed (except for some naive handling of generic parameters, but that isn't relevant to this particular issue).

pnkfelix (Nov 14 2019 at 15:46, on Zulip):

The revert here is meant to be "okay we'll back out the code and take some time to work out a more nuanced solution"

nikomatsakis (Nov 14 2019 at 15:46, on Zulip):

I think a revert is appropriate

centril (Nov 14 2019 at 15:46, on Zulip):

We'll discuss what the lint should/n't do on the T-Lang meeting I guess?

pnkfelix (Nov 14 2019 at 15:46, on Zulip):

(as opposed to "this PR was a bad idea and should never have been written" -- that's not the message I'm intending to send here)

nikomatsakis (Nov 14 2019 at 15:46, on Zulip):

(still afk-ish)

pnkfelix (Nov 14 2019 at 15:47, on Zulip):

My current thinking was that if some reasonable number of T-compiler members (lets say 3?) all agree that a revert is prudent, then they should be able to approve it.

centril (Nov 14 2019 at 15:47, on Zulip):

@pnkfelix re. "feel free to use some judgement", I agree up to a point -- in a scenario where e.g. there was a perf regression due to a PR fixing a soundness hole then I don't think that should be reverted by anyone just so

pnkfelix (Nov 14 2019 at 15:47, on Zulip):

Does that make sense? Does anyone think this is a bad idea? (or that I'm being too conservative? Perhaps any one T-compiler member could be entrusted with good enough judgement here?)

pnkfelix (Nov 14 2019 at 15:48, on Zulip):

pnkfelix re. "feel free to use some judgement", I agree up to a point -- in a scenario where e.g. there was a perf regression due to a PR fixing a soundness hole then I don't think that should be reverted by anyone just so

yes, this is an example why I'm hesistant to say that any one person could make the call

pnkfelix (Nov 14 2019 at 15:48, on Zulip):

but I think that if you can find three people, then there is probably merit to doing a revert.

pnkfelix (Nov 14 2019 at 15:48, on Zulip):

(to be clear: I mean three people on T-compiler)

simulacrum (Nov 14 2019 at 15:48, on Zulip):

I... sort of disagree

simulacrum (Nov 14 2019 at 15:48, on Zulip):

Like, one should be enough

centril (Nov 14 2019 at 15:49, on Zulip):

(or like lang team decisions in general shouldn't be reverted by t-compiler)

simulacrum (Nov 14 2019 at 15:49, on Zulip):

Obviously we'd expect a justification comment but I would lean towards trusting folks

simulacrum (Nov 14 2019 at 15:49, on Zulip):

Plus, it seems odd to place a higher bar on reverting than approving

pnkfelix (Nov 14 2019 at 15:49, on Zulip):

I guess part of my issue is that I don't want to get into a situation where we're policing people

pnkfelix (Nov 14 2019 at 15:50, on Zulip):

i.e. if you let one person have authority to revert, and they abuse it, then we need to figure out what to do about that

centril (Nov 14 2019 at 15:50, on Zulip):

@simulacrum depends on the change; my interest here would be to bias in favor of soundness (including if reverting a PR makes something sound)

simulacrum (Nov 14 2019 at 15:51, on Zulip):

I guess that's true, but seems true regardless?

pnkfelix (Nov 14 2019 at 15:51, on Zulip):

but if the authority requires consensus amongst some small group: that can still be abused, certainly, but I guess I'd just think there'd be less questions of "well are they acting responsbly in case X, and case Z"

simulacrum (Nov 14 2019 at 15:51, on Zulip):

since, well, we'd expect people to be somewhat cautious

simulacrum (Nov 14 2019 at 15:51, on Zulip):

But I don't care too much

pnkfelix (Nov 14 2019 at 15:52, on Zulip):

@simulacrum does make a good point that its weird to say that one person has authority to r+

pnkfelix (Nov 14 2019 at 15:52, on Zulip):

but reversion requires more than that

pnkfelix (Nov 14 2019 at 15:53, on Zulip):

I guess my basis for requiring a higher bar for reversion is that I worry about negative social impact associated with backing out someone's work

centril (Nov 14 2019 at 15:53, on Zulip):

@pnkfelix guess my point is more about team decisions; anyways -- if no one objects to a revert then it's fine

pnkfelix (Nov 14 2019 at 15:53, on Zulip):

an r+ is nearly always something that has positive effect on peoples emotional well-being

pnkfelix (Nov 14 2019 at 15:53, on Zulip):

(except for those who don't like the PR landing, of course! :smiley: )

Wesley Wiser (Nov 14 2019 at 15:53, on Zulip):

I think there's too many different situations to make hard and fast rules about. However, I think in general, we should lean toward reverting PRs because that preserves the status quo and allows us to make a coordinated decision rather than just reacting to issues as they pop up.

pnkfelix (Nov 14 2019 at 15:54, on Zulip):

hmm

simulacrum (Nov 14 2019 at 15:54, on Zulip):

Right, yeah. Reverting is essentially "oh wait r- back 3 hours ago before this was approved" in my eyes

pnkfelix (Nov 14 2019 at 15:54, on Zulip):

well now we've spent more time on this topic than I expected. :smile:

Wesley Wiser (Nov 14 2019 at 15:58, on Zulip):

/me has a check-in prepared and ready to go :)

simulacrum (Nov 14 2019 at 15:58, on Zulip):

it'd be good to visit the rustfmt issue

simulacrum (Nov 14 2019 at 15:58, on Zulip):

or at least get some eyes on it, as I'd like to move ahead with it

pnkfelix (Nov 14 2019 at 15:58, on Zulip):

okay I moved the discussion of revert polciy over here: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/to.20revert.20or.20not.20to.20revert.20that.20is.20the.20question/near/180742558

centril (Nov 14 2019 at 15:58, on Zulip):

the one note I'd make re. rustfmt is that this will raise the bar on using new unstable features in rustc

pnkfelix (Nov 14 2019 at 15:59, on Zulip):

lets visit the rustfmt issue

centril (Nov 14 2019 at 15:59, on Zulip):

if they are syntactic, they now need to be supported in rustfmt as well

pnkfelix (Nov 14 2019 at 15:59, on Zulip):

nom: Enable incremental rustfmt adoption #65939

pnkfelix (Nov 14 2019 at 15:59, on Zulip):

to be honest

pnkfelix (Nov 14 2019 at 15:59, on Zulip):

I'm not ready to make a decision here

pnkfelix (Nov 14 2019 at 15:59, on Zulip):

because I need to read over it

centril (Nov 14 2019 at 15:59, on Zulip):

(but that's not a high bar)

pnkfelix (Nov 14 2019 at 15:59, on Zulip):

but I think its good to get everyone's eyeballs on it now

centril (Nov 14 2019 at 16:00, on Zulip):

wasn't a decision made in https://github.com/rust-lang/compiler-team/issues/80 ?

nikomatsakis (Nov 14 2019 at 16:00, on Zulip):

I feel like we already decided to go fwd with rustfmt a long time back -- what's being proposed here is basically an incremental impl strategy

nikomatsakis (Nov 14 2019 at 16:00, on Zulip):

Yes, that

nikomatsakis (Nov 14 2019 at 16:00, on Zulip):

I'm not sure how much "decision making" is really needed

pnkfelix (Nov 14 2019 at 16:00, on Zulip):

okay then

pnkfelix (Nov 14 2019 at 16:00, on Zulip):

we could certainly move to an async poll / rfcbot merge

nikomatsakis (Nov 14 2019 at 16:00, on Zulip):

I think we could mvoe to r+

nikomatsakis (Nov 14 2019 at 16:00, on Zulip):

we did that rfcbot merge already

centril (Nov 14 2019 at 16:01, on Zulip):

don't use rfcbot poll :P

pnkfelix (Nov 14 2019 at 16:01, on Zulip):

or just assume that the decision from compiler-team#80 carries over

centril (Nov 14 2019 at 16:01, on Zulip):

use merge instead ^^

nikomatsakis (Nov 14 2019 at 16:01, on Zulip):

basically we made a decision here not to go fwd with a meeting

pnkfelix (Nov 14 2019 at 16:01, on Zulip):

okay sounds fine to me

pnkfelix (Nov 14 2019 at 16:01, on Zulip):

if anyone has comments, post 'em on the PR

pnkfelix (Nov 14 2019 at 16:01, on Zulip):

(and I'll make sure to take a look at it too)

pnkfelix (Nov 14 2019 at 16:02, on Zulip):

so okay, maybe in the negative 2 minutes we have left

pnkfelix (Nov 14 2019 at 16:02, on Zulip):

we can get WG checkins

pnkfelix (Nov 14 2019 at 16:02, on Zulip):

@Wesley Wiser you still tee'd up?

Wesley Wiser (Nov 14 2019 at 16:02, on Zulip):

So wg-self-profile is been fairly active since the last time we checked in! I'll try to list just the major highlights:

- We've nearly completed our long standing MVP goal!
- @simulacrum has done some nice work to polish the integration with perf.rlo
- We've added tracking for all the events we're aware of that should be traced with the exception of trait selection.
- We could really use some input as to what would be helpful to track! (perhaps @nikomatsakis has a rough idea?)

- @mw has been working on some changes to the binary format we record events in.
- The new format is more compact so results in a smaller trace file and hopefully less runtime overhead.
- The new format is also more amenable to recording query keys, which is a highly requested feature.

- @wesleywiser has added some crate level docs to make getting into the code easier.

- @wesleywiser also added code to record process id, start time, and arguments to the trace file which we've started using.

- @andjo403 has been a roll with a lot of great PRs!
- We now have a dedicated tool for generating flamegraphs directly so you don't have to use the Perl scripts anymore.
- Some internal refactoring that makes adding new tools easier.
- Lots of work on the Chromium dev tools exporter:
- New option to collapse disjoint threads so it's a little more manageable
- New option to filter out small events under a configurable threshold (necessary for very large compilations)
- :tada: :tada: You can now have multiple crate compilations in the same export file. This is similar to what cargo build -Z timings can do but much more detailed.

Looking ahead, there are a few more changes we want to land related to the binary format before we will cut a new release and upgrade rustc to the latest version.

We're beginning to plan out the next thing to work on but no concrete plans yet.

(I suspect we'll focus on delivering support for recording query keys since that is so highly anticipated)

Wesley Wiser (Nov 14 2019 at 16:03, on Zulip):

what happened to my formatting?

pnkfelix (Nov 14 2019 at 16:03, on Zulip):

heh

pnkfelix (Nov 14 2019 at 16:03, on Zulip):

at least your code and emojis are preserved.

pnkfelix (Nov 14 2019 at 16:03, on Zulip):

so the the things where you want feedback are:

We could really use some input as to what would be helpful to track! (perhaps @nikomatsakis has a rough idea?)

mw (Nov 14 2019 at 16:04, on Zulip):

what happened to my formatting?

Use * instead of -

pnkfelix (Nov 14 2019 at 16:04, on Zulip):

any other feedback you want from people?

nikomatsakis (Nov 14 2019 at 16:04, on Zulip):

@Wesley Wiser I have a question

mw (Nov 14 2019 at 16:04, on Zulip):

That's a very detailed update, @Wesley Wiser! Thank you!

nikomatsakis (Nov 14 2019 at 16:04, on Zulip):

to what extentis this workflow documented? can we add it to the unstable book?

nikomatsakis (Nov 14 2019 at 16:05, on Zulip):

Also, this sounds amazing

nikomatsakis (Nov 14 2019 at 16:05, on Zulip):

I can definitely break out some time to try to help with profiling trait selection

Wesley Wiser (Nov 14 2019 at 16:05, on Zulip):

Which workflow do you mean specifically? There's already a small amount of documentation in the rustc guide.

centril (Nov 14 2019 at 16:06, on Zulip):

Btw... is there a guide to how one gets those nice graphical timings for looking at pipelining?

Wesley Wiser (Nov 14 2019 at 16:06, on Zulip):

Oh right, I also meant to post some images

nikomatsakis (Nov 14 2019 at 16:07, on Zulip):

Which workflow do you mean specifically? There's already a small amount of documentation in the rustc guide.

I think it should be in the unstable book, and pointed to, but that's a good question! Mostly I mean this:

Wesley Wiser (Nov 14 2019 at 16:07, on Zulip):

Here's a demo of the multi-crate support in the Chrome Dev Tools:

collapsed:

expanded:

nikomatsakis (Nov 14 2019 at 16:07, on Zulip):

I periodically get asked how to tell where compilation time is going

pnkfelix (Nov 14 2019 at 16:07, on Zulip):

Which workflow do you mean specifically? There's already a small amount of documentation in the rustc guide.

is this referring to this bullet: https://rust-lang.github.io/rustc-guide/profiling.html?highlight=self-profile#profiling-the-compiler ?

mw (Nov 14 2019 at 16:07, on Zulip):

we should probably create a tracking issue for documenting the most asked for workflows

nikomatsakis (Nov 14 2019 at 16:07, on Zulip):

AFAIK right now I have to go remember the github URL and point people to .md files that are buried in there

pnkfelix (Nov 14 2019 at 16:07, on Zulip):

(where the aforementioned bullet just links to measureme/summarize 's README.md ?)

nikomatsakis (Nov 14 2019 at 16:07, on Zulip):

just having a page in the rustc book (or unstable rustc book) that links to those readmes would be great

nikomatsakis (Nov 14 2019 at 16:08, on Zulip):

prob also good to link from rustc-guide

pnkfelix (Nov 14 2019 at 16:08, on Zulip):

Okay so it sounds like we want to get some links set up

nikomatsakis (Nov 14 2019 at 16:08, on Zulip):

Example: https://doc.rust-lang.org/unstable-book/compiler-flags/profile.html

pnkfelix (Nov 14 2019 at 16:08, on Zulip):

we're pretty overtime, but I do want to make sure @matklad has a chance to post their update from WG-rls-2.0

nikomatsakis (Nov 14 2019 at 16:08, on Zulip):

(Personally, I think we should put links everywhere that funnel to some central source)

nikomatsakis (Nov 14 2019 at 16:08, on Zulip):

Sorry that I'm like a broken record. I just want to help people to find this great work you're doing.

pnkfelix (Nov 14 2019 at 16:09, on Zulip):

(because I don't have a good methodolgy in place for changing the WG checkin schedul)

matklad (Nov 14 2019 at 16:09, on Zulip):

Here, here's a much shorter and worse formatted list :)

centril (Nov 14 2019 at 16:09, on Zulip):

oh hey; we're below 5000 issues again :tada:

matklad (Nov 14 2019 at 16:09, on Zulip):

And I really got to run now, but I'll catch up later!

pnkfelix (Nov 14 2019 at 16:09, on Zulip):

thanks @matklad !

pnkfelix (Nov 14 2019 at 16:10, on Zulip):

and thanks @Wesley Wiser for your awesome update

centril (Nov 14 2019 at 16:10, on Zulip):

damn; @matklad wanted to ask if y'all had plans for shipping rust-analyzer without having to build it myself

pnkfelix (Nov 14 2019 at 16:11, on Zulip):

So yeah, I have to run too. But I wanted to thank everyone in @T-compiler/meeting for joining! This was a great meeting

Wesley Wiser (Nov 14 2019 at 16:12, on Zulip):

I can definitely break out some time to try to help with profiling trait selection

@nikomatsakis That would be greatly appreciated!!

centril (Nov 14 2019 at 16:17, on Zulip):

@Esteban Küber should I r- https://github.com/rust-lang/rust/pull/66388 for now and let you implement @nikomatsakis's suggestion?

Esteban Küber (Nov 14 2019 at 16:18, on Zulip):

Sure

matklad (Nov 14 2019 at 17:27, on Zulip):

@centril opened https://rust-lang.zulipchat.com/#narrow/stream/185405-t-compiler.2Fwg-rls-2.2E0/topic/publishing.20rust-analyzer to disucss publishing

Last update: Dec 12 2019 at 01:15UTC