hey @WG-compiler-nll , we'll be starting our weekly "standup" meeting here in about 10 minutes!
okay; I know some people are travelling today ...
as usual, here is the NLL Triage Paper doc: https://paper.dropbox.com/doc/Non-lexical-lifetimes-NLL-Triage--APYgI~kUTrAATYrJI99crJs7Ag-Em2cJrvxQMMFWLE7lE5Be
I'll post my status update after the meeting... sorry for not being more on top of that.
(the executive summary is that I have two WIP branches for NLL)
(well, one that just went from WIP to r? and is nearly r+)
We have no I-nominated issues, that's either good or a process failure. :wink:
also we have no uncategorized issues; I did some categorization earlier today for that
We do have two issues that need milestones: https://github.com/rust-lang/rust/issues?utf8=%E2%9C%93&q=is%3Aopen+is%3Aissue+label%3AA-NLL+-label%3ANLL-deferred+-label%3ANLL-fixed-by-NLL+no%3Amilestone+
I'll start with "Compiler assertion failed: tcx.migrate_borrowck() when failing to move #[thread_local] static vars." #54797
this is assigned to me. This is sort of scary since it seems it is affecting code that is not using NLL and is not opting into the 2018 edition
(the ICE is coming from elaborate_drops' construction of a MoveData on the MIR)
((because the MoveData construction is yielding an error)
I'm not 100% sure what's up here. I tried adding some debug printouts in a local build and then proceeded to fail to replicate the ICE.
Anyway, the compiler has other ICE's, and that hasn't stopped us from shipping. I don't see this as an RC2 blocker as of now
But I'm tempted to put it on the Release milestone
are there any counterarguments to that proposal?
I have bandwidth to take on an issue or two this week (not including the type annotations with constants depending on whether @nikomatsakis handles that or not)
are there any counterarguments to that proposal?
sorry, I was busy, but I .. think that makes sense
is this a regression?
Accrording to the labels, its a stable-to-nightly regression. But I don't see how that's possible since its using a #![feature(..)]
(maybe someone is just going by version numbers rather than channel identities?)
I guess so
it looks like the bug occurs when you have a "move" from a thread-local? (which .. should probably not be allowed?)
ok, anyway, I agree it's not an NLL RC2 blocker I don't think
okay I've milestoned it accordingly
next up: "NLL doesn't check that user type annotations are well-formed in unreachable code" #54943
I think I mentioned this to @nikomatsakis earlier today in the "making plans" topic
My opinion: This definitely isn't an RC2 blocker, since unreachable code should never expose unsoundness. @nikomatsakis responded with the opinion that it isn't even a bug. :smile:
But I don't know; don't we generally check that types are well formed when they occur even in e.g. functions that are not called?
yeah, if we decided to make this a hard error, it'd obviously be best do so, but I'm not sure that it's wrong. e.g., if you have an annotation like
let x: &'static u32 = ... that also will have not give errors if the source can't live that long (and — particularly — under polonius it wouldn't really make sense for it to do so)
I guess another take on that question is: Is there a technical reason why we cannot do such a check here?
I think in the special case where the regions are all written out to named lifetimes from the signature, it's plausible to do the check
Oh, well ... I guess I was interepreting "well-formedness" as a kind of internal structural invariant of the type expression, rather than some relationship between the unreachable expression and the type?
maybe I misunderstand the issue here. I haven't looked at the test it linked
no, I mean, you're not wrong
in this case, it's true that everything you need is in the type. which is why it's borderline.
Are you concerned e.g. about checking that in
&'a &'b T that we'll ensure that
/me looking at the test now
it would have that effect, yes, but I think that's ok
it's just that — more generally — I'd like to evaluate predicates at a particular spot
Maybe lets tag with I-needs-decsion and the Release milestone ?
and there is no particular spot where this one can be evaluated
it seems sufficiently obscure that we could fix, particularly given the migration aspect
so that we know to come back to this, but we dont' stress about deciding before the RC2 deadline...
that is, I think this still errors under migration?
I don't see why it would?
The intent is that if something passes under NLL alone, then it should pass under migration...
lets just find out
(yes, it does, because region errors are not suppressed there)
I forgot about that hack
in this case it buys us time to debate and figure out the nicest fix :)
okay, next section: unassigned issues for RC2:
the first one is super super high priority: "NLL: change compare-mode=nll to use borrowck=migrate" #55118
its sort of a thankless mechanical task, I believe
namely: it should be a trivial change to
compiletest: find out where its doing
-Z borrowck=mir to support
compare-mode=nll, and change that to
And then you need to update all the tests
(which is I suspect just a matter of running
I'd volunteer but right now i'm kind of pressed for time with Rust Belt Rust prep (ps, won't be around on Friday much I suspect)
So anyway, this should be super simple. I'd do it myself but I think I want to focus my energy on my RC2 bugs...
So if we can get a volunteer for it tonight, that would be really nice
I can do that.
assigning to @davidtwco
if you need assistance, feel free to ping me. I'm probably the one to blame for most of the nightmares in
compiletest right now
next up: "nll type anntation not preserved for non-normalized projections" #54940
oh bother, I can perhaps investigate that
I suspect I know the cause
well if you're short on time right now
it doesn't perfectly overlap with my own user type annotations work
but its in the same ballpark ...
well, it's not the same level of urgency
i.e., it might be ok if I tackle it on Friday
Well I wanted to talk about that
(I think that a fix will build on https://github.com/rust-lang/rust/pull/55093 though, so that prob does want a review)
Do we want #54940 to block us turning on borrowck=migrate for 2018 edition in RC2 ?
okay, I'll get on #55093 pronto
I guess if all these other user-annotation issues are RC2 blockers, then #54940 should be one too
just seems like one of those hydra-problems: cut off one head and two more grow in its place...
my feeling is
#54570 is more imp't
for two reasons
1. it seems easier to hit
2. it is — I suspect — more invasive
and hence a "less comfortable" backport
certainly the way I've been resolving it feels more invasive
okay. that sounds reasonable
so we'll see how this week goes then w.r.t. #54940 ...
I dont' think #54940 is really a blocker personally
right. if its still open in a week, then we'll talk about it then, right?
particularly since borrowck=migrate mitigates a lot of these issues
migrate won't mitigate this though
which tbh I hadn't realized until today
hmm yes I suppose you're right
this particular error would've been reported by AST borrowck
It was only meant to mitigate NLL-complete issues. I had thought it wouldn't mitigate any NLL-sound issues, but you pointed out one counterexample earlier in the meeting. :)
well I still hope we can tackle these :)
Still, I'm happy with this state of things
but I also think we're getting down to increasingly obscure things
okay. Here's what I'm going to do: I'm going to assign both myself and @nikomatsakis to #54940
But that's only so that it shows up on my list of bugs when I look for things to tackle
My expectation is that @nikomatsakis will address it, if they have time.
okay I think that's everything I wanted to cover
Any Q's from the other participants?
I know some people are seeking work items
if it's ok with @davidtwco, I think I'll just try to tackle constants, since I suspect it's a small edit and they haven't started yet (and I'm trying to knock off things)
well that said
maybe I should investigate #54940 instead :)
(I have an hour or so right now I had planned to spend on something in this area)
maybe we should have a dedicated topic area for people who are seeking work items to post in
so that we can get a kind of asynchronous flow going there?
that's an interesting idea
/me tries to think of a funny topic line
"coders seeking bugs"
(sorry im in and out right now)
did you cover this issue @pnkfelix ? you mentioned you wanted to this morning https://github.com/rust-lang/rust/issues/55085
I debated internally about it, and then got around the matter of #55085 by mentioning it in the "making plans" topic
your edit of the diagnostic is definitely clearer
Well the problem is that I didn't just edit the diagnostic
I editted the source code itself
While we could inject spaces into the source code we prsent
we tend ... to avoid doing that when we're presenting highlights ...
clearly we should be emitting box characters like ┰ ┖
ah yea, makes sense. I will not be around til after next monday, but I could try to edit the diagnostics, run --bless and check if it makes a mess of things or not
if there is a concern of it making formatting look incredibly weird
ooops, I forgot the </sarcasm> tag
I didn't mean to imply that was the correct change here
hahah, no not the boxes
the spacing of the bracket
in the first edit, unless if that was /s too
oh, you think we would be justified in injecting the spaces?
I'm ... not as opposed to that
but there was one other bug once
where instead of presenting the source code as written (by consulting the span)
someone was presenting a reconstructed expression (or type, can't remember)
and I remember being really annoyed at the time, because I got confused by the resulting diagnostic
anyway this strikes me as something that might need broader discussion than just this WG
I doubt its always justified, for both formatting and that, but maybe it could fit somewhere? very low priority though
I don't mind experimenting with the idea
but I wouldnt mind experimenting yea
just don't be surprised if any PR you put up gets an immediate r-, a pointer to the RFC process, and perhaps a bunch of confused emojis ...
might be to present the span as immediately following the
15 | }; | - - borrowed value needs to live until here | | | `heap_ref` dropped here while still borrowed
that doesn't need any editing of the original source
and it might have the same desired end effect
oh, thats.. much more reasonable
okay, that's the sign that its close to bedtime for me and my butterfingers
thanks to everyone for coming!
oh, thats.. much more reasonable
You would think its reasonable, then you'll see cases like