Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NLL: review uses of revisions: migrate nll that are already covered by compare-mode #55312

Closed
pnkfelix opened this issue Oct 24, 2018 · 3 comments
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-diagnostics Working towards the "diagnostic parity" goal P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@pnkfelix
Copy link
Member

This is just a work item I want to file. We can clean up our test suite slightly by removing occurrences of revisions that are made redundant by compare-mode.

(Note that there are some cases where we deliberately opt out of compare-mode and then use the revisions system to emulate it. Those can stay, though it would be good to review them too.)

@pnkfelix pnkfelix added the A-NLL Area: Non-lexical lifetimes (NLL) label Oct 24, 2018
@pnkfelix pnkfelix added NLL-diagnostics Working towards the "diagnostic parity" goal P-medium Medium priority labels Nov 6, 2018
@matthewjasper matthewjasper self-assigned this Feb 10, 2019
@matthewjasper
Copy link
Contributor

Updated this to reflect the change to migrate mode everywhere.

This is waiting for #60171 to land first.

@matthewjasper matthewjasper changed the title NLL: review uses of revisions: ast mir that are already covered by compare-mode NLL: review uses of revisions: migrate nll that are already covered by compare-mode Apr 28, 2019
@crlf0710 crlf0710 added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jun 11, 2020
@marmeladema
Copy link
Contributor

I would be happy to help if someone could write some notes about what needs to be done 🙏

@jackh726
Copy link
Member

jackh726 commented Apr 5, 2022

I'm going to go ahead and close this. As mentioned on zulip, I think it makes sense to actually do the opposite here: explicitly move tests with different NLL output to use revisions. This allows us to incrementally review test output differences and add error annotations to the test files. This lowers to burden of review of the final stabilization PR (which should just remove the revision, non-NLL error annotations, and a bless).

compare-mode=nll is good to ensure we don't miss test differences though

@jackh726 jackh726 closed this as completed Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-diagnostics Working towards the "diagnostic parity" goal P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants