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

2229: Fix diagnostic issue when using FakeReads in closures #83521

Merged
merged 3 commits into from
Apr 4, 2021

Conversation

roxelo
Copy link
Member

@roxelo roxelo commented Mar 26, 2021

This PR fixes a diagnostic issue caused by #82536. A temporary work around was used in this merged PR which involved feature gating the addition of FakeReads introduced as a result of pattern matching in closures.

The fix involves adding an optional closure DefId to ForLet and ForMatchedPlace FakeReadCauses. This DefId will only be added if a closure pattern matches a Place starting with an Upvar.

r? @nikomatsakis

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 26, 2021
@rust-log-analyzer

This comment has been minimized.

@nikomatsakis
Copy link
Contributor

@roxelo is there a test case for this diagnostic issue?

@roxelo
Copy link
Member Author

roxelo commented Mar 26, 2021

Yes, the test case that should pass without feature gate is src/test/ui/pattern/move-ref-patterns/borrowck-move-ref-pattern.rs.
It previously used to output this instead: #82536 (comment).

@nikomatsakis
Copy link
Contributor

I see, ok!

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 26, 2021

📌 Commit 4b1cccdc299002815276bc77da23df26f03ee488 has been approved by nikomatsakis

@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 Mar 26, 2021
@nikomatsakis
Copy link
Contributor

@bors delegate+

@bors
Copy link
Contributor

bors commented Mar 26, 2021

✌️ @roxelo can now approve this pull request

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@roxelo roxelo force-pushed the quick-diagnostic-fix branch 2 times, most recently from fa23062 to c3cf93a Compare March 30, 2021 00:52
@rust-log-analyzer

This comment has been minimized.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Mar 31, 2021

📌 Commit c2c76a0 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Mar 31, 2021

⌛ Testing commit c2c76a0 with merge 30a97a5c8eb214c1cbbd95ca7540e6d81cf73bc1...

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Mar 31, 2021
…=nikomatsakis

2229: Fix diagnostic issue when using FakeReads in closures

This PR fixes a diagnostic issue caused by rust-lang#82536. A temporary work around was used in this merged PR which involved feature gating the addition of FakeReads introduced as a result of pattern matching in closures.

The fix involves adding an optional closure DefId to ForLet and ForMatchedPlace FakeReadCauses. This DefId will only be added if a closure pattern matches a Place starting with an Upvar.

r? `@nikomatsakis`
@Dylan-DPC-zz
Copy link

@bors retry (yield)

@bors
Copy link
Contributor

bors commented Apr 2, 2021

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

@nikomatsakis
Copy link
Contributor

@roxelo looks like this needs another rebase :(

Also, keep in mind that you can instruct bors with r=nikomatsakis afterwards.

@bors delegate+

@bors
Copy link
Contributor

bors commented Apr 3, 2021

✌️ @roxelo can now approve this pull request

@roxelo
Copy link
Member Author

roxelo commented Apr 3, 2021

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Apr 3, 2021

📌 Commit c29dc12 has been approved by nikomatsakis

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 3, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 4, 2021
…=nikomatsakis

2229: Fix diagnostic issue when using FakeReads in closures

This PR fixes a diagnostic issue caused by rust-lang#82536. A temporary work around was used in this merged PR which involved feature gating the addition of FakeReads introduced as a result of pattern matching in closures.

The fix involves adding an optional closure DefId to ForLet and ForMatchedPlace FakeReadCauses. This DefId will only be added if a closure pattern matches a Place starting with an Upvar.

r? `@nikomatsakis`
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 4, 2021
…=nikomatsakis

2229: Fix diagnostic issue when using FakeReads in closures

This PR fixes a diagnostic issue caused by rust-lang#82536. A temporary work around was used in this merged PR which involved feature gating the addition of FakeReads introduced as a result of pattern matching in closures.

The fix involves adding an optional closure DefId to ForLet and ForMatchedPlace FakeReadCauses. This DefId will only be added if a closure pattern matches a Place starting with an Upvar.

r? ``@nikomatsakis``
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 4, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#73945 (Add an unstable --json=unused-externs flag to print unused externs)
 - rust-lang#81619 (Implement `SourceIterator` and `InPlaceIterable` for `ResultShunt`)
 - rust-lang#82726 (BTree: move blocks around in node.rs)
 - rust-lang#83521 (2229: Fix diagnostic issue when using FakeReads in closures)
 - rust-lang#83532 (Fix compiletest on FreeBSD)
 - rust-lang#83793 (rustdoc: highlight macros more efficiently)
 - rust-lang#83809 (Remove unneeded INITIAL_IDS const)
 - rust-lang#83827 (cleanup leak after test to make miri happy)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a89eab9 into rust-lang:master Apr 4, 2021
@rustbot rustbot added this to the 1.53.0 milestone Apr 4, 2021
pnkfelix added a commit to pnkfelix/rust that referenced this pull request May 21, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 26, 2021
…disjoint-fields-gate, r=nikomatsakis

 readd capture disjoint fields gate

This readds a feature gate guard that was added in PR rust-lang#83521. (Basically, there were unintended consequences to the code exposed by removing the feature gate guard.)

The root bug still remains to be resolved, as discussed in issue rust-lang#85561. This is just a band-aid suitable for a beta backport.

Cc issue rust-lang#85435

Note that the latter issue is unfixed until we backport this (or another fix) to 1.53 beta
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 27, 2021
…disjoint-fields-gate, r=nikomatsakis

 readd capture disjoint fields gate

This readds a feature gate guard that was added in PR rust-lang#83521. (Basically, there were unintended consequences to the code exposed by removing the feature gate guard.)

The root bug still remains to be resolved, as discussed in issue rust-lang#85561. This is just a band-aid suitable for a beta backport.

Cc issue rust-lang#85435

Note that the latter issue is unfixed until we backport this (or another fix) to 1.53 beta
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request May 27, 2021
…disjoint-fields-gate, r=nikomatsakis

 readd capture disjoint fields gate

This readds a feature gate guard that was added in PR rust-lang#83521. (Basically, there were unintended consequences to the code exposed by removing the feature gate guard.)

The root bug still remains to be resolved, as discussed in issue rust-lang#85561. This is just a band-aid suitable for a beta backport.

Cc issue rust-lang#85435

Note that the latter issue is unfixed until we backport this (or another fix) to 1.53 beta
pnkfelix added a commit to pnkfelix/rust that referenced this pull request Jun 1, 2021
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants