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

Tracking issue for RFC #2145: Type privacy and private-in-public lints #48054

Closed
5 tasks
aturon opened this issue Feb 7, 2018 · 11 comments · Fixed by #120144
Closed
5 tasks

Tracking issue for RFC #2145: Type privacy and private-in-public lints #48054

aturon opened this issue Feb 7, 2018 · 11 comments · Fixed by #120144
Assignees
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-visibility Area: Visibility / privacy. B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. S-tracking-design-concerns Status: There are blocking ❌ design concerns. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue.

Comments

@aturon
Copy link
Member

aturon commented Feb 7, 2018

This is a tracking issue for the RFC "Type privacy and private-in-public lints " (rust-lang/rfcs#2145).

Steps:

Unresolved questions:

  • It's not fully clear if the restriction for associated type definitions required for
    type privacy soundness, or it's just a workaround for a technical difficulty.

  • Interactions between macros 2.0 and the notions of reachability / effective
    visibility used for the lints are unclear.

@aturon aturon added B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Feb 7, 2018
bors added a commit that referenced this issue Dec 31, 2018
privacy: Use common `DefId` visiting infrastructure for all privacy visitors

One repeating pattern in privacy checking is going through a type, visiting all `DefId`s inside it and doing something with them.
This is the case because visibilities and reachabilities are attached to `DefId`s.

Previously various privacy visitors visited types slightly differently using their own methods, with most recently written `TypePrivacyVisitor` being the "gold standard".
This mostly worked okay, but differences could manifest in overly conservative reachability analysis, some errors being reported twice, some private-in-public lints (not errors) being wrongly reported or not reported.

This PR does something that I wanted to do since #32674 (comment) - factoring out the common visiting logic!
Now all the common logic is contained in `struct DefIdVisitorSkeleton`, with specific privacy visitors deciding only what to do with visited `DefId`s (via `trait DefIdVisitor`).

A bunch of cleanups is also applied in the process.
This area is somewhat tricky due to lots of easily miss-able details, but thankfully it's was well covered by tests in #46083 and previous PRs, so I'm relatively sure in the refactoring correctness.

Fixes #56837 (comment) in particular.
Also this will help with implementing #48054.
@petrochenkov petrochenkov self-assigned this Feb 5, 2019
@Aaron1011
Copy link
Member

@rustbot claim

@rustbot rustbot assigned Aaron1011 and unassigned petrochenkov Aug 24, 2019
@Aaron1011
Copy link
Member

@petrochenkov: Oops - I didn't mean for that to un-assign you.

Is there a way to prevent these lints from becoming insta-stable? Would it make sense to put them behind a feature gate?

@Aaron1011
Copy link
Member

Perhaps we could only fire the lints if an unstable compiler flag (maybe -Z type-privacy-lints) is passed? This would still make the lint names insta-stable, though.

Aaron1011 added a commit to Aaron1011/pin-project that referenced this issue Aug 25, 2019
Previously, given code such as:

```rust

struct Private<T>;

\#[pin_project]
pub struct Public<T> {
    #[pin] private: Private<T>
}
```

we would generate an Unpin impl like this:

```rust
impl Unpin for Public where Private: Unpin {}
```

Unfortunately, since Private is not a public type,
this would cause an E0446 ('private type `Private` in public interface)

When RFC 2145 is implemented (rust-lang/rust#48054),
this will become a lint, rather then a hard error.

In the time being, we need a solution that will work with the current
type privacy rules.

The solution is to generate code like this:

```rust

fn __private_scope() {
    pub struct __UnpinPublic<T> {
        __field0: Private<T>
    }
    impl<T> Unpin for Public<T> where __UnpinPublic<T>: Unpin {}
}
```

That is, we generate a new struct, containing all of the pinned
fields from our #[pin_project] type. This struct is delcared within
a function, which makes it impossible to be named by user code.
This guarnatees that it will use the default auto-trait impl for Unpin -
that is, it will implement Unpin iff all of its fields implement Unpin.
This type can be safely declared as 'public', satisfiying the privacy
checker without actually allowing user code to access it.

This allows users to apply the #[pin_project] attribute to types
regardless of the privacy of the types of their fields.
Aaron1011 added a commit to Aaron1011/pin-project that referenced this issue Aug 25, 2019
Previously, given code such as:

```rust

struct Private<T>;

#[pin_project]
pub struct Public<T> {
    #[pin] private: Private<T>
}
```

we would generate an Unpin impl like this:

```rust
impl Unpin for Public where Private: Unpin {}
```

Unfortunately, since Private is not a public type,
this would cause an E0446 ('private type `Private` in public interface)

When RFC 2145 is implemented (rust-lang/rust#48054),
this will become a lint, rather then a hard error.

In the time being, we need a solution that will work with the current
type privacy rules.

The solution is to generate code like this:

```rust

fn __private_scope() {
    pub struct __UnpinPublic<T> {
        __field0: Private<T>
    }
    impl<T> Unpin for Public<T> where __UnpinPublic<T>: Unpin {}
}
```

That is, we generate a new struct, containing all of the pinned
fields from our #[pin_project] type. This struct is delcared within
a function, which makes it impossible to be named by user code.
This guarnatees that it will use the default auto-trait impl for Unpin -
that is, it will implement Unpin iff all of its fields implement Unpin.
This type can be safely declared as 'public', satisfiying the privacy
checker without actually allowing user code to access it.

This allows users to apply the #[pin_project] attribute to types
regardless of the privacy of the types of their fields.
Aaron1011 added a commit to Aaron1011/pin-project that referenced this issue Aug 25, 2019
Previously, given code such as:

```rust

struct Private<T>;

pub struct Public<T> {
    #[pin] private: Private<T>
}
```

we would generate an Unpin impl like this:

```rust
impl Unpin for Public where Private: Unpin {}
```

Unfortunately, since Private is not a public type,
this would cause an E0446 ('private type `Private` in public interface)

When RFC 2145 is implemented (rust-lang/rust#48054),
this will become a lint, rather then a hard error.

In the time being, we need a solution that will work with the current
type privacy rules.

The solution is to generate code like this:

```rust

fn __private_scope() {
    pub struct __UnpinPublic<T> {
        __field0: Private<T>
    }
    impl<T> Unpin for Public<T> where __UnpinPublic<T>: Unpin {}
}
```

That is, we generate a new struct, containing all of the pinned
fields from our #[pin_project] type. This struct is delcared within
a function, which makes it impossible to be named by user code.
This guarnatees that it will use the default auto-trait impl for Unpin -
that is, it will implement Unpin iff all of its fields implement Unpin.
This type can be safely declared as 'public', satisfiying the privacy
checker without actually allowing user code to access it.

This allows users to apply the #[pin_project] attribute to types
regardless of the privacy of the types of their fields.
Aaron1011 added a commit to Aaron1011/pin-project that referenced this issue Aug 29, 2019
Previously, given code such as:

```rust

struct Private<T>;

pub struct Public<T> {
    #[pin] private: Private<T>
}
```

we would generate an Unpin impl like this:

```rust
impl Unpin for Public where Private: Unpin {}
```

Unfortunately, since Private is not a public type,
this would cause an E0446 ('private type `Private` in public interface)

When RFC 2145 is implemented (rust-lang/rust#48054),
this will become a lint, rather then a hard error.

In the time being, we need a solution that will work with the current
type privacy rules.

The solution is to generate code like this:

```rust

fn __private_scope() {
    pub struct __UnpinPublic<T> {
        __field0: Private<T>
    }
    impl<T> Unpin for Public<T> where __UnpinPublic<T>: Unpin {}
}
```

That is, we generate a new struct, containing all of the pinned
fields from our #[pin_project] type. This struct is delcared within
a function, which makes it impossible to be named by user code.
This guarnatees that it will use the default auto-trait impl for Unpin -
that is, it will implement Unpin iff all of its fields implement Unpin.
This type can be safely declared as 'public', satisfiying the privacy
checker without actually allowing user code to access it.

This allows users to apply the #[pin_project] attribute to types
regardless of the privacy of the types of their fields.
bors bot added a commit to taiki-e/pin-project that referenced this issue Aug 29, 2019
53: Allow using #[pin_project]  type with private field types r=taiki-e a=Aaron1011

Previously, given code such as:

```rust

struct Private<T>;

#[pin_project]
pub struct Public<T> {
    #[pin] private: Private<T>
}
```

we would generate an Unpin impl like this:

```rust
impl Unpin for Public where Private: Unpin {}
```

Unfortunately, since Private is not a public type,
this would cause an E0446 ('private type `Private` in public interface)

When RFC 2145 is implemented (rust-lang/rust#48054),
this will become a lint, rather then a hard error.

In the time being, we need a solution that will work with the current
type privacy rules.

The solution is to generate code like this:

```rust

fn __private_scope() {
    pub struct __UnpinPublic<T> {
        __field0: Private<T>
    }
    impl<T> Unpin for Public<T> where __UnpinPublic<T>: Unpin {}
}
```

That is, we generate a new struct, containing all of the pinned
fields from our #[pin_project] type. This struct is delcared within
a function, which makes it impossible to be named by user code.
This guarnatees that it will use the default auto-trait impl for Unpin -
that is, it will implement Unpin iff all of its fields implement Unpin.
This type can be safely declared as 'public', satisfiying the privacy
checker without actually allowing user code to access it.

This allows users to apply the #[pin_project] attribute to types
regardless of the privacy of the types of their fields.

Co-authored-by: Aaron Hill <aa1ronham@gmail.com>
bors added a commit to rust-lang-ci/rust that referenced this issue May 6, 2023
Populate effective visibilities in 'rustc_privacy'

Next part of RFC rust-lang#48054.

r? `@petrochenkov`
RalfJung pushed a commit to RalfJung/miri that referenced this issue May 7, 2023
Populate effective visibilities in 'rustc_privacy'

Next part of RFC rust-lang/rust#48054.

r? `@petrochenkov`
bors added a commit to rust-lang-ci/rust that referenced this issue Jun 11, 2023
Private-in-public lints implementation

Next part of RFC rust-lang#48054.

r? `@petrochenkov`
thomcc pushed a commit to tcdi/postgrestd that referenced this issue Jul 18, 2023
Populate effective visibilities in 'rustc_privacy'

Next part of RFC rust-lang/rust#48054.

r? `@petrochenkov`
compiler-errors added a commit to compiler-errors/rust that referenced this issue Aug 22, 2023
fix rust-lang#113702 emit a proper diagnostic message for unstable lints passed from CLI

Current output:
```bash
$ build/host/stage1/bin/rustc hello.rs -Wunnameable_types
warning: unknown lint: `unnameable_types`
  |
  = note: the `unnameable_types` lint is unstable
  = note: see issue rust-lang#48054 <rust-lang#48054> for more information
  = help: add `-Zcrate-attr="feature(type_privacy_lints)"` to the command-line options to enable
  = note: `#[warn(unknown_lints)]` on by default

warning: 1 warning emitted
```

Previously, the feature gate diagnostic message is like below, which is the same as the message for unstable lints from the root module.

```shell
= help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable
```

Fixes rust-lang#113702
bors added a commit to rust-lang-ci/rust that referenced this issue Sep 1, 2023
Replace old private-in-public diagnostic with type privacy lints

Next part of RFC rust-lang#48054.

r? `@petrochenkov`
RalfJung pushed a commit to RalfJung/miri that referenced this issue Sep 2, 2023
Replace old private-in-public diagnostic with type privacy lints

Next part of RFC rust-lang/rust#48054.

r? `@petrochenkov`
@petrochenkov
Copy link
Contributor

This issue should be closed by #120144.

Two follow up issues about improving the effective visibility tables

lnicola pushed a commit to lnicola/rust-analyzer that referenced this issue Apr 7, 2024
Replace old private-in-public diagnostic with type privacy lints

Next part of RFC rust-lang/rust#48054.

r? `@petrochenkov`
@bors bors closed this as completed in 337be99 Apr 8, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 8, 2024
Rollup merge of rust-lang#120144 - petrochenkov:unty, r=davidtwco

privacy: Stabilize lint `unnameable_types`

This is the last piece of ["RFC rust-lang#2145: Type privacy and private-in-public lints"](rust-lang#48054).

Having unstable lints is not very useful because you cannot even dogfood them in the compiler/stdlib in this case (rust-lang#113284).
The worst thing that may happen when a lint is removed are some `removed_lints` warnings, but I haven't heard anyone suggesting removing this specific lint.

This lint is allow-by-default and is supposed to be enabled explicitly.
Some false positives are expected, because sometimes unnameable types are a legitimate pattern.
This lint also have some unnecessary false positives, that can be fixed - see rust-lang#120146 and rust-lang#120149.

Closes rust-lang#48054.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
Populate effective visibilities in 'rustc_resolve'

Next part of RFC rust-lang/rust#48054.
previous: rust-lang/rust#101713

`@rustbot` author
r? `@petrochenkov`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 20, 2024
privacy: Rename "accessibility levels" to "effective visibilities"

And a couple of other naming and comment tweaks.

Related to rust-lang/rust#48054

For `enum Level` I initially used naming `enum EffectiveVisibilityLevel`, but it was too long and inconvenient because it's used pretty often.
So I shortened it to just `Level`, if it needs to be used from some context where this name would be ambiguous, then it can be imported with renaming like `use rustc_middle::privacy::Level as EffVisLevel` or something.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Populate effective visibilities in 'rustc_resolve'

Next part of RFC rust-lang/rust#48054.
previous: rust-lang/rust#101713

`@rustbot` author
r? `@petrochenkov`
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
privacy: Rename "accessibility levels" to "effective visibilities"

And a couple of other naming and comment tweaks.

Related to rust-lang/rust#48054

For `enum Level` I initially used naming `enum EffectiveVisibilityLevel`, but it was too long and inconvenient because it's used pretty often.
So I shortened it to just `Level`, if it needs to be used from some context where this name would be ambiguous, then it can be imported with renaming like `use rustc_middle::privacy::Level as EffVisLevel` or something.
RalfJung pushed a commit to RalfJung/rust-analyzer that referenced this issue Apr 27, 2024
Replace old private-in-public diagnostic with type privacy lints

Next part of RFC rust-lang/rust#48054.

r? `@petrochenkov`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: Lints (warnings about flaws in source code) such as unused_mut. A-visibility Area: Visibility / privacy. B-RFC-approved Blocker: Approved by a merged RFC but not yet implemented. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. S-tracking-design-concerns Status: There are blocking ❌ design concerns. S-tracking-needs-summary Status: It's hard to tell what's been done and what hasn't! Someone should do some investigation. T-lang Relevant to the language team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants