Hi @T-compiler/meeting ; the triage meeting will be starting in 1 hours 23 minutes
I will be doing pre-triage in a parallel topic
are either of @Alex Crichton or @nnethercote around? I was wondering if we might get a check-in from WG-pipelining today
Hi @T-compiler/meeting , lets get this meeting started!
first up, lets see if there are any announcements.
I'll start: this was already discussed in last Friday's meeting, but just so everyone knows: I'm going on parental leave for 8 weeks, starting this coming monday
@nikomatsakis and @nagisa will be running the meetings (or delegating thereof) in my absence.
I'm around now!
@Alex Crichton great! would you be prepared to give a WG-pipelining check in in say ... 40 minutes?
@Alex Crichton if you're not available then and are in fact ready now, we could do the checkin first, before the triage stuff
sure! I can check in whenever
but if you'd prefer time to prepare (by which I mean get jazzed up on some coffee), we can keep it at the end of the meeting
okay then lets aim to hear from alex in about 38 minutes
a follow-up announcement: @nikomatsakis do you want to mention your own PTO ?
(again, I know we covered this on friday, but I don't want to assume everyone here was there)
Sure! As I do most years, I'm going to be on PTO for roughly the next 4 weeks. I expect to announce a more detailed schedule, since I will likely be spending some days checking in and keeping track of things.
(But probably less than most years, where I've often done like 50% time.)
I guess most likely I would be around for some parts of the week of July 22
And I think I would be happy to commit to running the meeting that week, but @nagisa let's talk in a separate topic I guess
okay, so lets get to the standing agenda stuff
there are five P-high unassigned T-compiler issues
two of them are regressions of some kind
I would assign these to myself, but, see above announcement about going on leave.
anyway, if no one self-assigns these, I'll probably hand out auto-assignments tomorrow as one of my parting blows
I think @Esteban Küber might be a good candidate to investigate #62546
Since they do a lot of work around parse errors and recovery
okay, we have two beta-nominations
first beta-nom: "Raise the default recursion limit to 128" #62450
For this one I believe the regression with rustdoc got to beta via trains, but I never really verified that.
Seems pretty harmless.
Does anyone want to object to #62450 ? I can imagine arguments such as "slippery slope"
but mostly I am in camp "harmless"
There is an objection by IDE folks about compiler needing bounded runtime and whatnot
I think that was more of a theoretical point
can one override this value via the interface that the IDE uses ?
i.e., IDEs will need limits to ensure respnsiveness
I say let's revisit "slippery slope" if we come to this point again (i.e. raising to 256)
vs an argument against this particular change
yeah, I took the IDE point being more in response to the comment "Ideally we wouldn't have such arbitrary limits"
as "here is a reason why we need such limits"
anyway, looks like we all agree we should accept, at least in this instance.
My understanding is that we have these limits due to undecidability?
@centril (in some cases I think its more that no one wants to bother putting a better check in ?)
((but I could be wrong; I'm thinking in some cases of recursive data structures where I was/am pretty sure the matter is decidable but we rely on recursion limits))
okay, anyway, next beta nom
oh I kept thinking you meant "nom" (the crate) not "nomination"
beta nom: "Fix ICEs when
Self is used in type aliases" #62417
oh I kept thinking you meant "nom" (the crate) not "nomination"
I could write "nom'" but I bet that would be even more confusing
One byte at a time, omnomnom
beta nom: "Fix ICEs when
Selfis used in type aliases" #62417
this appears to be a reverted commit?
sorry, I was referring to @Alexander Regueiro's comments
i.e., the change was kind of reverting some commit that they felt was causing problems?
I'm just verifying
yeah, it was more or less a revert
just of a small hunk
Feels safe to backport
okay looks good then, lets backport
@nikomatsakis BTW, think you'll get time to review that PR of mine before you go on hols? specific tips on the coercion code to modify w.r.t. trait upcasting would be great too, if that doesn't distract you too much.
we have no stable backport nominations to review
@pnkfelix I believe I nominated one thing for beta backport
maybe I messed up :)
I think I neglected the T-compiler label
Emit warning when trying to use PGO in conjunction with unwinding on … #61853
I was asked if we could backport this by the FF team, since it is blocking their use of PGO
It is a pretty mild change (error -> warn)
(hmm I must have forgotten to look at all beta-nominations without team labels. I thought I did)
Feels like a pretty semantic change on the other hand
the potential danger seems high, no?
"it sometimes does not crash and will probably generate a corrupted binary"
these things sound bad to me
nothing that works today is affected, obviously.
I mean it sounds like you're arguing against the change itself
(vs the risk of backporting)
That's probably true
Yeah that's what I'm hearing
which is potentially fair, I just want to be clear :)
and thinking myself also
But if Felix is having doubts about its existence on master it seems bad to backport, no?
so if we don't backport
that gives us time to debate
but I'm going on leave; I'm in no position to support my side of argument
I hear that but I think the risk is being overblown, personally. That is, I think that people who are building with PGO may encounter some problems perhaps, but in the meantime we block the feature from being used at all
Is there a way we could give this to FF but not the world at large?
maybe that's the wrong attitude
We could stick it in a lint group that is forbid by default, I guess?
(That might be hard)
Moreover, the people driving PGO is basically FF, so they're the ones who would be encountering the problems (and fixing them)
I guess the NLL migrate borrowck currently emits (stern) warnings about things that might actually be weaponizable
so okay, I'm not going to fight this.
so > does not crash and will probably generate a corrupted binary.
does that mean it might lead to unsoundness?
let me actually put up the link with title and emojis for people to click
I guess I think my take is that, if we had a lot of people around, it seems ok, but in the meantime I think it's ok to issue a warning. Messing around with things like PGO (to me) and linker behavior is fairly low level hackery and it is possible for things to go wrong.
last beta nom: "Emit warning when trying to use PGO in conjunction with unwinding on [Windows]" #61853
@nikomatsakis but how wrong are we talking?
if it justs leads to a crash and not UB then fine
I don't know exactly
Maybe a good idea at minimum is to update the text on https://github.com/rust-lang/rust/issues/61002
there is bug connected to a old versions of GNU ld we discussed during pre-triage that one might put in a similar boat, in terms of asking "what is our obligation when it comes to linker issues"
So I'd like to be sure that this won't lead to unsoundness at least, which I would not be fine with
In the meantime, I expect mw will investigate this in more detail when they return from leave in ... 4 weeks or so
Is it possible to just move the check later in the compilation process? EricRahm says the issue is that the error is being emitted too eagerly ie even when they're not asking rustc to actually do PGO https://github.com/rust-lang/rust/issues/61002#issuecomment-500075739
It may be
but then again I assume EricRahm is fine with the current "solution" under discussino since they authored it?
We can make a decision (one way or the other) right now. Or we can say "lets delay until next week", but @nikomatsakis and I will both be absent then. Is that okay?
@Wesley Wiser so if you do "use PGO in conjunction with unwinding on Windows" and actually use PGO you'd get an error?
it seems like we should probably investigate at least a tiny bit more before we approve for backport. Is someone here willing to take point on that investigation in the absence of myself and @nikomatsakis ?
I feel a distinct lack of information to say I'm fine
@Wesley Wiser oh; well in that case if there's an error later in compilation then it cannot be a problem in terms of soundness and whatnot?
Is someone here willing to take point on that investigation in the absence of myself and @nikomatsakis ?
(if I don't see any takers on this question, then I feel pressured to make a decision, one way or the other, right now...)
If we can find an alternate fix, that's great. Maybe we can move the check and make it more precise. I'd like it if we identified who will investigate, though.
@Wesley Wiser would you like to take it on perhaps?
I can do some investigation and try to report back next week with my findings.
(or maybe @varkor since they reviewed the original PR ... )
I'll ping Eric to try and get a contact who might have more info from FF
okay, great, thanks @Wesley Wiser !
okay, we have five nominated issues but only 4 minutes before I promised @Alex Crichton a chance to speak. :)
nominated: "ICE: thread 'rustc' panicked at 'capacity overflow'" #62554
I only nominated this so people are aware I tagged it as P-medium. :)
if you think it should be P-high, speak up.
(preferably on issue)
next: "1.30 -> 1.31 dylib late-binding regression with GNU binutils 2.28 or older." #61539
this is the bug I mentioned up above with things with GNU linker versions
I nominated it to raise awareness of this stray thought
namely, the idea of using dynamic linker version detect " just to drive the emission of a diagnostic warning. Where said diagnostic tells people to take one of the aforementioned steps to work around the bug."
I wanted to take peoples temperature about that idea.
But again, please feel free to respond on ticket, we don't have to do it live here.
Well, the warning message based on just linker version would lead to false positives more often than not, but I don’t know of any other solution that’s not reenabling plt by default.
next, I nominated "Creating a recursive type with infinite size leads to internal compiler error" #61323 in hopes of finding someone other than niko to look into it
@nagisa is there particular pattern of codegen we could associate the diagnostic with, to try to lower the false positive rate?
Or is it basically going to happen all the time?
Maybe? Ultimately to trigger that particular bug a dynamic library needs to be loaded at runtime (via e.g. dlopen)
Most relevant case where this almost always happen are compiler plugins
two other nominations; "Add a "diagnostic item" scheme for lints referring to libstd items" #60966 (is, according to @eddyb , waiting on @oli , who is on vacation for another week). and "Stable rustc always panics on arm/musl" #60297 is still looking for some love
@nagisa oh yeah I even forgot how rare this is
(does anyone here have access to arm/musl? Hey @Alex Crichton , do you? want to do some rustc hacking for us?)
Heh I do not outside of qemu emulation
that seems as good a segue as any to a checkin from WG-pipelining, take it away @Alex Crichton
(10 mins left)
ok! so the general tracking issue is still at https://github.com/rust-lang/rust/issues/60988
and everything is set enough that I've been proposing stabilizing the pieces in rustc so we can stabilize the support in Cargo
and get it all out to everyone by default
that requires two sub-issues to be stabilized
one is https://github.com/rust-lang/rust/issues/60987#issuecomment-509310492, where a
--json flag is proposed to replace
--json-rendered and it takes a list of arguments of what to print
(and various configurations and whatnot)
The next is https://github.com/rust-lang/rust/issues/60419#issuecomment-502228173 which is stabilizing the fact that JSON notifications come out for artifacts when you compile rustc and turn on json messages
--json imply JSON output without any other flags?
it's worth pointing out that there is a pending FCP proposal and @Zoxc, @Esteban Küber, @nagisa, @oli, @Vadim Petrochenkov, and @varkor still pending
(because that would probably be neat)
in that latter issue in the discussion it was brought up that a
-C flag isn't really necessary and we can just infer from json output
@eddyb perhaps eventually, but seems like a separable question
but yes the main thing now is ticky boxes and I don't mind doing the legwork actually stabilizing support
long-term there's more possibilities for pipelining still, but for now this is just focused on what we have working today becoming stable
and that's all I've got for pipelining!
I guess there are two pending proposals
IMO one thing I'd be fine with is just increasing the amount of detail in the JSON output, and expect any consumers to adapt
as an alternative to increasing amounts of configurability
The second one is waiting on @Zoxc, @nagisa, @Vadim Petrochenkov, and @pnkfelix
Seems like it'd be good to get something here out the door...
-Z flags need to go through rfcbot ?
PR issue title is misleading!
yeah sorry if anything is unclear let me know and I can try to clear it up!
these were stabilization proposals on the tracking issue rather than creation of a new issue proposing stabilization
I have a separate question -- there is a rustc guide that documents how to use rustc -- not to be confused with the one that teaches you how to hack on rustc, we should fix that...
does anybody know how complete that is or whether it's maintained?
seems like a useful artifact
(I was thinking that both PGO + pipelining would be nice things to document in there)
@Alex Crichton it seems like @eddyb and @Zoxc both care to some degree about whether the flag for #60419 goes under
-C or [ ... somewhere else ...]
(https://doc.rust-lang.org/rustc/lints/levels.html should probably be moved partially to the reference)
should we try to resolve that question right here, while people are present?
I'd argue it should be called
rustc CLIor even an outright manpage :P
rustc guide seems confusing
I'm :+1: on resolving questions now if we can
@pnkfelix I prefer no flag, just add it to the JSON output by default
pnkfelix I prefer no flag, just add it to the JSON output by default
yeah I found @Alex Crichton 's response to this suggestion confusing
sorry to clarify, I'm fine with whatever, if someone feels strongly I'm happy to go that route
I have no preference myself other than to get this stabilized
why wouldn't we do what @eddyb suggests?
It has a higher risk of being a breaking change
(also relevant for other JSON additions)
just to make
--json independently testable?
if consumers of json output expect a particular structure
but we don'
don't even hav a
--json flag right now, right?
shouldn't they handle failure gracefully? I thought we've been adding stuff to it over the past couple years
I'm just saying "higher risk", I'm not saying "something will break"
/me is probably overoptimistic tbh
we do not have
--json now, no
I do think we should feel free to add to the json
I also think we should avoid bikeshedding this to death =)
oh maybe I misunderstood @eddyb 's suggestion. I took it to mean that the change would be implied by the
I also can't remember how much we've extended the json before :) at least sometimes, I think
Okay well I'm going to officially say :shrug: and check off my box, as I need to go
thanks to everyone in @T-compiler/meeting for attending!
(i do think the
--cargo=foo=bar options sounds like a good way to introduce functionality like this in the future)
I think my take on the issue is that I like @eddyb's suggestion but I would be quite happy to stabilize what's there and then introduce a "grand unified mechanism" separately.
And yes I sort of like having a "mid-way point" that lets us create explicitly temporary interfaces that can get "stabilized" separately
Unfortunatey I will never be around for the meeting due to timezones :/