Skip to content

rustc_privacy: Audit VisiblePrivateTypesVisitor for correct processing of items in blocks #29524

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
petrochenkov opened this issue Nov 2, 2015 · 0 comments

Comments

@petrochenkov
Copy link
Contributor

Spawned from https://github.com/rust-lang/rust/pull/29291/files#r43682594

VisiblePrivateTypesVisitor assumes that blocks can't contain exported items, but it's not true, blocks can contain impls, and they can be exported.

bors added a commit that referenced this issue Dec 18, 2015
Some notes:
This patch enforces the rules from [RFC 136](https://github.com/rust-lang/rfcs/blob/master/text/0136-no-privates-in-public.md) and makes "private in public" a module-level concept and not crate-level. Only `pub` annotations are used by the new algorithm, crate-level exported node set produced by `EmbargoVisitor` is not used. The error messages are tweaked accordingly and don't use the word "exported" to avoid confusing people (#29668).

The old algorithm tried to be extra smart with impls, but it mostly led to unpredictable behavior and bugs like #28325.
The new algorithm tries to be as simple as possible - an impl is considered public iff its type is public and its trait is public (if presents).
A type or trait is considered public if all its components are public, [complications](https://internals.rust-lang.org/t/limits-of-type-inference-smartness/2919) with private types leaking to other crates/modules through trait impls and type inference are deliberately ignored so far.

The new algorithm is not recursive and uses the nice new facility `Crate::visit_all_items`!

Obsolete pre-1.0 feature `visible_private_types` is removed.

This is a [breaking-change].
The two main vectors of breakage are type aliases (#28450) and impls (#28325).
I need some statistics from a crater run (cc @alexcrichton) to decide on the breakage mitigation strategy.
UPDATE: All the new errors are reported as warnings controlled by a lint `private_in_public` and lint group `future_incompatible`, but the intent is to make them hard errors eventually.

Closes #28325
Closes #28450
Closes #29524
Closes #29627
Closes #29668
Closes #30055

r? @nikomatsakis
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant