Skip to content

Make all dist functions and structs crate-private and cleanup unused functions #2808

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

Merged
merged 1 commit into from
Jul 18, 2021

Conversation

0xPoe
Copy link
Member

@0xPoe 0xPoe commented Jun 26, 2021

part of #2730

@0xPoe 0xPoe marked this pull request as ready for review June 27, 2021 02:25
@0xPoe
Copy link
Member Author

0xPoe commented Jun 28, 2021

@kinnison Thanks for your review!

Regarding the use of pub(crate) in the pub(crate) structure I understand that it is useful? Because the visibility of the fields of a pub(crate) structure is private by default and cannot be accessed by other mods in the same crate. So we can use pub(crate)? If we use pub, then other crates will be able to use it. This would result in the accessibility being amplified?

An example of a compilation error that does not use pub(crate): https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=ece1d5cb3da661daf651d6dbd575d21b

An example of a successful compilation using pub(crate): https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=5dc0ff67c55081d1ffec5fd0666bc6f3

I'm not sure if I'm understanding this correctly?

@0xPoe 0xPoe requested a review from kinnison July 4, 2021 02:25
@0xPoe
Copy link
Member Author

0xPoe commented Jul 6, 2021

@kinnison ping~

Could you please take a look? Thanks!

@kinnison
Copy link
Contributor

Hi, the thing is that with the struct being pub(crate) there's no difference in meaning between pub and pub(crate) on the fields. Except the latter implies that the crate has privileged and special knowledge of the interior of the structure which frankly it doesn't. I'd prefer pub(crate) on the struct, and then just plain pub on the fields. There can be no access amplification because nothing outside of the crate can address the struct type at all.

@0xPoe
Copy link
Member Author

0xPoe commented Jul 18, 2021

Hi, the thing is that with the struct being pub(crate) there's no difference in meaning between pub and pub(crate) on the fields. Except the latter implies that the crate has privileged and special knowledge of the interior of the structure which frankly it doesn't. I'd prefer pub(crate) on the struct, and then just plain pub on the fields. There can be no access amplification because nothing outside of the crate can address the struct type at all.

I modified them all to pub. Thanks for you review!

@kinnison kinnison merged commit 457d643 into rust-lang:master Jul 18, 2021
@0xPoe 0xPoe deleted the rustin-dist branch July 19, 2021 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants