Update visibility of intermediate use items. #57922

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

src/librustc_privacy/lib.rs

+                                    let item = self.tcx.hir().expect_item(item_id.id);
+                                    if item.ident != name.ident { continue; }
+                                    if let hir::ItemKind::Use(..) = item.node {
+                                        self.update(item.id, Some(AccessLevel::Exported));


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.

src/librustc/middle/privacy.rs

src/librustc_privacy/lib.rs

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. Fixes rust-lang#57410 and fixes rust-lang#53925 and fixes rust-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 e24431f Compare January 26, 2019 22:48

This comment has been minimized.

Sign in to view

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

petrochenkov reviewed Jan 27, 2019

View reviewed changes

src/librustc/middle/privacy.rs Outdated Show resolved Hide resolved

petrochenkov reviewed Jan 27, 2019

View reviewed changes

src/librustc_privacy/lib.rs Show resolved Hide resolved

petrochenkov reviewed Jan 27, 2019

View reviewed changes

src/librustc_privacy/lib.rs Outdated Show resolved Hide resolved

petrochenkov reviewed Jan 27, 2019

View reviewed changes

src/librustc_privacy/lib.rs Outdated Show resolved Hide resolved

davidtwco force-pushed the issue-57410 branch from 4302d92 to b487a4b Compare 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 036819b Compare February 2, 2019 14:27

davidtwco force-pushed the issue-57410 branch from 036819b to 7102339 Compare 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:master Feb 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

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update visibility of intermediate use items. #57922

Update visibility of intermediate use items. #57922

davidtwco commented Jan 26, 2019 •

edited

Loading

rust-highfive commented Jan 26, 2019

davidtwco Jan 26, 2019

petrochenkov Jan 27, 2019

petrochenkov Jan 27, 2019

davidtwco Jan 27, 2019 •

edited

Loading

davidtwco Jan 27, 2019

This comment has been minimized.

This comment has been minimized.

oli-obk commented Jan 27, 2019

petrochenkov commented Jan 27, 2019

bors commented Jan 27, 2019

bors commented Jan 28, 2019

bors commented Jan 28, 2019

Centril commented Jan 28, 2019

This comment has been minimized.

bors commented Jan 28, 2019

Dylan-DPC-zz commented Jan 28, 2019

davidtwco commented Jan 28, 2019

bors commented Jan 28, 2019

Centril commented Jan 28, 2019

davidtwco commented Jan 28, 2019

davidtwco commented Feb 2, 2019

petrochenkov commented Feb 3, 2019

bors commented Feb 3, 2019

bors commented Feb 3, 2019

bors commented Feb 3, 2019

Update visibility of intermediate use items. #57922

Update visibility of intermediate use items. #57922

Conversation

davidtwco commented Jan 26, 2019 • edited Loading

rust-highfive commented Jan 26, 2019

davidtwco Jan 26, 2019

Choose a reason for hiding this comment

petrochenkov Jan 27, 2019

Choose a reason for hiding this comment

petrochenkov Jan 27, 2019

Choose a reason for hiding this comment

davidtwco Jan 27, 2019 • edited Loading

Choose a reason for hiding this comment

davidtwco Jan 27, 2019

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

oli-obk commented Jan 27, 2019

petrochenkov commented Jan 27, 2019

bors commented Jan 27, 2019

bors commented Jan 28, 2019

bors commented Jan 28, 2019

Centril commented Jan 28, 2019

This comment has been minimized.

bors commented Jan 28, 2019

Dylan-DPC-zz commented Jan 28, 2019

davidtwco commented Jan 28, 2019

bors commented Jan 28, 2019

Centril commented Jan 28, 2019

davidtwco commented Jan 28, 2019

davidtwco commented Feb 2, 2019

petrochenkov commented Feb 3, 2019

bors commented Feb 3, 2019

bors commented Feb 3, 2019

bors commented Feb 3, 2019

davidtwco commented Jan 26, 2019 •

edited

Loading

davidtwco Jan 27, 2019 •

edited

Loading