Skip to content

Commit

Permalink
Rollup merge of rust-lang#90586 - jswrenn:relax-privacy-lints, r=petr…
Browse files Browse the repository at this point in the history
…ochenkov

Relax priv-in-pub lint on generic bounds and where clauses of trait impls.

The priv-in-pub lint is a legacy mechanism of the compiler, supplanted by a reachability-based [type privacy](https://github.com/rust-lang/rfcs/blob/master/text/2145-type-privacy.md) analysis. This PR does **not** relax type privacy; it only relaxes the lint (as proposed by the type privacy RFC) in the case of trait impls.

## Current Behavior
On public trait impls, it's currently an **error** to have a `where` bound constraining a private type with a trait:
```rust
pub trait Trait {}
pub struct Type {}

struct Priv {}
impl Trait for Priv {}

impl Trait for Type
where
    Priv: Trait // ERROR
{}
```

...and it's a **warning** to have have a public type constrained by a private trait:
```rust
pub trait Trait {}
pub struct Type {}

pub struct Pub {}
trait Priv {}
impl Priv for Pub {}

impl Trait for Type
where
    Pub: Priv // WARNING
{}
```

This lint applies to `where` clauses in other contexts, too; e.g. on free functions:
```rust
struct Priv<T>(T);
pub trait Pub {}
impl<T: Pub> Pub for Priv<T> {}

pub fn function<T>()
where
    Priv<T>: Pub // WARNING
{}
```

**These constraints could be relaxed without issue.**

## New Behavior
This lint is relaxed for `where` clauses on trait impls, such that it's okay to have a `where` bound constraining a private type with a trait:
```rust
pub trait Trait {}
pub struct Type {}

struct Priv {}
impl Trait for Priv {}

impl Trait for Type
where
    Priv: Trait // OK
{}
```

...and it's okay to have a public type constrained by a private trait:
```rust
pub trait Trait {}
pub struct Type {}

pub struct Pub {}
trait Priv {}
impl Priv for Pub {}

impl Trait for Type
where
    Pub: Priv // OK
{}
```

## Rationale
While the priv-in-pub lint is not essential for soundness, it *can* help programmers avoid pitfalls that would make their libraries difficult to use by others. For instance, such a lint *is* useful for free functions; e.g. if a downstream crate tries to call the `function` in the previous snippet in a generic context:
```rust
fn callsite<T>()
where
    Priv<T>: Pub // ERROR: omitting this bound is a compile error, but including it is too
{
    function::<T>()
}
```
...it cannot do so without repeating `function`'s `where` bound, which we cannot do because `Priv` is out-of-scope. A lint for this case is arguably helpful.

However, this same reasoning **doesn't** hold for trait impls. To call an unconstrained method on a public trait impl with private bounds, you don't need to forward those private bounds, you can forward the public trait:
```rust
mod upstream {
    pub trait Trait {
        fn method(&self) {}
    }
    pub struct Type<T>(T);

    pub struct Pub<T>(T);
    trait Priv {}
    impl<T: Priv> Priv for Pub<T> {}

    impl<T> Trait for Type<T>
    where
        Pub<T>: Priv // WARNING
    {}
}

mod downstream {
    use super::upstream::*;

    fn function<T>(value: Type<T>)
    where
        Type<T>: Trait // <- no private deets!
    {
        value.method();
    }
}
```

**This PR only eliminates the lint on trait impls.** It leaves it intact for all other contexts, including trait definitions, inherent impls, and function definitions. It doesn't need to exist in those cases either, but I figured I'd first target a case where it's mostly pointless.

## Other Notes
- See discussion [on zulip](https://rust-lang.zulipchat.com/#narrow/stream/213817-t-lang/topic/relax.20priv-in-pub.20lint.20for.20trait.20impl.20.60where.60.20bounds/near/222458397).
- This PR effectively reverts rust-lang#79291.
  • Loading branch information
matthiaskrgr authored Dec 27, 2021
2 parents f8abed9 + ebef8a8 commit b57a6b3
Show file tree
Hide file tree
Showing 9 changed files with 325 additions and 82 deletions.
6 changes: 5 additions & 1 deletion compiler/rustc_privacy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2064,7 +2064,11 @@ impl<'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'tcx> {
// Subitems of trait impls have inherited publicity.
hir::ItemKind::Impl(ref impl_) => {
let impl_vis = ty::Visibility::of_impl(item.def_id, tcx, &Default::default());
self.check(item.def_id, impl_vis).generics().predicates();
// check that private components do not appear in the generics or predicates of inherent impls
// this check is intentionally NOT performed for impls of traits, per #90586
if impl_.of_trait.is_none() {
self.check(item.def_id, impl_vis).generics().predicates();
}
for impl_item_ref in impl_.items {
let impl_item_vis = if impl_.of_trait.is_none() {
min(tcx.visibility(impl_item_ref.id.def_id), impl_vis, tcx)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,7 @@ pub trait Trait {
fn assoc_fn() -> Self::AssocTy;
}

impl<const U: u8> Trait for Const<U>
//~^ WARN private type
//~| WARN this was previously
//~| WARN private type
//~| WARN this was previously

impl<const U: u8> Trait for Const<U> // OK, trait impl predicates
where
Const<{ my_const_fn(U) }>: ,
{
Expand Down
35 changes: 2 additions & 33 deletions src/test/ui/const-generics/generic_const_exprs/eval-privacy.stderr
Original file line number Diff line number Diff line change
@@ -1,43 +1,12 @@
warning: private type `fn(u8) -> u8 {my_const_fn}` in public interface (error E0446)
--> $DIR/eval-privacy.rs:12:1
|
LL | / impl<const U: u8> Trait for Const<U>
LL | |
LL | |
LL | |
... |
LL | | }
LL | | }
| |_^
|
= note: `#[warn(private_in_public)]` on by default
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

warning: private type `fn(u8) -> u8 {my_const_fn}` in public interface (error E0446)
--> $DIR/eval-privacy.rs:12:1
|
LL | / impl<const U: u8> Trait for Const<U>
LL | |
LL | |
LL | |
... |
LL | | }
LL | | }
| |_^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error[E0446]: private type `fn(u8) -> u8 {my_const_fn}` in public interface
--> $DIR/eval-privacy.rs:21:5
--> $DIR/eval-privacy.rs:16:5
|
LL | type AssocTy = Const<{ my_const_fn(U) }>;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak private type
...
LL | const fn my_const_fn(val: u8) -> u8 {
| ----------------------------------- `fn(u8) -> u8 {my_const_fn}` declared as private

error: aborting due to previous error; 2 warnings emitted
error: aborting due to previous error

For more information about this error, try `rustc --explain E0446`.
7 changes: 2 additions & 5 deletions src/test/ui/privacy/private-in-public-warn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ mod traits {
}
impl<T: PrivTr> Pub<T> {} //~ ERROR private trait `traits::PrivTr` in public interface
//~^ WARNING hard error
impl<T: PrivTr> PubTr for Pub<T> {} //~ ERROR private trait `traits::PrivTr` in public interface
//~^ WARNING hard error
impl<T: PrivTr> PubTr for Pub<T> {} // OK, trait impl predicates
}

mod traits_where {
Expand All @@ -87,9 +86,7 @@ mod traits_where {
impl<T> Pub<T> where T: PrivTr {}
//~^ ERROR private trait `traits_where::PrivTr` in public interface
//~| WARNING hard error
impl<T> PubTr for Pub<T> where T: PrivTr {}
//~^ ERROR private trait `traits_where::PrivTr` in public interface
//~| WARNING hard error
impl<T> PubTr for Pub<T> where T: PrivTr {} // OK, trait impl predicates
}

mod generics {
Expand Down
56 changes: 19 additions & 37 deletions src/test/ui/privacy/private-in-public-warn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -156,17 +156,8 @@ LL | impl<T: PrivTr> Pub<T> {}
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private trait `traits::PrivTr` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:66:5
|
LL | impl<T: PrivTr> PubTr for Pub<T> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private trait `traits_where::PrivTr` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:75:5
--> $DIR/private-in-public-warn.rs:74:5
|
LL | pub type Alias<T> where T: PrivTr = T;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -175,7 +166,7 @@ LL | pub type Alias<T> where T: PrivTr = T;
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private trait `traits_where::PrivTr` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:79:5
--> $DIR/private-in-public-warn.rs:78:5
|
LL | pub trait Tr2<T> where T: PrivTr {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -184,7 +175,7 @@ LL | pub trait Tr2<T> where T: PrivTr {}
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private trait `traits_where::PrivTr` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:83:9
--> $DIR/private-in-public-warn.rs:82:9
|
LL | fn f<T>(arg: T) where T: PrivTr {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -193,25 +184,16 @@ LL | fn f<T>(arg: T) where T: PrivTr {}
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private trait `traits_where::PrivTr` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:87:5
--> $DIR/private-in-public-warn.rs:86:5
|
LL | impl<T> Pub<T> where T: PrivTr {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private trait `traits_where::PrivTr` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:90:5
|
LL | impl<T> PubTr for Pub<T> where T: PrivTr {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private trait `generics::PrivTr<generics::Pub>` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:101:5
--> $DIR/private-in-public-warn.rs:98:5
|
LL | pub trait Tr1: PrivTr<Pub> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -220,7 +202,7 @@ LL | pub trait Tr1: PrivTr<Pub> {}
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private type `generics::Priv` in public interface (error E0446)
--> $DIR/private-in-public-warn.rs:104:5
--> $DIR/private-in-public-warn.rs:101:5
|
LL | pub trait Tr2: PubTr<Priv> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -229,7 +211,7 @@ LL | pub trait Tr2: PubTr<Priv> {}
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private type `generics::Priv` in public interface (error E0446)
--> $DIR/private-in-public-warn.rs:106:5
--> $DIR/private-in-public-warn.rs:103:5
|
LL | pub trait Tr3: PubTr<[Priv; 1]> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -238,7 +220,7 @@ LL | pub trait Tr3: PubTr<[Priv; 1]> {}
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private type `generics::Priv` in public interface (error E0446)
--> $DIR/private-in-public-warn.rs:108:5
--> $DIR/private-in-public-warn.rs:105:5
|
LL | pub trait Tr4: PubTr<Pub<Priv>> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -247,7 +229,7 @@ LL | pub trait Tr4: PubTr<Pub<Priv>> {}
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error[E0446]: private type `impls::Priv` in public interface
--> $DIR/private-in-public-warn.rs:135:9
--> $DIR/private-in-public-warn.rs:132:9
|
LL | struct Priv;
| ------------ `impls::Priv` declared as private
Expand All @@ -256,7 +238,7 @@ LL | type Alias = Priv;
| ^^^^^^^^^^^^^^^^^^ can't leak private type

error: private type `aliases_pub::Priv` in public interface (error E0446)
--> $DIR/private-in-public-warn.rs:206:9
--> $DIR/private-in-public-warn.rs:203:9
|
LL | pub fn f(arg: Priv) {}
| ^^^^^^^^^^^^^^^^^^^
Expand All @@ -265,7 +247,7 @@ LL | pub fn f(arg: Priv) {}
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error[E0446]: private type `aliases_pub::Priv` in public interface
--> $DIR/private-in-public-warn.rs:210:9
--> $DIR/private-in-public-warn.rs:207:9
|
LL | struct Priv;
| ------------ `aliases_pub::Priv` declared as private
Expand All @@ -274,7 +256,7 @@ LL | type Check = Priv;
| ^^^^^^^^^^^^^^^^^^ can't leak private type

error[E0446]: private type `aliases_pub::Priv` in public interface
--> $DIR/private-in-public-warn.rs:213:9
--> $DIR/private-in-public-warn.rs:210:9
|
LL | struct Priv;
| ------------ `aliases_pub::Priv` declared as private
Expand All @@ -283,7 +265,7 @@ LL | type Check = Priv;
| ^^^^^^^^^^^^^^^^^^ can't leak private type

error[E0446]: private type `aliases_pub::Priv` in public interface
--> $DIR/private-in-public-warn.rs:216:9
--> $DIR/private-in-public-warn.rs:213:9
|
LL | struct Priv;
| ------------ `aliases_pub::Priv` declared as private
Expand All @@ -292,7 +274,7 @@ LL | type Check = Priv;
| ^^^^^^^^^^^^^^^^^^ can't leak private type

error[E0446]: private type `aliases_pub::Priv` in public interface
--> $DIR/private-in-public-warn.rs:219:9
--> $DIR/private-in-public-warn.rs:216:9
|
LL | struct Priv;
| ------------ `aliases_pub::Priv` declared as private
Expand All @@ -301,7 +283,7 @@ LL | type Check = Priv;
| ^^^^^^^^^^^^^^^^^^ can't leak private type

error: private trait `PrivTr1` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:249:5
--> $DIR/private-in-public-warn.rs:246:5
|
LL | pub trait Tr1: PrivUseAliasTr {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -310,7 +292,7 @@ LL | pub trait Tr1: PrivUseAliasTr {}
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private trait `PrivTr1<Priv2>` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:252:5
--> $DIR/private-in-public-warn.rs:249:5
|
LL | pub trait Tr2: PrivUseAliasTr<PrivAlias> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -319,7 +301,7 @@ LL | pub trait Tr2: PrivUseAliasTr<PrivAlias> {}
= note: for more information, see issue #34537 <https://github.com/rust-lang/rust/issues/34537>

error: private type `Priv2` in public interface (error E0446)
--> $DIR/private-in-public-warn.rs:252:5
--> $DIR/private-in-public-warn.rs:249:5
|
LL | pub trait Tr2: PrivUseAliasTr<PrivAlias> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Expand All @@ -341,7 +323,7 @@ LL + pub type Alias<T> = T;
|

warning: where clauses are not enforced in type aliases
--> $DIR/private-in-public-warn.rs:75:29
--> $DIR/private-in-public-warn.rs:74:29
|
LL | pub type Alias<T> where T: PrivTr = T;
| ^^^^^^^^^
Expand All @@ -352,6 +334,6 @@ LL - pub type Alias<T> where T: PrivTr = T;
LL + pub type Alias<T> = T;
|

error: aborting due to 36 previous errors; 2 warnings emitted
error: aborting due to 34 previous errors; 2 warnings emitted

For more information about this error, try `rustc --explain E0446`.
90 changes: 90 additions & 0 deletions src/test/ui/privacy/where-priv-type.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
// priv-in-pub lint tests where the private type appears in the
// `where` clause of a public item

#![crate_type = "lib"]
#![feature(generic_const_exprs)]
#![allow(incomplete_features)]


struct PrivTy;
trait PrivTr {}
pub struct PubTy;
pub struct PubTyGeneric<T>(T);
pub trait PubTr {}
impl PubTr for PrivTy {}
pub trait PubTrWithAssocTy { type AssocTy; }
impl PubTrWithAssocTy for PrivTy { type AssocTy = PrivTy; }


pub struct S
//~^ WARNING private type `PrivTy` in public interface
//~| WARNING hard error
where
PrivTy:
{}


pub enum E
//~^ WARNING private type `PrivTy` in public interface
//~| WARNING hard error
where
PrivTy:
{}


pub fn f()
//~^ WARNING private type `PrivTy` in public interface
//~| WARNING hard error
where
PrivTy:
{}


impl S
//~^ ERROR private type `PrivTy` in public interface
where
PrivTy:
{
pub fn f()
//~^ WARNING private type `PrivTy` in public interface
//~| WARNING hard error
where
PrivTy:
{}
}


impl PubTr for PubTy
where
PrivTy:
{}


impl<T> PubTr for PubTyGeneric<T>
where
T: PubTrWithAssocTy<AssocTy=PrivTy>
{}


pub struct Const<const U: u8>;

pub trait Trait {
type AssocTy;
fn assoc_fn() -> Self::AssocTy;
}

impl<const U: u8> Trait for Const<U>
where
Const<{ my_const_fn(U) }>: ,
{
type AssocTy = Const<{ my_const_fn(U) }>;
//~^ ERROR private type
fn assoc_fn() -> Self::AssocTy {
Const
}
}

const fn my_const_fn(val: u8) -> u8 {
// body of this function doesn't matter
val
}
Loading

0 comments on commit b57a6b3

Please sign in to comment.