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

[WIP] Tracking all the unsolved variables that was assigned ! type because of fallback #74535

Closed
wants to merge 6 commits into from
Closed

[WIP] Tracking all the unsolved variables that was assigned ! type because of fallback #74535

wants to merge 6 commits into from

Conversation

blitzerr
Copy link
Contributor

Addresses #66173

This is the first part. It will be followed by a commit that will separate out the live nodes and then only lint on the live nodes.

@rust-highfive
Copy link
Collaborator

r? @lcnr

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 19, 2020
@blitzerr
Copy link
Contributor Author

r? @nikomatsakis

@rust-highfive rust-highfive assigned nikomatsakis and unassigned lcnr Jul 19, 2020
@nikomatsakis nikomatsakis changed the title Tracking all the unsolved variables that was assigned ! type because of fallback [WIP] Tracking all the unsolved variables that was assigned ! type because of fallback Jul 20, 2020
@nikomatsakis
Copy link
Contributor

Thanks @blitzerr . Marking as WIP for now since it doesn't (yet) address its stated goals.

src/librustc_typeck/check/mod.rs Outdated Show resolved Hide resolved
src/librustc_typeck/check/writeback.rs Outdated Show resolved Hide resolved
@nikomatsakis
Copy link
Contributor

OK so the only failure at this point is weird-exprs.rs...

warning: resulted from diverging fallback: ()
  --> $DIR/weird-exprs.rs:40:37
   |
LL |                           if (return) {
   |  _____________________________________^
LL | |                             return
LL | |                         } else {
   | |_________________________^

I guess that this happens because the result of if return { return } else { return } winds up being diverging fallback, and the if is live on entry (but not live on exit). I'm wondering if we can tweak the output to capture this sort of case...

@crlf0710
Copy link
Member

This needs more discussions, so i'll convert this PR to draft to hide it from triaging list. Feel free to change it back when CI is green!

@crlf0710 crlf0710 marked this pull request as draft August 28, 2020 18:00
@crlf0710 crlf0710 added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 28, 2020
@bors
Copy link
Contributor

bors commented Aug 30, 2020

☔ The latest upstream changes (presumably #74862) made this pull request unmergeable. Please resolve the merge conflicts.

@nikomatsakis
Copy link
Contributor

Discussed in the rust-lang/rust meeting:

  • Conclusion was that the false warning from false-exprs.rs might be ok, but we should see if we can easily avoid it.
  • We might be able to issue warnings only for expressions that are live on exit, not on entry (@blitzerr I'll ping you with some thoughts after meeting).
  • Else we can rebase and do a crater run (but we have to change lint to deny by default).

@blitzerr
Copy link
Contributor Author

blitzerr commented Sep 8, 2020

After the latest rustc refactoring, the files have changed to:

  • compiler/rustc_typeck/src/check/mod.rs
  • compiler/rustc_typeck/src/check/writeback.rs

@Dylan-DPC-zz Dylan-DPC-zz removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 12, 2020
@bors
Copy link
Contributor

bors commented Sep 22, 2020

☔ The latest upstream changes (presumably #76906) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@bors
Copy link
Contributor

bors commented Oct 14, 2020

☔ The latest upstream changes (presumably #77926) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@blitzerr blitzerr closed this Oct 17, 2020
@blitzerr blitzerr deleted the lint-for-never-fallback branch October 17, 2020 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants