Skip to content

Commit

Permalink
Auto merge of #56878 - petrochenkov:privdyn, r=arielb1
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Dec 31, 2018
2 parents 9eac386 + 60d1fa7 commit fe6a54d
Show file tree
Hide file tree
Showing 22 changed files with 621 additions and 559 deletions.
9 changes: 9 additions & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2090,6 +2090,15 @@ impl VisibilityKind {
VisibilityKind::Restricted { .. } => true,
}
}

pub fn descr(&self) -> &'static str {
match *self {
VisibilityKind::Public => "public",
VisibilityKind::Inherited => "private",
VisibilityKind::Crate(..) => "crate-visible",
VisibilityKind::Restricted { .. } => "restricted",
}
}
}

#[derive(Clone, RustcEncodable, RustcDecodable, Debug)]
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/util/ppaux.rs
Original file line number Diff line number Diff line change
Expand Up @@ -709,6 +709,9 @@ define_print! {

define_print! {
('tcx) ty::ExistentialTraitRef<'tcx>, (self, f, cx) {
display {
cx.parameterized(f, self.substs, self.def_id, &[])
}
debug {
ty::tls::with(|tcx| {
let dummy_self = tcx.mk_infer(ty::FreshTy(0));
Expand Down
960 changes: 446 additions & 514 deletions src/librustc_privacy/lib.rs

Large diffs are not rendered by default.

5 changes: 0 additions & 5 deletions src/libstd/sys/windows/handle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,6 @@ impl RawHandle {
}
}

pub fn read_to_end(&self, buf: &mut Vec<u8>) -> io::Result<usize> {
let mut me = self;
(&mut me).read_to_end(buf)
}

pub fn write(&self, buf: &[u8]) -> io::Result<usize> {
let mut amt = 0;
let len = cmp::min(buf.len(), <c::DWORD>::max_value() as usize) as c::DWORD;
Expand Down
5 changes: 0 additions & 5 deletions src/libstd/sys/windows/stdio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,6 @@ impl Stdin {
// MemReader shouldn't error here since we just filled it
utf8.read(buf)
}

pub fn read_to_end(&self, buf: &mut Vec<u8>) -> io::Result<usize> {
let mut me = self;
(&mut me).read_to_end(buf)
}
}

#[unstable(reason = "not public", issue = "0", feature = "fd_read")]
Expand Down
3 changes: 0 additions & 3 deletions src/test/ui/error-codes/E0445.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,9 @@ trait Foo {

pub trait Bar : Foo {}
//~^ ERROR private trait `Foo` in public interface [E0445]
//~| NOTE can't leak private trait
pub struct Bar2<T: Foo>(pub T);
//~^ ERROR private trait `Foo` in public interface [E0445]
//~| NOTE can't leak private trait
pub fn foo<T: Foo> (t: T) {}
//~^ ERROR private trait `Foo` in public interface [E0445]
//~| NOTE can't leak private trait

fn main() {}
4 changes: 2 additions & 2 deletions src/test/ui/error-codes/E0445.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ LL | pub trait Bar : Foo {}
| ^^^^^^^^^^^^^^^^^^^^^^ can't leak private trait

error[E0445]: private trait `Foo` in public interface
--> $DIR/E0445.rs:8:1
--> $DIR/E0445.rs:7:1
|
LL | pub struct Bar2<T: Foo>(pub T);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak private trait

error[E0445]: private trait `Foo` in public interface
--> $DIR/E0445.rs:11:1
--> $DIR/E0445.rs:9:1
|
LL | pub fn foo<T: Foo> (t: T) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't leak private trait
Expand Down
4 changes: 3 additions & 1 deletion src/test/ui/impl-trait/issue-49376.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ fn gen() -> impl PartialOrd + PartialEq + Debug { }

struct Bar {}
trait Foo<T = Self> {}
trait FooNested<T = Option<Self>> {}
impl Foo for Bar {}
impl FooNested for Bar {}

fn foo() -> impl Foo {
fn foo() -> impl Foo + FooNested {
Bar {}
}

Expand Down
3 changes: 3 additions & 0 deletions src/test/ui/issues/issue-18389.stderr
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
error[E0445]: private trait `Private<<Self as Public>::P, <Self as Public>::R>` in public interface
--> $DIR/issue-18389.rs:7:1
|
LL | trait Private<P, R> {
| - `Private<<Self as Public>::P, <Self as Public>::R>` declared as private
...
LL | / pub trait Public: Private<
LL | | //~^ ERROR private trait `Private<<Self as Public>::P, <Self as Public>::R>` in public interface
LL | | <Self as Public>::P,
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/privacy/associated-item-privacy-type-binding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@ mod priv_trait {

pub macro mac1() {
let _: Box<PubTr<AssocTy = u8>>;
//~^ ERROR type `(dyn priv_trait::PubTr<AssocTy=u8> + '<empty>)` is private
//~| ERROR type `(dyn priv_trait::PubTr<AssocTy=u8> + '<empty>)` is private
//~^ ERROR trait `priv_trait::PrivTr` is private
//~| ERROR trait `priv_trait::PrivTr` is private
type InSignatureTy2 = Box<PubTr<AssocTy = u8>>;
//~^ ERROR type `(dyn priv_trait::PubTr<AssocTy=u8> + 'static)` is private
//~^ ERROR trait `priv_trait::PrivTr` is private
trait InSignatureTr2: PubTr<AssocTy = u8> {}
//~^ ERROR trait `priv_trait::PrivTr` is private
}
pub macro mac2() {
let _: Box<PrivTr<AssocTy = u8>>;
//~^ ERROR type `(dyn priv_trait::PrivTr<AssocTy=u8> + '<empty>)` is private
//~| ERROR type `(dyn priv_trait::PrivTr<AssocTy=u8> + '<empty>)` is private
//~^ ERROR trait `priv_trait::PrivTr` is private
//~| ERROR trait `priv_trait::PrivTr` is private
type InSignatureTy1 = Box<PrivTr<AssocTy = u8>>;
//~^ ERROR type `(dyn priv_trait::PrivTr<AssocTy=u8> + 'static)` is private
//~^ ERROR trait `priv_trait::PrivTr` is private
trait InSignatureTr1: PrivTr<AssocTy = u8> {}
//~^ ERROR trait `priv_trait::PrivTr` is private
}
Expand Down
12 changes: 6 additions & 6 deletions src/test/ui/privacy/associated-item-privacy-type-binding.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
error: type `(dyn priv_trait::PubTr<AssocTy=u8> + '<empty>)` is private
error: trait `priv_trait::PrivTr` is private
--> $DIR/associated-item-privacy-type-binding.rs:11:13
|
LL | let _: Box<PubTr<AssocTy = u8>>;
Expand All @@ -7,7 +7,7 @@ LL | let _: Box<PubTr<AssocTy = u8>>;
LL | priv_trait::mac1!();
| -------------------- in this macro invocation

error: type `(dyn priv_trait::PubTr<AssocTy=u8> + '<empty>)` is private
error: trait `priv_trait::PrivTr` is private
--> $DIR/associated-item-privacy-type-binding.rs:11:16
|
LL | let _: Box<PubTr<AssocTy = u8>>;
Expand All @@ -16,7 +16,7 @@ LL | let _: Box<PubTr<AssocTy = u8>>;
LL | priv_trait::mac1!();
| -------------------- in this macro invocation

error: type `(dyn priv_trait::PubTr<AssocTy=u8> + 'static)` is private
error: trait `priv_trait::PrivTr` is private
--> $DIR/associated-item-privacy-type-binding.rs:14:31
|
LL | type InSignatureTy2 = Box<PubTr<AssocTy = u8>>;
Expand All @@ -34,7 +34,7 @@ LL | trait InSignatureTr2: PubTr<AssocTy = u8> {}
LL | priv_trait::mac1!();
| -------------------- in this macro invocation

error: type `(dyn priv_trait::PrivTr<AssocTy=u8> + '<empty>)` is private
error: trait `priv_trait::PrivTr` is private
--> $DIR/associated-item-privacy-type-binding.rs:20:13
|
LL | let _: Box<PrivTr<AssocTy = u8>>;
Expand All @@ -43,7 +43,7 @@ LL | let _: Box<PrivTr<AssocTy = u8>>;
LL | priv_trait::mac2!();
| -------------------- in this macro invocation

error: type `(dyn priv_trait::PrivTr<AssocTy=u8> + '<empty>)` is private
error: trait `priv_trait::PrivTr` is private
--> $DIR/associated-item-privacy-type-binding.rs:20:16
|
LL | let _: Box<PrivTr<AssocTy = u8>>;
Expand All @@ -52,7 +52,7 @@ LL | let _: Box<PrivTr<AssocTy = u8>>;
LL | priv_trait::mac2!();
| -------------------- in this macro invocation

error: type `(dyn priv_trait::PrivTr<AssocTy=u8> + 'static)` is private
error: trait `priv_trait::PrivTr` is private
--> $DIR/associated-item-privacy-type-binding.rs:23:31
|
LL | type InSignatureTy1 = Box<PrivTr<AssocTy = u8>>;
Expand Down
13 changes: 13 additions & 0 deletions src/test/ui/privacy/private-in-public-expr-pat.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// Patterns and expressions are not interface parts and don't produce private-in-public errors.

// compile-pass

struct Priv1(usize);
struct Priv2;

pub struct Pub(Priv2);

pub fn public_expr(_: [u8; Priv1(0).0]) {} // OK
pub fn public_pat(Pub(Priv2): Pub) {} // OK

fn main() {}
13 changes: 13 additions & 0 deletions src/test/ui/privacy/private-in-public-non-principal-2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#![feature(optin_builtin_traits)]

#[allow(private_in_public)]
mod m {
pub trait PubPrincipal {}
auto trait PrivNonPrincipal {}
pub fn leak_dyn_nonprincipal() -> Box<PubPrincipal + PrivNonPrincipal> { loop {} }
}

fn main() {
m::leak_dyn_nonprincipal();
//~^ ERROR trait `m::PrivNonPrincipal` is private
}
8 changes: 8 additions & 0 deletions src/test/ui/privacy/private-in-public-non-principal-2.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: trait `m::PrivNonPrincipal` is private
--> $DIR/private-in-public-non-principal-2.rs:11:5
|
LL | m::leak_dyn_nonprincipal();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

20 changes: 20 additions & 0 deletions src/test/ui/privacy/private-in-public-non-principal.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![feature(optin_builtin_traits)]

pub trait PubPrincipal {}
auto trait PrivNonPrincipal {}

pub fn leak_dyn_nonprincipal() -> Box<PubPrincipal + PrivNonPrincipal> { loop {} }
//~^ WARN private trait `PrivNonPrincipal` in public interface
//~| WARN this was previously accepted

#[deny(missing_docs)]
fn container() {
impl dyn PubPrincipal {
pub fn check_doc_lint() {} //~ ERROR missing documentation for a method
}
impl dyn PubPrincipal + PrivNonPrincipal {
pub fn check_doc_lint() {} // OK, no missing doc lint
}
}

fn main() {}
24 changes: 24 additions & 0 deletions src/test/ui/privacy/private-in-public-non-principal.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
warning: private trait `PrivNonPrincipal` in public interface (error E0445)
--> $DIR/private-in-public-non-principal.rs:6:1
|
LL | pub fn leak_dyn_nonprincipal() -> Box<PubPrincipal + PrivNonPrincipal> { loop {} }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= 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>

error: missing documentation for a method
--> $DIR/private-in-public-non-principal.rs:13:9
|
LL | pub fn check_doc_lint() {} //~ ERROR missing documentation for a method
| ^^^^^^^^^^^^^^^^^^^^^^^
|
note: lint level defined here
--> $DIR/private-in-public-non-principal.rs:10:8
|
LL | #[deny(missing_docs)]
| ^^^^^^^^^^^^

error: aborting due to previous error

9 changes: 9 additions & 0 deletions src/test/ui/privacy/private-in-public-warn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,15 @@ mod aliases_pub {
impl PrivUseAliasTr for <Priv as PrivTr>::AssocAlias {
type Check = Priv; //~ ERROR private type `aliases_pub::Priv` in public interface
}
impl PrivUseAliasTr for Option<<Priv as PrivTr>::AssocAlias> {
type Check = Priv; //~ ERROR private type `aliases_pub::Priv` in public interface
}
impl PrivUseAliasTr for (<Priv as PrivTr>::AssocAlias, Priv) {
type Check = Priv; // OK
}
impl PrivUseAliasTr for Option<(<Priv as PrivTr>::AssocAlias, Priv)> {
type Check = Priv; // OK
}
}

mod aliases_priv {
Expand Down
21 changes: 15 additions & 6 deletions src/test/ui/privacy/private-in-public-warn.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -297,33 +297,42 @@ LL | struct Priv;
LL | type Check = Priv; //~ ERROR private type `aliases_pub::Priv` in public interface
| ^^^^^^^^^^^^^^^^^^ can't leak private type

error[E0446]: private type `aliases_pub::Priv` in public interface
--> $DIR/private-in-public-warn.rs:217:9
|
LL | struct Priv;
| - `aliases_pub::Priv` declared as private
...
LL | type Check = Priv; //~ ERROR private type `aliases_pub::Priv` in public interface
| ^^^^^^^^^^^^^^^^^^ can't leak private type

error: private trait `aliases_priv::PrivTr1` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:238:5
--> $DIR/private-in-public-warn.rs:247:5
|
LL | pub trait Tr1: PrivUseAliasTr {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= 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 type `aliases_priv::Priv2` in public interface (error E0446)
--> $DIR/private-in-public-warn.rs:241:5
error: private trait `aliases_priv::PrivTr1<aliases_priv::Priv2>` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:250:5
|
LL | pub trait Tr2: PrivUseAliasTr<PrivAlias> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= 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 `aliases_priv::PrivTr1<aliases_priv::Priv2>` in public interface (error E0445)
--> $DIR/private-in-public-warn.rs:241:5
error: private type `aliases_priv::Priv2` in public interface (error E0446)
--> $DIR/private-in-public-warn.rs:250:5
|
LL | pub trait Tr2: PrivUseAliasTr<PrivAlias> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= 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: aborting due to 35 previous errors
error: aborting due to 36 previous errors

For more information about this error, try `rustc --explain E0446`.
4 changes: 2 additions & 2 deletions src/test/ui/privacy/private-in-public.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ mod aliases_pub {

// This should be OK, but associated type aliases are not substituted yet
pub fn f3(arg: <Priv as PrivTr>::Assoc) {}
//~^ ERROR private type `<aliases_pub::Priv as aliases_pub::PrivTr>::Assoc` in public interface
//~^ ERROR private trait `aliases_pub::PrivTr` in public interface
//~| ERROR private type `aliases_pub::Priv` in public interface

impl PrivUseAlias {
Expand Down Expand Up @@ -131,7 +131,7 @@ mod aliases_priv {
pub fn f1(arg: PrivUseAlias) {} //~ ERROR private type `aliases_priv::Priv1` in public interface
pub fn f2(arg: PrivAlias) {} //~ ERROR private type `aliases_priv::Priv2` in public interface
pub fn f3(arg: <Priv as PrivTr>::Assoc) {}
//~^ ERROR private type `<aliases_priv::Priv as aliases_priv::PrivTr>::Assoc` in public
//~^ ERROR private trait `aliases_priv::PrivTr` in public interface
//~| ERROR private type `aliases_priv::Priv` in public interface
}

Expand Down
Loading

0 comments on commit fe6a54d

Please sign in to comment.