Stream: t-compiler

Topic: weekly meeting 2019-05-09 #54818


nikomatsakis (May 09 2019 at 14:01, on Zulip):

Hello @T-compiler/meeting! Triage meeting starting now. You can find the pre-triage topic here -- mostly I assigned priorities to nominated things.

nikomatsakis (May 09 2019 at 14:02, on Zulip):

Let me start by highlighting some issues for which we could use volunteers :hand: , or where I would like some feedback

nikomatsakis (May 09 2019 at 14:03, on Zulip):

I think that @eddyb had a prety specific proposal on this that seemed actionable. However, I can't find those comments right now. =) It's currently p-medium, but it's blocking a p-high regression that seems similar.

nikomatsakis (May 09 2019 at 14:03, on Zulip):

This is an incremental panic that a few people have highlighted. It looks like it's "close" to have a clear reproduction, but not quite. It'd be awesome if somebody could try and make a clear reproduction (i.e., compile this, apply this patch, compile again).

nikomatsakis (May 09 2019 at 14:05, on Zulip):

A regression from 1.30 that @**centril** has helpfully reduced. Looks like it is somewhere around HIR lowering or something? A bit hard to tell.

nikomatsakis (May 09 2019 at 14:05, on Zulip):

This causes crashes on windows and may be windows specific. Something in the runtime behavior of TypeMap seems to have changed. The work here would be to try and narrow down the problem.

nikomatsakis (May 09 2019 at 14:06, on Zulip):

OK, that's all I have in my notes, but there are also a large number of unassigned P-high issues.

nikomatsakis (May 09 2019 at 14:06, on Zulip):

Before we look at that, I wanted to start by looking at a beta/stable nomination

nikomatsakis (May 09 2019 at 14:07, on Zulip):

save-analysis: Fix ICE when processing associated constant #60649

nikomatsakis (May 09 2019 at 14:07, on Zulip):

This is a PR by @Igor Matuszewski that, I believe, seems to fix some important RLS regressions. I've not created a poll yet, I see there were some comments by @oli.

nikomatsakis (May 09 2019 at 14:08, on Zulip):

Actually, based on comments in the PR, I guess it's mostly 'beta nominated', since a point release is unlikely.

nikomatsakis (May 09 2019 at 14:08, on Zulip):

/poll Should we backport #60649?

Wesley Wiser (May 09 2019 at 14:09, on Zulip):

(Do we have any mobile users for whom the poll will not show)

oli (May 09 2019 at 14:09, on Zulip):

I don't grok save_analysis and don't have time to dig in deeper as to why my ideas don't fit. The change fixes the panic and it's semi-clear to me why it does, so my comments are resolved

nikomatsakis (May 09 2019 at 14:09, on Zulip):

feel free to use either emoji or poll

nikomatsakis (May 09 2019 at 14:09, on Zulip):

or whatever :)

nikomatsakis (May 09 2019 at 14:09, on Zulip):

it's just to get a rough sense anyway

nikomatsakis (May 09 2019 at 14:10, on Zulip):

OK, i'm going to mark that as beta-accepted

nikomatsakis (May 09 2019 at 14:11, on Zulip):

OK, so, let's see -- we have a bunch of P-high issues, and we have I-nominated issues.

nikomatsakis (May 09 2019 at 14:12, on Zulip):

Let's do I-nominated

nikomatsakis (May 09 2019 at 14:12, on Zulip):

Hide type errors likely caused by incorrect struct literal #60118

I think we resolved this, right? At least, we would want to do more design here? (cc @Esteban Küber)

nikomatsakis (May 09 2019 at 14:13, on Zulip):

add support for unchecked math #59148

nikomatsakis (May 09 2019 at 14:13, on Zulip):

the proposal is to add various intrinsics for arithmetic where it is UB to overflow, to aid in optimization

nikomatsakis (May 09 2019 at 14:13, on Zulip):

lang team says "experiment away but do not expose publicly"

Esteban Küber (May 09 2019 at 14:14, on Zulip):

I can close #60118

nikomatsakis (May 09 2019 at 14:14, on Zulip):

I'm inclined to say "compiler team says this is fine" -- it's not like it's a real maintenance burden for us?

nikomatsakis (May 09 2019 at 14:14, on Zulip):

(it's basically exposing some LLVM intrinsics)

nikomatsakis (May 09 2019 at 14:14, on Zulip):

I'm going to go with that and mark this as unblocked unless anyone objects or thinks we should discuss it more broadly

nikomatsakis (May 09 2019 at 14:15, on Zulip):

OK there are a lot of I-nominated issues

nikomatsakis (May 09 2019 at 14:15, on Zulip):

@Zoxc you want to start with the one you highlighted?

nikomatsakis (May 09 2019 at 14:16, on Zulip):

I actually forget what that was

nikomatsakis (May 09 2019 at 14:16, on Zulip):

Turn HIR indexing into a query #59064

nikomatsakis (May 09 2019 at 14:17, on Zulip):

My sense is that this might be a good candidate for the design meeting =)

mw (May 09 2019 at 14:17, on Zulip):

I might have nominated that?

mw (May 09 2019 at 14:18, on Zulip):

and yes, I think this is something that should be designed beforehand

mw (May 09 2019 at 14:18, on Zulip):

and agreed upon

nikomatsakis (May 09 2019 at 14:18, on Zulip):

How do we deal with a situation like this one where one member wants to work on something complicated with long-term effects but there is not enough bandwidth in the rest of the team to come up with (or at least think through and review) a proposed architecture/design?

nikomatsakis (May 09 2019 at 14:18, on Zulip):

that seems like context

mw (May 09 2019 at 14:18, on Zulip):

the PR, IIRC, is part of a series of PRs that do refactorings towards end-to-end queries

nikomatsakis (May 09 2019 at 14:18, on Zulip):

Yeah

mw (May 09 2019 at 14:19, on Zulip):

and there were a few things in this PR and the followups that would do non-trivial stuff around the query system

Zoxc (May 09 2019 at 14:19, on Zulip):

I don't think that PR should be discussed at the design meeting though, it's quite straightforward

nikomatsakis (May 09 2019 at 14:19, on Zulip):

I am inclined to agree, I feel like this is exactly the sort of thing I had hope we would discuss in design meetings beforehand -- to try and get buy in on the general direction, and to help reviewers so they have an ide what the PR is trying to do

nikomatsakis (May 09 2019 at 14:20, on Zulip):

I don't think that PR should be discussed at the design meeting though, it's quite straightforward

I dont' recall the specific PR, I was more referring to the overall plan

nikomatsakis (May 09 2019 at 14:20, on Zulip):

But I guess there isn't overall agreement on it being straightforward based on @mw's comments :)

mw (May 09 2019 at 14:21, on Zulip):

Here are my concerns: https://github.com/rust-lang/rust/pull/59064#issuecomment-478565733

mw (May 09 2019 at 14:21, on Zulip):

and the subsequent PR looked like they did similar things

nikomatsakis (May 09 2019 at 14:22, on Zulip):

So I think two things

nikomatsakis (May 09 2019 at 14:25, on Zulip):

OK, well, it seems like the comments have been made.

nikomatsakis (May 09 2019 at 14:27, on Zulip):

I still feel like I would like to have a design meeting discussion laying out the overall plan that we are shooting for, and some of the intermediate steps. I don't know how much detail I want, perhaps not a ton, but it seems like something we can discuss. I also think we should try to figure out who will be the right person to do the reviewing, especially as @mw will be doing on some further parental leave over the summer. I do think the PR seems like "the sort of step" that we are going to need though.

nikomatsakis (May 09 2019 at 14:28, on Zulip):

(Put another way, I think this end-to-end work should probably be part of a working group, even if it's mostly a working group of 1?)

oli (May 09 2019 at 14:28, on Zulip):

I've been reviewing some of the PRs, I don't think we need an @mw level reviewer for the PRs if the general direction is agreed upon

nikomatsakis (May 09 2019 at 14:28, on Zulip):

Anyway, we gotta move on. I'll leave some comments and a pointer to what was said.

mw (May 09 2019 at 14:28, on Zulip):

If we do something like this, there should be clear documentation on why it's done, what the pitfalls are and a way forward to remove the hack again, if possible

nikomatsakis (May 09 2019 at 14:28, on Zulip):

I've been reviewing some of the PRs, I don't think we need an @mw level reviewer for the PRs if the general direction is agreed upon

(what does "mw level" mean?)

oli (May 09 2019 at 14:29, on Zulip):

well... deep knowledge of our incremental system

nikomatsakis (May 09 2019 at 14:29, on Zulip):

I mostly mean that we should decide who will do it -- I think @oli I'd be quite happy for you to do it :)

nikomatsakis (May 09 2019 at 14:29, on Zulip):

Ah, ok

nikomatsakis (May 09 2019 at 14:31, on Zulip):

Left my comment

nikomatsakis (May 09 2019 at 14:33, on Zulip):

hmm, so we're at 32 minutes --

nikomatsakis (May 09 2019 at 14:33, on Zulip):

I'm thinking we should try to spend more time on nominated or p-high issues

nikomatsakis (May 09 2019 at 14:33, on Zulip):

until 45 after

nikomatsakis (May 09 2019 at 14:33, on Zulip):

Implement converting an AST to a token tree #43081

nikomatsakis (May 09 2019 at 14:33, on Zulip):

@eddyb are you around?

nikomatsakis (May 09 2019 at 14:34, on Zulip):

I think I will remove the nomination for this, but it seems like there is still ongoing design, and might make a great design meeting candidate too in time

centril (May 09 2019 at 14:34, on Zulip):

(pinged Eddy on Discord)

nikomatsakis (May 09 2019 at 14:35, on Zulip):

Pattern errors are too imprecise and should be removed in favor of MIR borrowck #45600

nikomatsakis (May 09 2019 at 14:35, on Zulip):

this was nominated by @Esteban Küber but I suspect we can remove that

nikomatsakis (May 09 2019 at 14:35, on Zulip):

in particular, there's been some discussion in the meantime

nikomatsakis (May 09 2019 at 14:36, on Zulip):

(actually maybe we'll touch on that today as part of @WG-nll check-in)

centril (May 09 2019 at 14:36, on Zulip):

(I thought you said WG-nll was over?)

nikomatsakis (May 09 2019 at 14:36, on Zulip):

well, it's still on the check-in calendar :P

nikomatsakis (May 09 2019 at 14:36, on Zulip):

/me is ruled by the calendar

nikomatsakis (May 09 2019 at 14:37, on Zulip):

(but I agree it's basically over at this point, and was going to raise that point)

nikomatsakis (May 09 2019 at 14:37, on Zulip):

Panics in destructors can cause the return value to be leaked #47949

nikomatsakis (May 09 2019 at 14:37, on Zulip):

I guess this is more nomated for lang; it's a longstanding "weird case"

nikomatsakis (May 09 2019 at 14:37, on Zulip):

(since 1.0)

Esteban Küber (May 09 2019 at 14:37, on Zulip):

I think the biggest discussion is wether we want to remove that pattern checker in lieu of relying on the borrow checker and if we do, does that require an rfc because we'd start accepting more code.

centril (May 09 2019 at 14:37, on Zulip):

@Esteban Küber that's a T-Lang discussion however

nikomatsakis (May 09 2019 at 14:37, on Zulip):

I think the biggest discussion is wether we want to remove that pattern checker in lieu of relying on the borrow checker and if we do, does that require an rfc because we'd start accepting more code.

ah, ok. I feel like we always "intended to" as part of NLL but it wasn't explicitly discussed as such

nikomatsakis (May 09 2019 at 14:37, on Zulip):

in fact, I think we had an RFC about it maybe? I can't remember

nikomatsakis (May 09 2019 at 14:38, on Zulip):

in any case, we did cover this a bit in T-lang ..

centril (May 09 2019 at 14:38, on Zulip):

yeah; bind by move & stuff

centril (May 09 2019 at 14:38, on Zulip):

Matthew helpfully made a preliminary report

nikomatsakis (May 09 2019 at 14:38, on Zulip):

Exponential compile-time and type_length_limit blowup when nesting closure wrappers #54540

nikomatsakis (May 09 2019 at 14:38, on Zulip):

I nominated this

nikomatsakis (May 09 2019 at 14:38, on Zulip):

mostly beacuse @eddyb and I were talking about it last time

nikomatsakis (May 09 2019 at 14:39, on Zulip):

it's more of a "volunteer task" than anything I guess

nikomatsakis (May 09 2019 at 14:39, on Zulip):

I'm going to remove nomination

nikomatsakis (May 09 2019 at 14:39, on Zulip):

Update dependency versions in the various Cargo.tomls to the version that we actually use #57443

nikomatsakis (May 09 2019 at 14:39, on Zulip):

@oli nominated this

nikomatsakis (May 09 2019 at 14:39, on Zulip):

Nominating for discussion at the team meeting. Let's create a repo for this. It's a general purpose tool that we want to use in rustbuild. As per this year's strategy of moving things out of tree, I want to start this out of tree. I can take care of everything once we have a repo where I have merge or r+ powers.

oli (May 09 2019 at 14:39, on Zulip):

oh yea, so the thing is less the impl, but whether we are ok to create a new repo and start out of tree

nikomatsakis (May 09 2019 at 14:40, on Zulip):

works for me

nikomatsakis (May 09 2019 at 14:40, on Zulip):

we did have a sketch of a process on this

nikomatsakis (May 09 2019 at 14:40, on Zulip):

but it was pretty informal, maybe a PR or something..?

centril (May 09 2019 at 14:40, on Zulip):

An idea: have a bot tell us when a dependency is outdated by making an issue

oli (May 09 2019 at 14:40, on Zulip):

hmm.. I'll search around and if I don't find anything... I'll add a process thing to the compiler-team repo?

nikomatsakis (May 09 2019 at 14:41, on Zulip):

The procedure is described here

nikomatsakis (May 09 2019 at 14:41, on Zulip):

Create a PR modifying this document to include the crate in the list below. Use @rfcbot merge to gain agreement from compiler team members.

nikomatsakis (May 09 2019 at 14:41, on Zulip):

it's a bit .. hidden in that document though

oli (May 09 2019 at 14:41, on Zulip):

cool... I should have looked around

nikomatsakis (May 09 2019 at 14:41, on Zulip):

took me a bit to find it

nikomatsakis (May 09 2019 at 14:41, on Zulip):

maybe should be made more prominent :)

oli (May 09 2019 at 14:42, on Zulip):

ok, that is resolved then. I'll start the process and try to make the doc more visible

nikomatsakis (May 09 2019 at 14:42, on Zulip):

+1

nikomatsakis (May 09 2019 at 14:43, on Zulip):

mir-opt tests extremely slow. #58485

nikomatsakis (May 09 2019 at 14:43, on Zulip):

I don't know why this is nominated

nikomatsakis (May 09 2019 at 14:43, on Zulip):

seems like it's p-medium, nice to have, etc?

nikomatsakis (May 09 2019 at 14:44, on Zulip):

Ah, I guess, "Nominating for discussion (specifically, investigating this) at the next @rust-lang/compiler team meeting.", but I think we're all set

nikomatsakis (May 09 2019 at 14:44, on Zulip):

Oh, I guess it was whether to port to use ui tests ? (I think that seems fine...)

nikomatsakis (May 09 2019 at 14:45, on Zulip):

(If more discussion is needed, we can use the design meeting for that)

nikomatsakis (May 09 2019 at 14:45, on Zulip):

OK, that's all the time we've got

nikomatsakis (May 09 2019 at 14:45, on Zulip):

Let's do some quick check-ins. First,

WG-Diagnostics

@oli was going to tell us about @WG-diagnostics

oli (May 09 2019 at 14:46, on Zulip):

so I just checked, and we have not done anything towards our goal of ripping diagnostics out of the compiler that I know of. Maybe @Esteban Küber knows more.

oli (May 09 2019 at 14:46, on Zulip):

We haven't had a first meeting yet, so I'll organize that

nikomatsakis (May 09 2019 at 14:46, on Zulip):

OK =)

nikomatsakis (May 09 2019 at 14:47, on Zulip):

Sounds good!

nikomatsakis (May 09 2019 at 14:47, on Zulip):

Next was @WG-nll -- there remain a few items of follow-up:

nikomatsakis (May 09 2019 at 14:48, on Zulip):

(There is no more regular meeting etc though)

nikomatsakis (May 09 2019 at 14:48, on Zulip):

To be quite honest I've been too busy to keep up with those items, but I don't think @Matthew Jasper is here right now to fill us in

nikomatsakis (May 09 2019 at 14:48, on Zulip):

Er, well, https://github.com/rust-lang/rust/pull/59114 mergd -- so I guess we have migration mode on 2015

nikomatsakis (May 09 2019 at 14:48, on Zulip):

:tada:

centril (May 09 2019 at 14:49, on Zulip):

we do; NLL is on 2015 in 1.36

nikomatsakis (May 09 2019 at 14:49, on Zulip):

So then the question now is to establish the precise criteria for the next step, killing the old AST borrow checker.

varkor (May 09 2019 at 14:49, on Zulip):

if everything is using NLL now, is there any reason to wait?

nikomatsakis (May 09 2019 at 14:49, on Zulip):

We should probably create a tracking issue for this (or maybe a GH project)

nikomatsakis (May 09 2019 at 14:49, on Zulip):

if everything is using NLL now, is there any reason to wait?

yes, because migation mode gives warnings for old patterns that used to be (incorrectly) accepted

varkor (May 09 2019 at 14:50, on Zulip):

ah, of course

nikomatsakis (May 09 2019 at 14:50, on Zulip):

we'll have to give people time to fix those, do crater runs, and all that

centril (May 09 2019 at 14:50, on Zulip):

@nikomatsakis next step might be to do crater then?

nikomatsakis (May 09 2019 at 14:50, on Zulip):

there may also be some outstanding bugs we wanted to fix

nikomatsakis (May 09 2019 at 14:50, on Zulip):

seems like we need to "take stock" of the full set of items

centril (May 09 2019 at 14:50, on Zulip):

(this is where I remind y'all of the virtues of minimum lint levels ^,-)

nikomatsakis (May 09 2019 at 14:51, on Zulip):

OK, well, that's all there is to say on that, I think.

nikomatsakis (May 09 2019 at 14:52, on Zulip):

We've got a few minutes left...

nikomatsakis (May 09 2019 at 14:53, on Zulip):

I'm going to try to get through a few I-nominated issues dang it

nikomatsakis (May 09 2019 at 14:53, on Zulip):

Decouple nightly RLS from Clippy #59761

mw (May 09 2019 at 14:54, on Zulip):

I'd like to quickly discuss https://github.com/rust-lang/rust/pull/59752 if possible

nikomatsakis (May 09 2019 at 14:54, on Zulip):

This was nominated by @kennytm -- but I'm not sure what there is to discuss. Maybe it was nominated for dev-tools though

nikomatsakis (May 09 2019 at 14:54, on Zulip):

Limit dylib symbols #59752

nikomatsakis (May 09 2019 at 14:54, on Zulip):

Go for it :)

mw (May 09 2019 at 14:54, on Zulip):

here's a summary: https://github.com/rust-lang/rust/pull/59752#issuecomment-490020239

nikomatsakis (May 09 2019 at 14:55, on Zulip):

This PR "fixes" how we export symbols from Rust dylibs by giving the linker an explicit list of symbols to export. I'm writing "fixes" in quotes here because:

we actually don't define what symbols get exported from Rust dylibs exactly
the current behavior is inconsistent between different platforms
there's at least one case that relies on the more lenient behavior on some platforms.

So the PR makes things more consistent but breaks something that relies on underspecified behavior. I'm wondering what to do here. Crater doesn't show breakage for the change, the problematic case seems to have a proposed fix upstream and Rust dylibs aren't used that much in the wild anyway. On the other hand this isn't an urgent problem and the PR might introduce breakage.

Ideally we'd fix the situation around Rust dylibs (by either deprecating them or specifying them more completely).

Meanwhile, do we want to merge this?

mw (May 09 2019 at 14:55, on Zulip):

the PR makes Rust dylibs more consistent across platforms

mw (May 09 2019 at 14:55, on Zulip):

it breaks a bit of stuff

nikomatsakis (May 09 2019 at 14:55, on Zulip):

cc @Alex Crichton

mw (May 09 2019 at 14:55, on Zulip):

but that relies on things that we don't really guarantee

mw (May 09 2019 at 14:56, on Zulip):

which is easy because Rust dylibs are rather shaky to begin with

nikomatsakis (May 09 2019 at 14:56, on Zulip):

@Alex Crichton wrote:

To throw in my personal opinion, I'm all for locking down the dylib crate type to basically "work for rustc as priority number 1 and we'll figure other things out later". Other use cases of dynamic libraries in Rust should all be using cdylib which is much more appropriate and has better definitions about symbols and whatnot. For dylib we've largely (AFAIK) just kept it working for the compiler and haven't really done much else to it.

kennytm (May 09 2019 at 14:56, on Zulip):

This was nominated by kennytm -- but I'm not sure what there is to discuss. Maybe it was nominated for dev-tools though

(59761 was nominated for T-infra when that issue still had that label)

mw (May 09 2019 at 14:56, on Zulip):

I'd like to merge

nikomatsakis (May 09 2019 at 14:56, on Zulip):

Have we reached out to the case that we know to be affected?

mw (May 09 2019 at 14:56, on Zulip):

but there might be breakage so I wanted to at least inform the compiler team

mw (May 09 2019 at 14:56, on Zulip):

yes, we have

mw (May 09 2019 at 14:56, on Zulip):

there's a PR open to fix the issue, I think

nikomatsakis (May 09 2019 at 14:56, on Zulip):

OK, I am ok to merge. I wonder where we would document this sort of thing.

mw (May 09 2019 at 14:57, on Zulip):

document what exactly?

nikomatsakis (May 09 2019 at 14:58, on Zulip):

what symbols etc are expected to be exported

nikomatsakis (May 09 2019 at 14:58, on Zulip):

I feel like this is an area where I would like input -- I see Josh Tripplet sort of weighed in a bit on thread

Vadim Petrochenkov (May 09 2019 at 14:59, on Zulip):

I'd rather document something in the direction of dylib being unstable, deprecated and for internal use only.

nikomatsakis (May 09 2019 at 14:59, on Zulip):

that seems fine to document for now

nikomatsakis (May 09 2019 at 15:00, on Zulip):

eventually I think we may want to support it more properly though

nikomatsakis (May 09 2019 at 15:00, on Zulip):

anyway, i'm fine with this, but I'd like to hear back from josh tripplet on his opinion

Alex Crichton (May 09 2019 at 15:00, on Zulip):

(reading some backscroll now)

nikomatsakis (May 09 2019 at 15:01, on Zulip):

I'm not entirely clear btw if this is "t-compiler" or "t-lang" or what =) it's... t-something

mw (May 09 2019 at 15:01, on Zulip):

the underlying issue is t-lang, I'd say

centril (May 09 2019 at 15:01, on Zulip):

@nikomatsakis when in doubt, say "both" :P

Alex Crichton (May 09 2019 at 15:01, on Zulip):

@mw so the main breakage is that C symbols not declared in rust don't show up, right?

Alex Crichton (May 09 2019 at 15:01, on Zulip):

or are there other forms of breakage that we know of?

mw (May 09 2019 at 15:02, on Zulip):

the crater run came back clean

Zoxc (May 09 2019 at 15:02, on Zulip):

I don't think C symbol will show up at all

mw (May 09 2019 at 15:02, on Zulip):

but this is rather platform dependent, so I don't know

Alex Crichton (May 09 2019 at 15:03, on Zulip):

C symbols need to show up at least somewhat for the compiler to work over time

mw (May 09 2019 at 15:03, on Zulip):

we only know that it broke that one crate because it's used by rustc during bootstrap, I think

Alex Crichton (May 09 2019 at 15:03, on Zulip):

e.g. if you have an #[inline] function in one dylib calling a C symbol linked into that dylib

Alex Crichton (May 09 2019 at 15:03, on Zulip):

that was just the crate trying to use LLVM symbols implicitly through the dylib we have, right?

mw (May 09 2019 at 15:03, on Zulip):

yes

Alex Crichton (May 09 2019 at 15:04, on Zulip):

ok, I just wanted to confirm

Alex Crichton (May 09 2019 at 15:04, on Zulip):

I'm still in favor...

mw (May 09 2019 at 15:04, on Zulip):

let's maybe wait until the beginning of the next cycle with merging

mw (May 09 2019 at 15:05, on Zulip):

then we should be on the safe side

Last update: Nov 21 2019 at 15:15UTC