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.

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

NLL: need to run the run-pass tests under compare-mode=nll #53764

Closed
42 of 43 tasks
pnkfelix opened this issue Aug 28, 2018 · 6 comments
Closed
42 of 43 tasks

NLL: need to run the run-pass tests under compare-mode=nll #53764

pnkfelix opened this issue Aug 28, 2018 · 6 comments
Assignees
@pnkfelix
Labels
A-NLL Area: Non-lexical lifetimes (NLL) P-high High priority
Milestone
Edition 2018 RC 2

Comments

@pnkfelix
Copy link
Member

pnkfelix commented Aug 28, 2018 •
edited
Loading

The NLL work has been coasting for a long time by focusing on either adding tests that opt into NLL (via #[feature(nll)]) or using compare-mode=nll to exercise the ui/ test suite.

This obviously was imperfect, since it skipped everything outside of ui/ that didn't use the explicit opt-in, so we have been missing those cases.

  • (We resolved this for compile-fail/ by porting those tests to the ui/ directory.)

At this point, we really should be exercising NLL against (at least) the run-pass test suite.

  • We might do this by hacking compiletest to have some sort of compare-mode=nll that works with the run-pass/ suite.
  • Or we might do it by "just" migrating all of the run-pass/ tests to the ui/ directory.
    • Right now, this second option is the plan of record.

Below is a task list of directories to be ported from src/test/run-pass/*/:
@pnkfelix pnkfelix added A-NLL Area: Non-lexical lifetimes (NLL) I-nominated labels Aug 28, 2018
@pnkfelix
Copy link
Member Author

nominating so that we discuss at tonight's meeting assign this to a milestone. (I.e. I want to know if this is work we should ensure is done for RC 2.)

@pnkfelix pnkfelix mentioned this issue Aug 28, 2018
Closed
49 tasks
@pnkfelix
Copy link
Member Author

Note that migrating 100% of the run-pass/ tests to the ui/ suite may not be possible until #49855 is fixed.

(But I would be entirely satisfied if we just ported 99% of the run-pass/ suite and left some small set of exceptional cases behind.)

@pnkfelix
Copy link
Member Author

whelp I failed to actually look at all the nominated issues during the meeting last night, and so I overlooked my own request to ensure this one was addressed.

Still, seems like something important that we may want to ensure is done for RC 2, or perhaps even RC...

This was referenced Aug 31, 2018
Closed
Open
Open
Merged
bors added a commit that referenced this issue Sep 3, 2018
@bors
Auto merge of #53860 - pnkfelix:issue-53764-migrate-run-pass-to-ui, r…
eb74017 
…=nikomatsakis

Migrate (some) of run-pass/ to ui

This is a step towards addressing #53764. Much still remains.

I went through a large portion of the `*.rs` files that were directly stored into `src/test/run-pass/` and moved them into various subdirectories of a newly created `src/test/ui/run-pass/`.

(yes, it would have perhaps been nice to meld it more directly with directories already in `src/test/ui/`; but the sad truth is that opens up the reality of filename collisions, and one of my short term goals for resolving #53764 is to keep the *filenames* invariant, even as their parents directories and contents are mildly revised...)
@pnkfelix
Copy link
Member Author

pnkfelix commented Sep 4, 2018

discussed at meeting. We definitely need to do this before we ship. Probably best to do it even sooner, since it would be good to know about anything it happens to catch

P-high, Edition RC 2.

@pnkfelix pnkfelix added this to the Edition RC 2 milestone Sep 4, 2018
@pnkfelix pnkfelix added P-high High priority and removed I-nominated labels Sep 4, 2018
This was referenced Sep 6, 2018
Merged
Merged
kennytm added a commit to kennytm/rust that referenced this issue Sep 7, 2018
@kennytm
Rollup merge of rust-lang#53860 - pnkfelix:issue-53764-migrate-run-pa…
3792dbd 
…ss-to-ui, r=nikomatsakis

Migrate (some) of run-pass/ to ui

This is a step towards addressing rust-lang#53764. Much still remains.

I went through a large portion of the `*.rs` files that were directly stored into `src/test/run-pass/` and moved them into various subdirectories of a newly created `src/test/ui/run-pass/`.

(yes, it would have perhaps been nice to meld it more directly with directories already in `src/test/ui/`; but the sad truth is that opens up the reality of filename collisions, and one of my short term goals for resolving rust-lang#53764 is to keep the *filenames* invariant, even as their parents directories and contents are mildly revised...)
kennytm added a commit to kennytm/rust that referenced this issue Sep 7, 2018
@kennytm
Rollup merge of rust-lang#53992 - pnkfelix:issue-53764-migrate-run-pa…
12b3826 
…ss-borrowck-to-ui, r=nikomatsakis

migrate run-pass/borrowck to ui/run-pass

Part of rust-lang#53764
kennytm added a commit to kennytm/rust that referenced this issue Sep 7, 2018
@kennytm
Rollup merge of rust-lang#53994 - pnkfelix:issue-53764-migrate-run-pa…
ba57ef1 
…ss-dirs-to-ui, r=alexcrichton

migrate run-pass/*/ to ui/run-pass

I think this is all that remains of rust-lang#53764
@pnkfelix
Copy link
Member Author

I should soon have a PR up that migrates almost the entire remaining run-pass tests.

The few exceptions are cases where e.g. there are linker warnings being generated and I am not convinced it would be useful to try to maintain such output in src/test/ui/...

@pnkfelix pnkfelix self-assigned this Sep 11, 2018
@pnkfelix pnkfelix mentioned this issue Sep 11, 2018
Closed
@pnkfelix pnkfelix mentioned this issue Sep 12, 2018
Closed
@pnkfelix
Copy link
Member Author

Update: after discussion that was prompted by issue #54047, I am going to take a new tack of turning the src/test/run-pass/ directory into a second ui/ suite (and in tandem with that, I'll move all the tests that cannot be readily ui-ified elsewhere (such as run-pass-fulldeps or something), and then I'll move everything from src/test/ui/run-pass/ back to the new ui-ified src/test/run-pass/

@pnkfelix pnkfelix mentioned this issue Sep 14, 2018
Merged
bors added a commit that referenced this issue Sep 21, 2018
@bors
Auto merge of #54223 - pnkfelix:issue-53764-uiify-run-pass, r=nikomat…
63c75d3 
…sakis

`ui`ify run-pass

This addresses the remainder of #53764 by first converting `src/test/run-pass` into another `ui`-style test suite, and then turning on `compare-mode=nll` for that new suite.

After this lands, we can address #54047 for the short term by moving all the `src/test/ui/run-pass` tests back to `src/test/run-pass`.

(Longer term, the compiler team's current (hypothetical sketch of a) plan (see [1][], [2][]) is unify all the tests by embedding these meta-properties like "// run-pass` into their headers explicitly and dropping the significance of their location on the file system.)

[1]: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/subject/weekly.20meeting.202018-09-13/near/133889370
[2]: #54047 (comment)
@bors bors closed this as completed in 2664db2 Sep 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pnkfelix pnkfelix

Labels
A-NLL Area: Non-lexical lifetimes (NLL) P-high High priority
Projects
None yet
Milestone
Edition 2018 RC 2
Development

No branches or pull requests
1 participant
@pnkfelix