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

Use revisions to track NLL test output (part 1) #95591

Merged
merged 4 commits into from
Apr 6, 2022

Conversation

jackh726
Copy link
Member

@jackh726 jackh726 commented Apr 2, 2022

The idea here is 2 fold: 1) When we eventually do make NLL default on, that PR should be systematic in "delete revisions and corresponding error annotations" 2) This allows us to look at test NLL outputs in chunks. (Though, I've opted here not to "mark" these tests. There are some tests with NLL revisions now that will be missed. I expect we do a second pass once we have all the tests with NLL revisions; these tests should be easy enough to eyeball.)

The actual review here should be "easy", but a bit tedious. I expect we should manually go through each test output and confirm it's okay.

The majority of these are either: 1) Only span change (the one I see most common is highlighting an entire function call, rather than just the function name in that call) 2) "E0308 mismatched types" -> "lifetime does not live long enough" 3) "E0495 cannot infer an appropriate lifetime for lifetime parameter" -> "lifetime does not live long enough" 4) "E0312 lifetime of reference outlives lifetime of borrowed content" -> "lifetime does not live long enough" 5) "E0759 XXX has an anonymous lifetime '_ but it needs to satisfy a 'static lifetime requirement" -> "lifetime does not live long enough" 6) "E0623 lifetime mismatch" -> "lifetime does not live long enough"

Other than the now lack of an error code, most of these look fine (with most giving more helpful suggestions now).

rfc1623 output isn't great.

cc @marmeladema if you want to look through these

Let's r? @oli-obk since you've commented on the Zulip thread ;)

@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Apr 2, 2022
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 2, 2022
@jackh726 jackh726 force-pushed the nll-revisions-1 branch 2 times, most recently from 13eab50 to 3e78fc3 Compare April 4, 2022 16:34
@rust-log-analyzer

This comment has been minimized.

@@ -1,3 +1,7 @@
// revisions: base nll
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file's stderr output files got deleted

@oli-obk
Copy link
Contributor

oli-obk commented Apr 5, 2022

r=me with issues opened for the minor diagnostics regressions

@jackh726
Copy link
Member Author

jackh726 commented Apr 5, 2022

Filed #95686 and #95687

@bors r=oli-obk rollup

@bors
Copy link
Collaborator

bors commented Apr 5, 2022

📌 Commit f881bf7 has been approved by oli-obk

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 5, 2022
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Apr 5, 2022
Use revisions to track NLL test output (part 1)

The idea here is 2 fold: 1) When we eventually do make NLL default on, that PR should be systematic in "delete revisions and corresponding error annotations" 2) This allows us to look at test NLL outputs in chunks. (Though, I've opted here not to "mark" these tests. There are some tests with NLL revisions *now* that will be missed. I expect we do a second pass once we have all the tests with NLL revisions; these tests should be easy enough to eyeball.)

The actual review here should be "easy", but a bit tedious. I expect we should manually go through each test output and confirm it's okay.

The majority of these are either: 1) Only span change (the one I see most common is highlighting an entire function call, rather than just the function name in that call) 2) "E0308 mismatched types" -> "lifetime does not live long enough" 3) "E0495 cannot infer an appropriate lifetime for lifetime parameter" -> "lifetime does not live long enough" 4) "E0312 lifetime of reference outlives lifetime of borrowed content" -> "lifetime does not live long enough" 5) "E0759 `XXX` has an anonymous lifetime `'_` but it needs to satisfy a `'static` lifetime requirement" -> "lifetime does not live long enough" 6) "E0623 lifetime mismatch" -> "lifetime does not live long enough"

Other than the now lack of an error code, most of these look fine (with most giving more helpful suggestions now).

`rfc1623` output isn't great.

cc `@marmeladema` if you want to look through these

Let's r? `@oli-obk` since you've commented on the Zulip thread ;)
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 5, 2022
Rollup of 8 pull requests

Successful merges:

 - rust-lang#88025 (ScmCredentials netbsd implementation.)
 - rust-lang#95473 (track individual proc-macro expansions in the self-profiler)
 - rust-lang#95547 (caution against ptr-to-int transmutes)
 - rust-lang#95585 (Explain why `&T` is cloned when `T` is not `Clone`)
 - rust-lang#95591 (Use revisions to track NLL test output (part 1))
 - rust-lang#95663 (diagnostics: give a special note for unsafe fn / Fn/FnOnce/FnMut)
 - rust-lang#95673 (:arrow_up: rust-analyzer)
 - rust-lang#95681 (resolve: Fix resolution of empty paths passed from rustdoc)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit cbf54fa into rust-lang:master Apr 6, 2022
@rustbot rustbot added this to the 1.62.0 milestone Apr 6, 2022
@jackh726 jackh726 deleted the nll-revisions-1 branch April 6, 2022 03:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants