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: change compare-mode=nll to use borrowck=migrate #55118

Closed
pnkfelix opened this issue Oct 16, 2018 · 2 comments · Fixed by #55134
Closed

NLL: change compare-mode=nll to use borrowck=migrate #55118

pnkfelix opened this issue Oct 16, 2018 · 2 comments · Fixed by #55134
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-complete Working towards the "valid code works" goal NLL-diagnostics Working towards the "diagnostic parity" goal NLL-sound Working towards the "invalid code does not compile" goal

Comments

@pnkfelix
Copy link
Member

We have a couple different modes that NLL can run in.

borrowck=mir is sort of the "adopt NLL whole hog"; it just runs the MIR-based borrow checker, and reports its errors as, well, errors.

borrowck=migrate is the migration plan for the 2018 edition. It runs the MIR-based borrow checker, and if it signals an error, then it runs the old AST-based borrow checker as a backup. If the code is accepted by AST-based borrow checker, then all the MIR-based errors are downgraded to warnings. This is the mode that we'll be deploying for Rust 2018.

Anyway: We are currently testing borrowck=mir for our test suite via compare-mode=nll.

The plan is to deploy borrowck=migrate.

We have barely any automated testing of borrowck=migrate.

See the problem here?


My proposal: We should change compare-mode=nll to use borrowck=migrate instead of borrowck=mir. We should make this change ASAP.

@pnkfelix
Copy link
Member Author

(The reason I hadn't proposed this earlier is that I had hoped that I would "simply" resolve issue #52979. But that turned out to be quite difficult in practice.)

@pnkfelix pnkfelix added the A-NLL Area: Non-lexical lifetimes (NLL) label Oct 16, 2018
@pnkfelix pnkfelix added this to the Edition 2018 RC 2 milestone Oct 16, 2018
@pnkfelix pnkfelix added NLL-sound Working towards the "invalid code does not compile" goal NLL-complete Working towards the "valid code works" goal NLL-diagnostics Working towards the "diagnostic parity" goal labels Oct 16, 2018
@pnkfelix
Copy link
Member Author

tagging with multiple NLL-labels because this kind of task overlaps all these buckets

bors added a commit that referenced this issue Oct 17, 2018
NLL: change compare-mode=nll to use borrowck=migrate

Fixes #55118.

This PR is split into two parts:

The first commit is a minor change that fixes a flaw in the existing `borrowck=migrate` implementation whereby a lint that was promoted to an error in the AST borrow checker would result in the same lint from the NLL borrow checker being downgraded to a warning in migrate mode. This PR fixes this by ensuring lints are exempt from buffering in the NLL borrow checker.

The second commit updates `compiletest` to make the NLL compare mode use `-Z borrowck=migrate` rather than `-Z borrowck=mir`. The third commit shows all the test output changes that result from this.

r? @pnkfelix
bors added a commit that referenced this issue Oct 17, 2018
NLL: change compare-mode=nll to use borrowck=migrate

Fixes #55118.

This PR is split into two parts:

The first commit is a minor change that fixes a flaw in the existing `borrowck=migrate` implementation whereby a lint that was promoted to an error in the AST borrow checker would result in the same lint from the NLL borrow checker being downgraded to a warning in migrate mode. This PR fixes this by ensuring lints are exempt from buffering in the NLL borrow checker.

The second commit updates `compiletest` to make the NLL compare mode use `-Z borrowck=migrate` rather than `-Z borrowck=mir`. The third commit shows all the test output changes that result from this.

r? @pnkfelix
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-complete Working towards the "valid code works" goal NLL-diagnostics Working towards the "diagnostic parity" goal NLL-sound Working towards the "invalid code does not compile" goal
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants