Skip to content

region tests incorrectly passing with NLL enabled #56993

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.

Sign up for GitHub

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

Jump to bottom
Closed
nikomatsakis opened this issue Dec 19, 2018 · 2 comments · Fixed by #60171
Closed

region tests incorrectly passing with NLL enabled #56993

nikomatsakis opened this issue Dec 19, 2018 · 2 comments · Fixed by #60171
Assignees
@nikomatsakis @matthewjasper
Labels
A-NLL Area: Non-lexical lifetimes (NLL) E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. NLL-sound Working towards the "invalid code does not compile" goal P-medium Medium priority

Comments

@nikomatsakis
Copy link
Contributor

nikomatsakis commented Dec 19, 2018 •
edited
Loading

The following tests pass under NLL, but ought not to (in some cases, light editing is required to observe that):

To observe the problems, though, you need to run in true NLL mode (i.e., #![feature(nll)] or -Zborrowck=mir). In migration mode, the lexical borrow checker still reports the errors in question.
@nikomatsakis nikomatsakis added A-NLL Area: Non-lexical lifetimes (NLL) NLL-sound Working towards the "invalid code does not compile" goal P-medium Medium priority labels Dec 19, 2018
@nikomatsakis nikomatsakis self-assigned this Dec 19, 2018
@matthewjasper
Copy link
Contributor

matthewjasper commented Dec 22, 2018 •
edited
Loading

This appear to have regressed between nightly-2018-10-24 and nightly-2018-10-26.

#55323 looks suspicious

@matthewjasper matthewjasper mentioned this issue Jan 17, 2019
Merged
@pnkfelix pnkfelix mentioned this issue Jan 21, 2019
Closed
@matthewjasper matthewjasper self-assigned this Jan 23, 2019
bors added a commit that referenced this issue Jan 25, 2019
@bors
Auto merge of #57714 - matthewjasper:wellformed-unreachable, r=pnkfelix
0b1669d 
[NLL] Clean up handling of type annotations

* Renames (Canonical)?UserTypeAnnotation -> (Canonical)?UserType so that the name CanonicalUserTypeAnnotation is free.
* Keep the inferred type associated to user type annotations in the MIR, so that it can be compared against the annotated type, even when the annotated expression gets removed from the MIR. (#54943)
* Use the inferred type to allow infallible handling of user type projections (#57531)
* Uses revisions for the tests in #56993
* Check the types of `Unevaluated` constants with no annotations (#46702)
* Some drive-by cleanup

Closes #46702
Closes #54943
Closes #57531
Closes #57731
cc #56993 leaving this open to track the underlying issue: we are not running tests with full NLL enabled on CI at the moment

r? @nikomatsakis
@matthewjasper
Copy link
Contributor

These tests are now properly tested with NLL enabled using revisions. Once migrate mode is the default we can switch compare mode to use -Zborrowck=mir and close this issue.

@matthewjasper matthewjasper mentioned this issue Mar 14, 2019
Merged
@matthewjasper matthewjasper added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label Mar 27, 2019
bors added a commit that referenced this issue Apr 22, 2019
@bors
Auto merge of #59114 - matthewjasper:enable-migate-2015, r=pnkfelix
bf0ea60 
Enable NLL migrate mode on the 2015 edition

## What is in this PR?

* Remove the `-Zborrowck=ast` flag option from rustc.
* The default in the 2015 edition is now `-Zborrowck=migrate`.
* The 2018 edition default is unchanged: it's still `-Zborrowck=migrate`.
* Enable two-phase borrows (currently toggled via the `-Ztwo-phase-borrows` flag) on all editions.
* Remove most dead code that handled these options.
* Update tests for the above changes.

## What is *not* in this PR?

These are left for future PRs

* Use `-Zborrowck=mir` in NLL compare mode tests (#56993)
* Remove the `-Zborrowck=compare` option (#59193)
* Remove the `-Ztwo-phase-borrows` flag. It's kept, as a flag that does nothing so that perf.rlo has time to stop using it (cc @Mark-Simulacrum)
* Remove MIR typeck as its own MIR pass - it's now run by NLL.
* Enabling `-Zborrowck=mir` by default (#58781)
* Replace `allow_bind_by_move_patterns_with_guards` and `check_for_mutation_in_guard_via_ast_walk` with just using the feature gate. (#59192)

Soundness issues that are fixed by NLL will stay open until full NLL is emitting hard errors. However, these diagnostics and completeness issues can now be closed:

Closes #18330
Closes #22323
Closes #23591
Closes #26736
Closes #27487
Closes #28092
Closes #28970
Closes #29733
Closes #30104
Closes #38915
Closes #39908
Closes #43407
Closes #47524
Closes #48540
Closes #49073
Closes #52614
Closes #55085
Closes #56093
Closes #56496
Closes #57804

cc #43234

r? @pnkfelix
cc @rust-lang/lang
cc @rust-lang/wg-compiler-nll
bors added a commit that referenced this issue Apr 22, 2019
@bors
Auto merge of #59114 - matthewjasper:enable-migate-2015, r=pnkfelix
c21fbfe 
Enable NLL migrate mode on the 2015 edition

## What is in this PR?

* Remove the `-Zborrowck=ast` flag option from rustc.
* The default in the 2015 edition is now `-Zborrowck=migrate`.
* The 2018 edition default is unchanged: it's still `-Zborrowck=migrate`.
* Enable two-phase borrows (currently toggled via the `-Ztwo-phase-borrows` flag) on all editions.
* Remove most dead code that handled these options.
* Update tests for the above changes.

## What is *not* in this PR?

These are left for future PRs

* Use `-Zborrowck=mir` in NLL compare mode tests (#56993)
* Remove the `-Zborrowck=compare` option (#59193)
* Remove the `-Ztwo-phase-borrows` flag. It's kept, as a flag that does nothing so that perf.rlo has time to stop using it (cc @Mark-Simulacrum)
* Remove MIR typeck as its own MIR pass - it's now run by NLL.
* Enabling `-Zborrowck=mir` by default (#58781)
* Replace `allow_bind_by_move_patterns_with_guards` and `check_for_mutation_in_guard_via_ast_walk` with just using the feature gate. (#59192)

Soundness issues that are fixed by NLL will stay open until full NLL is emitting hard errors. However, these diagnostics and completeness issues can now be closed:

Closes #18330
Closes #22323
Closes #23591
Closes #26736
Closes #27487
Closes #28092
Closes #28970
Closes #29733
Closes #30104
Closes #38915
Closes #39908
Closes #43407
Closes #47524
Closes #48540
Closes #49073
Closes #52614
Closes #55085
Closes #56093
Closes #56496
Closes #57804

cc #43234

r? @pnkfelix
cc @rust-lang/lang
cc @rust-lang/wg-compiler-nll
@matthewjasper matthewjasper mentioned this issue Apr 22, 2019
Merged
bors added a commit that referenced this issue May 17, 2019
@bors
Auto merge of #60171 - matthewjasper:full-nll-compare-mode, r=pnkfelix
b982867 
Use -Zborrowck=mir for NLL compare mode

closes #56993

r? @pnkfelix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nikomatsakis nikomatsakis

@matthewjasper matthewjasper

Labels
A-NLL Area: Non-lexical lifetimes (NLL) E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. NLL-sound Working towards the "invalid code does not compile" goal P-medium Medium priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use -Zborrowck=mir for NLL compare mode matthewjasper/rust
2 participants
@nikomatsakis @matthewjasper