Repository navigation
NLL: need 3rd round of review comparing .stderr and .nll.stderr output #54528
Description
NLL keeps getting better and better. But so does our test suite (or at least the percentage of our test suite that is exercising --compare-mode=nll)!
So once again, we need to do another round of review of the differences between AST borrowck diagnostic output and NLL borrowck diagnostic output.
- see NLL: must review all .nll.stderr files #49862 and NLL: need a (new) round of review of deltas between .stderr and .nll.stderr output #52663 for the previous rounds of review
Review started with the .nll.stderr files in the repository as of commit f99911a Tue Oct 23 17:44:19 2018 +0000
though I am actually going to review the state of these files that results from merging in PR #55221
Each file is mapped to a "card" on the project: https://github.com/rust-lang/rust/projects/10
You can use the search function to find a card (though note that most of the cards use a path like ui/vec/vec-mut-iter-borrow.nll.stderr and the search bar look for text within a token. So you need to type the path starting from ui/... into the search bar. I may go through and add spaces in the paths to counteract this.)
Activity
pnkfelix commentedon Sep 25, 2018
triage of unassigned milestone issues at NLL meeting, assigning to self
pnkfelix commentedon Sep 26, 2018
Note: This is assigned to me, but I don't actually intend to start working on it there are fewer diagnostic changes in flight. In particular, I at least plan to address #54556 first
pnkfelix commentedon Oct 2, 2018
(This gets "all the labels" because it is actually a work item that can uncover arbitrary kinds of bugs as one goes through the exercise...)
davidtwco commentedon Oct 4, 2018
Can we avoid using the same format for the results of this review as we did the last round of review? - the large table and comments was hard to keep up-to-date and not great for discoverability. I'd suggest a bunch of issues and a "NLL Diagnostic Review" project to view them all.
pnkfelix commentedon Oct 8, 2018
@davidtwco sure I'm happy to take suggestions. The method used in the previous round of reviews ( #52663) was meant to be an improvement on what was done before that (#49862), but I'm not wedded to any particular methodology, as long as I can operate with reasonable efficiency.
In particular, if I happen to follow the methodology from #52663 for expediency's sake (e.g. because I am not familiar with this "project" feature of github), I won't object to someone translating the results into some other format.
davidtwco commentedon Oct 8, 2018
@pnkfelix In order to use projects, I think we would just need issues. The main issue I found with having one large issue with comments is that it is hard to keep it up to date and know what still needs fixed - if we have issues then they get visited when looking over all the NLL issues and can be closed by PRs.
nikomatsakis commentedon Oct 17, 2018
I'm bumping this to the release milestone -- it's not going to be an RC2 blocker, I don't think.
8 remaining items
pnkfelix commentedon Nov 9, 2018
Okay I finished the review.
As noted above, there are still work items on the cards on the project (https://github.com/rust-lang/rust/projects/10).
But the main thing I wanted to address for the Release milestone, namely the actual review task, is now dealt with.
pnkfelix commentedon Nov 9, 2018
marking as NLL-deferred to reflect the lower priority nature of going through the set of work items on the project cards.
pnkfelix commentedon Dec 5, 2018
the remaining work items do not have NLL-sound issues, so I'm removing that label from this issue.
pnkfelix commentedon Dec 21, 2018
Re-triaging for #56754. P-medium.
pnkfelix commentedon Nov 5, 2019
I'd say that since we have now actually removed the AST-borrowck, this is effectively a non-issue anymore.