Update visibility of intermediate use items. by davidtwco · Pull Request #57922 · rust-lang/rust

davidtwco · 2019-01-26T19:34:55Z

Fixes#57410 and fixes#53925 and fixes#47816.

Currently, the target of a use statement will be updated with
the visibility of the use statement itself (if the use statement was
visible).

This PR ensures that if the path to the target item is via another
use statement then that intermediate use statement will also have the
visibility updated like the target. This silences incorrect
unreachable_pub lints with inactionable suggestions.

rust-highfive · 2019-01-26T19:35:05Z

r? @oli-obk

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

davidtwco · 2019-01-26T19:35:52Z

I've only taken this approach to updating the intermediate use statement as I couldn't find a way to go to it directly from the first use statement. If anyone has a better way of getting to this, I'm happy to change it.

Oh man, what a hack.
The import -> import -> import -> real definition link is lost after resolve.
The real solution is to calculate AccessLevel::Public and AccessLevel::Exported levels in resolve, while keeping calculating Reachable/ReachableFromImplTrait during the late privacy stage.
It will obviously require changing some infrastructure as well.

This solution won't work with glob imports (they don't have a name) and also doesn't respect namespaces.
It's an ok approximation, I think we can merge it after adding a couple of FIXMEs describing why it's not entirely correct.

~~Would you prefer I attempt to do that in this PR or file a separate issue so that this is followed up on?~~
Clarified by your later message.

I've added a FIXME comment.

oli-obk · 2019-01-27T13:05:29Z

r? @petrochenkov

petrochenkov · 2019-01-27T23:36:22Z

@bors r+

bors · 2019-01-27T23:36:23Z

📌 Commit b487a4b1c15bfeb0337b079038498ed1d13e5aa8 has been approved by petrochenkov

bors · 2019-01-28T00:44:28Z

⌛ Testing commit b487a4b1c15bfeb0337b079038498ed1d13e5aa8 with merge c02deb70ed6d7166c91f3aa33831d99cc1c090cd...

bors · 2019-01-28T00:46:04Z

💔 Test failed - status-appveyor

Centril · 2019-01-28T00:47:33Z

@bors retry

bors · 2019-01-28T03:21:58Z

⌛ Testing commit b487a4b1c15bfeb0337b079038498ed1d13e5aa8 with merge 3d4225349c6db6f369352904a004cd9abeef9609...

Dylan-DPC-zz · 2019-01-28T14:11:38Z

ping from triage @petrochenkov waiting for your review on the latest change.

davidtwco · 2019-01-28T14:14:03Z

@bors r=petrochenkov

bors · 2019-01-28T14:14:04Z

📌 Commit ad22de4 has been approved by petrochenkov

Update visibility of intermediate use items. Fixesrust-lang#57410 and fixesrust-lang#53925 and fixesrust-lang#47816. Currently, the target of a use statement will be updated with the visibility of the use statement itself (if the use statement was visible). This PR ensures that if the path to the target item is via another use statement then that intermediate use statement will also have the visibility updated like the target. This silences incorrect `unreachable_pub` lints with inactionable suggestions.

Centril · 2019-01-28T21:18:00Z

Failed in rollup, #57955 (comment) (same reason as before).

@bors r-

Please test locally or with try that this builds.

davidtwco · 2019-01-28T23:13:48Z

Please test locally or with try that this builds.

My bad, I had ran tests locally but didn't consider conditional code for other platforms. I'll r+ again when I'm sure it'll be fine.

Currently, the target of a use statement will be updated with the visibility of the use statement itself (if the use statement was visible). This commit ensures that if the path to the target item is via another use statement then that intermediate use statement will also have the visibility updated like the target. This silences incorrect `unreachable_pub` lints with inactionable suggestions.

davidtwco · 2019-02-02T16:41:51Z

I've ran all the tests for this on both Windows and Linux, I think there shouldn't be any issues.

petrochenkov · 2019-02-03T11:42:20Z

@bors r+

bors · 2019-02-03T11:42:21Z

📌 Commit 7102339 has been approved by petrochenkov

bors · 2019-02-03T13:35:23Z

⌛ Testing commit 7102339 with merge 42b8c77...

Update visibility of intermediate use items. Fixes#57410 and fixes#53925 and fixes#47816. Currently, the target of a use statement will be updated with the visibility of the use statement itself (if the use statement was visible). This PR ensures that if the path to the target item is via another use statement then that intermediate use statement will also have the visibility updated like the target. This silences incorrect `unreachable_pub` lints with inactionable suggestions.

bors · 2019-02-03T16:06:51Z

☀️ Test successful - checks-travis, status-appveyor
Approved by: petrochenkov
Pushing 42b8c77 to master...

rust-highfive assigned oli-obk Jan 26, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 26, 2019

davidtwco commented Jan 26, 2019
View reviewed changes

petrochenkov assigned petrochenkov and unassigned oli-obk Jan 26, 2019

This comment has been minimized.
Sign in to view

davidtwco force-pushed the issue-57410 branch from ff9a1b7 to e24431fCompare January 26, 2019 22:48

This comment has been minimized.
Sign in to view

davidtwco force-pushed the issue-57410 branch from e24431f to 4302d92Compare January 26, 2019 23:28

petrochenkov reviewed Jan 27, 2019
View reviewed changes

Comment threadsrc/librustc/middle/privacy.rs Outdated

petrochenkov reviewed Jan 27, 2019
View reviewed changes

Comment threadsrc/librustc_privacy/lib.rs Outdated

petrochenkov reviewed Jan 27, 2019
View reviewed changes

Comment threadsrc/librustc_privacy/lib.rs Outdated

petrochenkov reviewed Jan 27, 2019
View reviewed changes

Comment threadsrc/librustc_privacy/lib.rs Outdated

davidtwco force-pushed the issue-57410 branch from 4302d92 to b487a4bCompare January 27, 2019 16:04

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 Jan 27, 2019

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 28, 2019

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 Jan 28, 2019

This comment has been minimized.
Sign in to view

Dylan-DPC-zz added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 28, 2019

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 Jan 28, 2019

Centril mentioned this pull request Jan 28, 2019
Rollup of 5 pull requests #57955
Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 28, 2019

davidtwco force-pushed the issue-57410 branch from ad22de4 to 036819bCompare February 2, 2019 14:27

davidtwco force-pushed the issue-57410 branch from 036819b to 7102339Compare February 2, 2019 16:41

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 Feb 3, 2019

bors merged commit 7102339 into rust-lang:masterFeb 3, 2019

davidtwco deleted the issue-57410 branch February 4, 2019 08:06

petrochenkov mentioned this pull request Feb 5, 2019
Change privacy checks, particularly for tuple structs #58173
Closed

jyn514 mentioned this pull request Feb 28, 2020
Lint unreachable_pub got easily confused by multiple items in a single pub use ... statement #64762
Closed

petrochenkov mentioned this pull request Feb 13, 2021
Move collection of items accessible directly or through reexports from rustc_privacy to rustc_resolve#82064
Closed

Uh oh!

Uh oh!

Conversation

davidtwco commented Jan 26, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Jan 26, 2019

Uh oh!

davidtwcoJan 26, 2019

Choose a reason for hiding this comment

Uh oh!

petrochenkovJan 27, 2019

Choose a reason for hiding this comment

Uh oh!

petrochenkovJan 27, 2019

Choose a reason for hiding this comment

Uh oh!

davidtwcoJan 27, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidtwcoJan 27, 2019

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oli-obk commented Jan 27, 2019

Uh oh!

petrochenkov commented Jan 27, 2019

Uh oh!

bors commented Jan 27, 2019

Uh oh!

bors commented Jan 28, 2019

Uh oh!

bors commented Jan 28, 2019

Uh oh!

Centril commented Jan 28, 2019

Uh oh!

This comment has been minimized.

bors commented Jan 28, 2019

Uh oh!

Dylan-DPC-zz commented Jan 28, 2019

Uh oh!

davidtwco commented Jan 28, 2019

Uh oh!

bors commented Jan 28, 2019

Uh oh!

Centril commented Jan 28, 2019

Uh oh!

davidtwco commented Jan 28, 2019

Uh oh!

davidtwco commented Feb 2, 2019

Uh oh!

petrochenkov commented Feb 3, 2019

Uh oh!

bors commented Feb 3, 2019

Uh oh!

bors commented Feb 3, 2019

Uh oh!

bors commented Feb 3, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

davidtwco commented Jan 26, 2019•
edited
Loading

davidtwcoJan 27, 2019•
edited
Loading