From d326aed46f6fdc741fe78f29c948a4c358150094 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 15 Jun 2023 15:47:38 +0300 Subject: [PATCH 1/4] privacy: Feature gate new type privacy lints --- compiler/rustc_feature/src/active.rs | 2 + compiler/rustc_lint_defs/src/builtin.rs | 10 +- compiler/rustc_span/src/symbol.rs | 1 + .../private-in-public.rs | 2 +- .../generic_const_exprs/eval-privacy.rs | 2 +- tests/ui/error-codes/E0445.rs | 1 + tests/ui/error-codes/E0445.stderr | 20 ++-- .../feature-gate-type_privacy_lints.rs | 12 +++ .../feature-gate-type_privacy_lints.stderr | 93 +++++++++++++++++++ tests/ui/issues/issue-18389.rs | 1 + tests/ui/issues/issue-18389.stderr | 8 +- .../private-in-public-non-principal.rs | 2 +- tests/ui/privacy/unnameable_types.rs | 2 +- tests/ui/privacy/where-priv-type.rs | 2 +- .../where-pub-type-impls-priv-trait.rs | 2 +- ...174-restricted-type-in-public-interface.rs | 1 + ...restricted-type-in-public-interface.stderr | 20 ++-- 17 files changed, 149 insertions(+), 32 deletions(-) create mode 100644 tests/ui/feature-gates/feature-gate-type_privacy_lints.rs create mode 100644 tests/ui/feature-gates/feature-gate-type_privacy_lints.stderr diff --git a/compiler/rustc_feature/src/active.rs b/compiler/rustc_feature/src/active.rs index 4c53f9d8369fd..93b40c05a67f3 100644 --- a/compiler/rustc_feature/src/active.rs +++ b/compiler/rustc_feature/src/active.rs @@ -539,6 +539,8 @@ declare_features! ( /// Allows creation of instances of a struct by moving fields that have /// not changed from prior instances of the same struct (RFC #2528) (active, type_changing_struct_update, "1.58.0", Some(86555), None), + /// Allows using type privacy lints (`private_interfaces`, `private_bounds`, `unnameable_types`). + (active, type_privacy_lints, "CURRENT_RUSTC_VERSION", Some(48054), None), /// Enables rustc to generate code that instructs libstd to NOT ignore SIGPIPE. (active, unix_sigpipe, "1.65.0", Some(97889), None), /// Allows unsized fn parameters. diff --git a/compiler/rustc_lint_defs/src/builtin.rs b/compiler/rustc_lint_defs/src/builtin.rs index 53ece08ac3d92..65caa79ae138f 100644 --- a/compiler/rustc_lint_defs/src/builtin.rs +++ b/compiler/rustc_lint_defs/src/builtin.rs @@ -4263,6 +4263,7 @@ declare_lint! { /// ### Example /// /// ```rust,compile_fail + /// # #![feature(type_privacy_lints)] /// # #![allow(unused)] /// # #![allow(private_in_public)] /// #![deny(private_interfaces)] @@ -4287,6 +4288,7 @@ declare_lint! { pub PRIVATE_INTERFACES, Allow, "private type in primary interface of an item", + @feature_gate = sym::type_privacy_lints; } declare_lint! { @@ -4297,6 +4299,7 @@ declare_lint! { /// ### Example /// /// ```rust,compile_fail + /// # #![feature(type_privacy_lints)] /// # #![allow(private_in_public)] /// # #![allow(unused)] /// #![deny(private_bounds)] @@ -4316,7 +4319,8 @@ declare_lint! { /// the item actually provides. pub PRIVATE_BOUNDS, Allow, - "private type in secondary interface of an item" + "private type in secondary interface of an item", + @feature_gate = sym::type_privacy_lints; } declare_lint! { @@ -4326,6 +4330,7 @@ declare_lint! { /// ### Example /// /// ```rust,compile_fail + /// # #![feature(type_privacy_lints)] /// # #![allow(unused)] /// #![deny(unnameable_types)] /// mod m { @@ -4344,5 +4349,6 @@ declare_lint! { /// you can name the type `T` as well, this lint attempts to enforce this rule. pub UNNAMEABLE_TYPES, Allow, - "effective visibility of a type is larger than the area in which it can be named" + "effective visibility of a type is larger than the area in which it can be named", + @feature_gate = sym::type_privacy_lints; } diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index c5ce2575fff06..b98761fafb084 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -1555,6 +1555,7 @@ symbols! { type_length_limit, type_macros, type_name, + type_privacy_lints, u128, u16, u32, diff --git a/tests/ui/associated-inherent-types/private-in-public.rs b/tests/ui/associated-inherent-types/private-in-public.rs index 44a20a79ad625..33b84fc9506b2 100644 --- a/tests/ui/associated-inherent-types/private-in-public.rs +++ b/tests/ui/associated-inherent-types/private-in-public.rs @@ -1,7 +1,7 @@ #![feature(inherent_associated_types)] +#![feature(type_privacy_lints)] #![allow(incomplete_features)] #![crate_type = "lib"] - #![deny(private_in_public)] #![warn(private_interfaces)] diff --git a/tests/ui/const-generics/generic_const_exprs/eval-privacy.rs b/tests/ui/const-generics/generic_const_exprs/eval-privacy.rs index 96b769699ccd4..5c43213e898a0 100644 --- a/tests/ui/const-generics/generic_const_exprs/eval-privacy.rs +++ b/tests/ui/const-generics/generic_const_exprs/eval-privacy.rs @@ -1,7 +1,7 @@ #![crate_type = "lib"] #![feature(generic_const_exprs)] +#![feature(type_privacy_lints)] #![allow(incomplete_features)] - #![warn(private_interfaces)] // In this test both old and new private-in-public diagnostic were emitted. diff --git a/tests/ui/error-codes/E0445.rs b/tests/ui/error-codes/E0445.rs index f5f35fb8a4d2d..30c222ae6f21d 100644 --- a/tests/ui/error-codes/E0445.rs +++ b/tests/ui/error-codes/E0445.rs @@ -1,3 +1,4 @@ +#![feature(type_privacy_lints)] #[warn(private_bounds)] #[warn(private_interfaces)] diff --git a/tests/ui/error-codes/E0445.stderr b/tests/ui/error-codes/E0445.stderr index ac3637a821858..ba2c21485ef9d 100644 --- a/tests/ui/error-codes/E0445.stderr +++ b/tests/ui/error-codes/E0445.stderr @@ -1,5 +1,5 @@ error[E0445]: private trait `Foo` in public interface - --> $DIR/E0445.rs:12:1 + --> $DIR/E0445.rs:13:1 | LL | trait Foo { | --------- `Foo` declared as private @@ -10,23 +10,23 @@ LL | pub trait Bar : Foo {} warning: trait `Foo` is more private than the item `Bar` | note: trait `Bar` is reachable at visibility `pub` - --> $DIR/E0445.rs:12:1 + --> $DIR/E0445.rs:13:1 | LL | pub trait Bar : Foo {} | ^^^^^^^^^^^^^^^^^^^ note: but trait `Foo` is only usable at visibility `pub(crate)` - --> $DIR/E0445.rs:8:1 + --> $DIR/E0445.rs:9:1 | LL | trait Foo { | ^^^^^^^^^ note: the lint level is defined here - --> $DIR/E0445.rs:1:8 + --> $DIR/E0445.rs:2:8 | LL | #[warn(private_bounds)] | ^^^^^^^^^^^^^^ error[E0445]: private trait `Foo` in public interface - --> $DIR/E0445.rs:14:1 + --> $DIR/E0445.rs:15:1 | LL | trait Foo { | --------- `Foo` declared as private @@ -37,18 +37,18 @@ LL | pub struct Bar2(pub T); warning: trait `Foo` is more private than the item `Bar2` | note: struct `Bar2` is reachable at visibility `pub` - --> $DIR/E0445.rs:14:1 + --> $DIR/E0445.rs:15:1 | LL | pub struct Bar2(pub T); | ^^^^^^^^^^^^^^^^^^^^^^^ note: but trait `Foo` is only usable at visibility `pub(crate)` - --> $DIR/E0445.rs:8:1 + --> $DIR/E0445.rs:9:1 | LL | trait Foo { | ^^^^^^^^^ error[E0445]: private trait `Foo` in public interface - --> $DIR/E0445.rs:16:1 + --> $DIR/E0445.rs:17:1 | LL | trait Foo { | --------- `Foo` declared as private @@ -59,12 +59,12 @@ LL | pub fn foo (t: T) {} warning: trait `Foo` is more private than the item `foo` | note: function `foo` is reachable at visibility `pub` - --> $DIR/E0445.rs:16:1 + --> $DIR/E0445.rs:17:1 | LL | pub fn foo (t: T) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^ note: but trait `Foo` is only usable at visibility `pub(crate)` - --> $DIR/E0445.rs:8:1 + --> $DIR/E0445.rs:9:1 | LL | trait Foo { | ^^^^^^^^^ diff --git a/tests/ui/feature-gates/feature-gate-type_privacy_lints.rs b/tests/ui/feature-gates/feature-gate-type_privacy_lints.rs new file mode 100644 index 0000000000000..aad64c9d07326 --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-type_privacy_lints.rs @@ -0,0 +1,12 @@ +// check-pass + +#![warn(private_interfaces)] //~ WARN unknown lint + //~| WARN unknown lint + //~| WARN unknown lint +#![warn(private_bounds)] //~ WARN unknown lint + //~| WARN unknown lint + //~| WARN unknown lint +#![warn(unnameable_types)] //~ WARN unknown lint + //~| WARN unknown lint + //~| WARN unknown lint +fn main() {} diff --git a/tests/ui/feature-gates/feature-gate-type_privacy_lints.stderr b/tests/ui/feature-gates/feature-gate-type_privacy_lints.stderr new file mode 100644 index 0000000000000..79cc974cca122 --- /dev/null +++ b/tests/ui/feature-gates/feature-gate-type_privacy_lints.stderr @@ -0,0 +1,93 @@ +warning: unknown lint: `private_interfaces` + --> $DIR/feature-gate-type_privacy_lints.rs:3:1 + | +LL | #![warn(private_interfaces)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `private_interfaces` lint is unstable + = note: see issue #48054 for more information + = help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable + = note: `#[warn(unknown_lints)]` on by default + +warning: unknown lint: `private_bounds` + --> $DIR/feature-gate-type_privacy_lints.rs:6:1 + | +LL | #![warn(private_bounds)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `private_bounds` lint is unstable + = note: see issue #48054 for more information + = help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable + +warning: unknown lint: `unnameable_types` + --> $DIR/feature-gate-type_privacy_lints.rs:9:1 + | +LL | #![warn(unnameable_types)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `unnameable_types` lint is unstable + = note: see issue #48054 for more information + = help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable + +warning: unknown lint: `private_interfaces` + --> $DIR/feature-gate-type_privacy_lints.rs:3:1 + | +LL | #![warn(private_interfaces)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `private_interfaces` lint is unstable + = note: see issue #48054 for more information + = help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable + +warning: unknown lint: `private_bounds` + --> $DIR/feature-gate-type_privacy_lints.rs:6:1 + | +LL | #![warn(private_bounds)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `private_bounds` lint is unstable + = note: see issue #48054 for more information + = help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable + +warning: unknown lint: `unnameable_types` + --> $DIR/feature-gate-type_privacy_lints.rs:9:1 + | +LL | #![warn(unnameable_types)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `unnameable_types` lint is unstable + = note: see issue #48054 for more information + = help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable + +warning: unknown lint: `private_interfaces` + --> $DIR/feature-gate-type_privacy_lints.rs:3:1 + | +LL | #![warn(private_interfaces)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `private_interfaces` lint is unstable + = note: see issue #48054 for more information + = help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable + +warning: unknown lint: `private_bounds` + --> $DIR/feature-gate-type_privacy_lints.rs:6:1 + | +LL | #![warn(private_bounds)] + | ^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `private_bounds` lint is unstable + = note: see issue #48054 for more information + = help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable + +warning: unknown lint: `unnameable_types` + --> $DIR/feature-gate-type_privacy_lints.rs:9:1 + | +LL | #![warn(unnameable_types)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: the `unnameable_types` lint is unstable + = note: see issue #48054 for more information + = help: add `#![feature(type_privacy_lints)]` to the crate attributes to enable + +warning: 9 warnings emitted + diff --git a/tests/ui/issues/issue-18389.rs b/tests/ui/issues/issue-18389.rs index 3686afc48af8d..0af693c4e0fe2 100644 --- a/tests/ui/issues/issue-18389.rs +++ b/tests/ui/issues/issue-18389.rs @@ -1,3 +1,4 @@ +#![feature(type_privacy_lints)] #![warn(private_bounds)] // In this test both old and new private-in-public diagnostic were emitted. diff --git a/tests/ui/issues/issue-18389.stderr b/tests/ui/issues/issue-18389.stderr index f9ebde48a4522..6b78151c60682 100644 --- a/tests/ui/issues/issue-18389.stderr +++ b/tests/ui/issues/issue-18389.stderr @@ -1,5 +1,5 @@ error[E0445]: private trait `Private<::P, ::R>` in public interface - --> $DIR/issue-18389.rs:13:1 + --> $DIR/issue-18389.rs:14:1 | LL | trait Private { | ------------------- `Private<::P, ::R>` declared as private @@ -14,7 +14,7 @@ LL | | > { warning: trait `Private<::P, ::R>` is more private than the item `Public` | note: trait `Public` is reachable at visibility `pub` - --> $DIR/issue-18389.rs:13:1 + --> $DIR/issue-18389.rs:14:1 | LL | / pub trait Public: Private< LL | | @@ -23,12 +23,12 @@ LL | | ::R LL | | > { | |_^ note: but trait `Private<::P, ::R>` is only usable at visibility `pub(crate)` - --> $DIR/issue-18389.rs:10:1 + --> $DIR/issue-18389.rs:11:1 | LL | trait Private { | ^^^^^^^^^^^^^^^^^^^ note: the lint level is defined here - --> $DIR/issue-18389.rs:1:9 + --> $DIR/issue-18389.rs:2:9 | LL | #![warn(private_bounds)] | ^^^^^^^^^^^^^^ diff --git a/tests/ui/privacy/private-in-public-non-principal.rs b/tests/ui/privacy/private-in-public-non-principal.rs index a80c154146378..9ae7512c5092d 100644 --- a/tests/ui/privacy/private-in-public-non-principal.rs +++ b/tests/ui/privacy/private-in-public-non-principal.rs @@ -1,6 +1,6 @@ #![feature(auto_traits)] #![feature(negative_impls)] - +#![feature(type_privacy_lints)] #![deny(private_interfaces)] // In this test both old and new private-in-public diagnostic were emitted. diff --git a/tests/ui/privacy/unnameable_types.rs b/tests/ui/privacy/unnameable_types.rs index 8b53f372fc937..eae20dd9df374 100644 --- a/tests/ui/privacy/unnameable_types.rs +++ b/tests/ui/privacy/unnameable_types.rs @@ -1,4 +1,4 @@ -#![allow(unused)] +#![feature(type_privacy_lints)] #![allow(private_in_public)] #![deny(unnameable_types)] diff --git a/tests/ui/privacy/where-priv-type.rs b/tests/ui/privacy/where-priv-type.rs index 9899902dd884f..468d6e98a74d9 100644 --- a/tests/ui/privacy/where-priv-type.rs +++ b/tests/ui/privacy/where-priv-type.rs @@ -3,8 +3,8 @@ #![crate_type = "lib"] #![feature(generic_const_exprs)] +#![feature(type_privacy_lints)] #![allow(incomplete_features)] - #![warn(private_bounds)] #![warn(private_interfaces)] diff --git a/tests/ui/privacy/where-pub-type-impls-priv-trait.rs b/tests/ui/privacy/where-pub-type-impls-priv-trait.rs index 3aad893eae2bf..35e33bcb3b715 100644 --- a/tests/ui/privacy/where-pub-type-impls-priv-trait.rs +++ b/tests/ui/privacy/where-pub-type-impls-priv-trait.rs @@ -2,8 +2,8 @@ #![crate_type = "lib"] #![feature(generic_const_exprs)] +#![feature(type_privacy_lints)] #![allow(incomplete_features)] - #![warn(private_bounds)] // In this test both old and new private-in-public diagnostic were emitted. diff --git a/tests/ui/pub/issue-33174-restricted-type-in-public-interface.rs b/tests/ui/pub/issue-33174-restricted-type-in-public-interface.rs index 9e4ba80a784a5..bd8f6585f4820 100644 --- a/tests/ui/pub/issue-33174-restricted-type-in-public-interface.rs +++ b/tests/ui/pub/issue-33174-restricted-type-in-public-interface.rs @@ -1,3 +1,4 @@ +#![feature(type_privacy_lints)] #![allow(non_camel_case_types)] // genus is always capitalized #![warn(private_interfaces)] //~^ NOTE the lint level is defined here diff --git a/tests/ui/pub/issue-33174-restricted-type-in-public-interface.stderr b/tests/ui/pub/issue-33174-restricted-type-in-public-interface.stderr index 52f67d4cdd504..5ebda47558ca4 100644 --- a/tests/ui/pub/issue-33174-restricted-type-in-public-interface.stderr +++ b/tests/ui/pub/issue-33174-restricted-type-in-public-interface.stderr @@ -1,5 +1,5 @@ error[E0446]: private type `Snail` in public interface - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:27:1 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:28:1 | LL | pub(crate) struct Snail; | ----------------------- `Snail` declared as private @@ -10,23 +10,23 @@ LL | pub type Helix_pomatia = Shell; warning: type `Snail` is more private than the item `Helix_pomatia` | note: type alias `Helix_pomatia` is reachable at visibility `pub` - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:27:1 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:28:1 | LL | pub type Helix_pomatia = Shell; | ^^^^^^^^^^^^^^^^^^^^^^ note: but type `Snail` is only usable at visibility `pub(crate)` - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:9:1 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:10:1 | LL | pub(crate) struct Snail; | ^^^^^^^^^^^^^^^^^^^^^^^ note: the lint level is defined here - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:2:9 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:3:9 | LL | #![warn(private_interfaces)] | ^^^^^^^^^^^^^^^^^^ error[E0446]: crate-private type `Turtle` in public interface - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:31:1 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:32:1 | LL | pub(super) struct Turtle; | ------------------------ `Turtle` declared as crate-private @@ -37,18 +37,18 @@ LL | pub type Dermochelys_coriacea = Shell; warning: type `Turtle` is more private than the item `Dermochelys_coriacea` | note: type alias `Dermochelys_coriacea` is reachable at visibility `pub` - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:31:1 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:32:1 | LL | pub type Dermochelys_coriacea = Shell; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: but type `Turtle` is only usable at visibility `pub(crate)` - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:14:5 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:15:5 | LL | pub(super) struct Turtle; | ^^^^^^^^^^^^^^^^^^^^^^^^ error[E0446]: private type `Tortoise` in public interface - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:35:1 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:36:1 | LL | struct Tortoise; | --------------- `Tortoise` declared as private @@ -59,12 +59,12 @@ LL | pub type Testudo_graeca = Shell; warning: type `Tortoise` is more private than the item `Testudo_graeca` | note: type alias `Testudo_graeca` is reachable at visibility `pub` - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:35:1 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:36:1 | LL | pub type Testudo_graeca = Shell; | ^^^^^^^^^^^^^^^^^^^^^^^ note: but type `Tortoise` is only usable at visibility `pub(crate)` - --> $DIR/issue-33174-restricted-type-in-public-interface.rs:19:1 + --> $DIR/issue-33174-restricted-type-in-public-interface.rs:20:1 | LL | struct Tortoise; | ^^^^^^^^^^^^^^^ From 17edd1a77907c4c4c03c0c4e22de9e4ec7dd9243 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 15 Jun 2023 17:15:24 +0300 Subject: [PATCH 2/4] privacy: Remove `(Non)ShallowEffectiveVis` Use a boolean constant parameter instead. Also turn some methods on `DefIdVisitor` into associated constants. --- compiler/rustc_privacy/src/lib.rs | 145 +++++++++++++----------------- 1 file changed, 61 insertions(+), 84 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index a51a1c9a8a436..11b024d7fc9de 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -73,14 +73,10 @@ impl<'tcx> fmt::Display for LazyDefPathStr<'tcx> { /// in `impl Trait`, see individual comments in `DefIdVisitorSkeleton::visit_ty`. trait DefIdVisitor<'tcx> { type BreakTy = (); + const SHALLOW: bool = false; + const SKIP_ASSOC_TYS: bool = false; fn tcx(&self) -> TyCtxt<'tcx>; - fn shallow(&self) -> bool { - false - } - fn skip_assoc_tys(&self) -> bool { - false - } fn visit_def_id( &mut self, def_id: DefId, @@ -129,11 +125,7 @@ where fn visit_trait(&mut self, trait_ref: TraitRef<'tcx>) -> ControlFlow { let TraitRef { def_id, substs, .. } = trait_ref; self.def_id_visitor.visit_def_id(def_id, "trait", &trait_ref.print_only_trait_path())?; - if self.def_id_visitor.shallow() { - ControlFlow::Continue(()) - } else { - substs.visit_with(self) - } + if V::SHALLOW { ControlFlow::Continue(()) } else { substs.visit_with(self) } } fn visit_projection_ty(&mut self, projection: ty::AliasTy<'tcx>) -> ControlFlow { @@ -152,7 +144,7 @@ where ) }; self.visit_trait(trait_ref)?; - if self.def_id_visitor.shallow() { + if V::SHALLOW { ControlFlow::Continue(()) } else { assoc_substs.iter().try_for_each(|subst| subst.visit_with(self)) @@ -222,7 +214,7 @@ where | ty::Closure(def_id, ..) | ty::Generator(def_id, ..) => { self.def_id_visitor.visit_def_id(def_id, "type", &ty)?; - if self.def_id_visitor.shallow() { + if V::SHALLOW { return ControlFlow::Continue(()); } // Default type visitor doesn't visit signatures of fn types. @@ -243,7 +235,7 @@ where } } ty::Alias(ty::Projection, proj) => { - if self.def_id_visitor.skip_assoc_tys() { + if V::SKIP_ASSOC_TYS { // Visitors searching for minimal visibility/reachability want to // conservatively approximate associated types like `::Alias` // as visible/reachable even if both `Type` and `Trait` are private. @@ -255,7 +247,7 @@ where return self.visit_projection_ty(proj); } ty::Alias(ty::Inherent, data) => { - if self.def_id_visitor.skip_assoc_tys() { + if V::SKIP_ASSOC_TYS { // Visitors searching for minimal visibility/reachability want to // conservatively approximate associated types like `Type::Alias` // as visible/reachable even if `Type` is private. @@ -271,7 +263,7 @@ where )?; // This will also visit substs if necessary, so we don't need to recurse. - return if self.def_id_visitor.shallow() { + return if V::SHALLOW { ControlFlow::Continue(()) } else { data.substs.iter().try_for_each(|subst| subst.visit_with(self)) @@ -333,11 +325,7 @@ where } } - if self.def_id_visitor.shallow() { - ControlFlow::Continue(()) - } else { - ty.super_visit_with(self) - } + if V::SHALLOW { ControlFlow::Continue(()) } else { ty.super_visit_with(self) } } fn visit_const(&mut self, c: Const<'tcx>) -> ControlFlow { @@ -354,22 +342,20 @@ fn min(vis1: ty::Visibility, vis2: ty::Visibility, tcx: TyCtxt<'_>) -> ty::Visib /// Visitor used to determine impl visibility and reachability. //////////////////////////////////////////////////////////////////////////////// -struct FindMin<'a, 'tcx, VL: VisibilityLike> { +struct FindMin<'a, 'tcx, VL: VisibilityLike, const SHALLOW: bool> { tcx: TyCtxt<'tcx>, effective_visibilities: &'a EffectiveVisibilities, min: VL, } -impl<'a, 'tcx, VL: VisibilityLike> DefIdVisitor<'tcx> for FindMin<'a, 'tcx, VL> { +impl<'a, 'tcx, VL: VisibilityLike, const SHALLOW: bool> DefIdVisitor<'tcx> + for FindMin<'a, 'tcx, VL, SHALLOW> +{ + const SHALLOW: bool = SHALLOW; + const SKIP_ASSOC_TYS: bool = true; fn tcx(&self) -> TyCtxt<'tcx> { self.tcx } - fn shallow(&self) -> bool { - VL::SHALLOW - } - fn skip_assoc_tys(&self) -> bool { - true - } fn visit_def_id( &mut self, def_id: DefId, @@ -385,17 +371,19 @@ impl<'a, 'tcx, VL: VisibilityLike> DefIdVisitor<'tcx> for FindMin<'a, 'tcx, VL> trait VisibilityLike: Sized { const MAX: Self; - const SHALLOW: bool = false; - fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self; + fn new_min( + find: &FindMin<'_, '_, Self, SHALLOW>, + def_id: LocalDefId, + ) -> Self; - // Returns an over-approximation (`skip_assoc_tys` = true) of visibility due to + // Returns an over-approximation (`SKIP_ASSOC_TYS` = true) of visibility due to // associated types for which we can't determine visibility precisely. - fn of_impl( + fn of_impl( def_id: LocalDefId, tcx: TyCtxt<'_>, effective_visibilities: &EffectiveVisibilities, ) -> Self { - let mut find = FindMin { tcx, effective_visibilities, min: Self::MAX }; + let mut find = FindMin::<_, SHALLOW> { tcx, effective_visibilities, min: Self::MAX }; find.visit(tcx.type_of(def_id).subst_identity()); if let Some(trait_ref) = tcx.impl_trait_ref(def_id) { find.visit_trait(trait_ref.subst_identity()); @@ -405,49 +393,28 @@ trait VisibilityLike: Sized { } impl VisibilityLike for ty::Visibility { const MAX: Self = ty::Visibility::Public; - fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self { + fn new_min( + find: &FindMin<'_, '_, Self, SHALLOW>, + def_id: LocalDefId, + ) -> Self { min(find.tcx.local_visibility(def_id), find.min, find.tcx) } } -struct NonShallowEffectiveVis(EffectiveVisibility); - -impl VisibilityLike for NonShallowEffectiveVis { - const MAX: Self = NonShallowEffectiveVis(EffectiveVisibility::from_vis(ty::Visibility::Public)); - const SHALLOW: bool = false; - - fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self { - let find = FindMin { - tcx: find.tcx, - effective_visibilities: find.effective_visibilities, - min: ShallowEffectiveVis(find.min.0), - }; - NonShallowEffectiveVis(VisibilityLike::new_min(&find, def_id).0) - } -} - -struct ShallowEffectiveVis(EffectiveVisibility); -impl VisibilityLike for ShallowEffectiveVis { - const MAX: Self = ShallowEffectiveVis(EffectiveVisibility::from_vis(ty::Visibility::Public)); - // Type inference is very smart sometimes. - // It can make an impl reachable even some components of its type or trait are unreachable. - // E.g. methods of `impl ReachableTrait for ReachableTy { ... }` - // can be usable from other crates (#57264). So we skip substs when calculating reachability - // and consider an impl reachable if its "shallow" type and trait are reachable. - // - // The assumption we make here is that type-inference won't let you use an impl without knowing - // both "shallow" version of its self type and "shallow" version of its trait if it exists - // (which require reaching the `DefId`s in them). - const SHALLOW: bool = true; - fn new_min(find: &FindMin<'_, '_, Self>, def_id: LocalDefId) -> Self { +impl VisibilityLike for EffectiveVisibility { + const MAX: Self = EffectiveVisibility::from_vis(ty::Visibility::Public); + fn new_min( + find: &FindMin<'_, '_, Self, SHALLOW>, + def_id: LocalDefId, + ) -> Self { let effective_vis = - find.effective_visibilities.effective_vis(def_id).cloned().unwrap_or_else(|| { + find.effective_visibilities.effective_vis(def_id).copied().unwrap_or_else(|| { let private_vis = ty::Visibility::Restricted(find.tcx.parent_module_from_def_id(def_id)); EffectiveVisibility::from_vis(private_vis) }); - ShallowEffectiveVis(effective_vis.min(find.min.0, find.tcx)) + effective_vis.min(find.min, find.tcx) } } @@ -785,12 +752,21 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> { } } hir::ItemKind::Impl(ref impl_) => { - let item_ev = ShallowEffectiveVis::of_impl( + // Type inference is very smart sometimes. It can make an impl reachable even some + // components of its type or trait are unreachable. E.g. methods of + // `impl ReachableTrait for ReachableTy { ... }` + // can be usable from other crates (#57264). So we skip substs when calculating + // reachability and consider an impl reachable if its "shallow" type and trait are + // reachable. + // + // The assumption we make here is that type-inference won't let you use an impl + // without knowing both "shallow" version of its self type and "shallow" version of + // its trait if it exists (which require reaching the `DefId`s in them). + let item_ev = EffectiveVisibility::of_impl::( item.owner_id.def_id, self.tcx, &self.effective_visibilities, - ) - .0; + ); self.update_eff_vis(item.owner_id.def_id, item_ev, None, Level::Direct); @@ -2154,27 +2130,28 @@ impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx, '_> { DefKind::Impl { .. } => { let item = tcx.hir().item(id); if let hir::ItemKind::Impl(ref impl_) = item.kind { - let impl_vis = - ty::Visibility::of_impl(item.owner_id.def_id, tcx, &Default::default()); + let impl_vis = ty::Visibility::of_impl::( + item.owner_id.def_id, + tcx, + &Default::default(), + ); - // we are using the non-shallow version here, unlike when building the + // We are using the non-shallow version here, unlike when building the // effective visisibilities table to avoid large number of false positives. - // For example: + // For example in // // impl From for Pub { // fn from(_: Priv) -> Pub {...} // } // - // lints shouldn't be emmited even `from` effective visibility - // is larger then `Priv` nominal visibility. - let impl_ev = Some( - NonShallowEffectiveVis::of_impl( - item.owner_id.def_id, - tcx, - self.effective_visibilities, - ) - .0, - ); + // lints shouldn't be emmited even if `from` effective visibility + // is larger than `Priv` nominal visibility and if `Priv` can leak + // in some scenarios due to type inference. + let impl_ev = Some(EffectiveVisibility::of_impl::( + item.owner_id.def_id, + tcx, + self.effective_visibilities, + )); // 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 From 95a24c6ed489de0fa6e87ef99390a744c8d46ddc Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 15 Jun 2023 19:07:51 +0300 Subject: [PATCH 3/4] privacy: Do not mark items reachable farther than their nominal visibility This commit reverts a change made in #111425. It was believed that this change was necessary for implementing type privacy lints, but #111801 showed that it was not necessary. Quite opposite, the revert fixes some issues. --- compiler/rustc_lint/src/builtin.rs | 8 ++--- compiler/rustc_middle/src/middle/privacy.rs | 29 ++++++++++--------- compiler/rustc_privacy/src/lib.rs | 18 +++++++----- library/core/src/iter/adapters/flatten.rs | 12 ++++---- .../effective_visibilities_full_priv.rs | 2 +- .../effective_visibilities_full_priv.stderr | 2 +- 6 files changed, 37 insertions(+), 34 deletions(-) diff --git a/compiler/rustc_lint/src/builtin.rs b/compiler/rustc_lint/src/builtin.rs index 7b05bff515148..e15bd98ed96f1 100644 --- a/compiler/rustc_lint/src/builtin.rs +++ b/compiler/rustc_lint/src/builtin.rs @@ -665,9 +665,7 @@ declare_lint_pass!(MissingCopyImplementations => [MISSING_COPY_IMPLEMENTATIONS]) impl<'tcx> LateLintPass<'tcx> for MissingCopyImplementations { fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) { - if !(cx.effective_visibilities.is_reachable(item.owner_id.def_id) - && cx.tcx.local_visibility(item.owner_id.def_id).is_public()) - { + if !cx.effective_visibilities.is_reachable(item.owner_id.def_id) { return; } let (def, ty) = match item.kind { @@ -786,9 +784,7 @@ impl_lint_pass!(MissingDebugImplementations => [MISSING_DEBUG_IMPLEMENTATIONS]); impl<'tcx> LateLintPass<'tcx> for MissingDebugImplementations { fn check_item(&mut self, cx: &LateContext<'_>, item: &hir::Item<'_>) { - if !(cx.effective_visibilities.is_reachable(item.owner_id.def_id) - && cx.tcx.local_visibility(item.owner_id.def_id).is_public()) - { + if !cx.effective_visibilities.is_reachable(item.owner_id.def_id) { return; } diff --git a/compiler/rustc_middle/src/middle/privacy.rs b/compiler/rustc_middle/src/middle/privacy.rs index f45cf788dd91c..a0b7c8de600f8 100644 --- a/compiler/rustc_middle/src/middle/privacy.rs +++ b/compiler/rustc_middle/src/middle/privacy.rs @@ -4,6 +4,7 @@ use crate::ty::{TyCtxt, Visibility}; use rustc_data_structures::fx::FxHashMap; use rustc_data_structures::stable_hasher::{HashStable, StableHasher}; +use rustc_hir::def::DefKind; use rustc_macros::HashStable; use rustc_query_system::ich::StableHashingContext; use rustc_span::def_id::{LocalDefId, CRATE_DEF_ID}; @@ -148,13 +149,12 @@ impl EffectiveVisibilities { }; } - pub fn check_invariants(&self, tcx: TyCtxt<'_>, early: bool) { + pub fn check_invariants(&self, tcx: TyCtxt<'_>) { if !cfg!(debug_assertions) { return; } for (&def_id, ev) in &self.map { // More direct visibility levels can never go farther than less direct ones, - // neither of effective visibilities can go farther than nominal visibility, // and all effective visibilities are larger or equal than private visibility. let private_vis = Visibility::Restricted(tcx.parent_module_from_def_id(def_id)); let span = tcx.def_span(def_id.to_def_id()); @@ -175,17 +175,20 @@ impl EffectiveVisibilities { ev.reachable_through_impl_trait ); } - let nominal_vis = tcx.visibility(def_id); - // FIXME: `rustc_privacy` is not yet updated for the new logic and can set - // effective visibilities that are larger than the nominal one. - if !nominal_vis.is_at_least(ev.reachable_through_impl_trait, tcx) && early { - span_bug!( - span, - "{:?}: reachable_through_impl_trait {:?} > nominal {:?}", - def_id, - ev.reachable_through_impl_trait, - nominal_vis - ); + // All effective visibilities except `reachable_through_impl_trait` are limited to + // nominal visibility. For some items nominal visibility doesn't make sense so we + // don't check this condition for them. + if !matches!(tcx.def_kind(def_id), DefKind::Impl { .. }) { + let nominal_vis = tcx.visibility(def_id); + if !nominal_vis.is_at_least(ev.reachable, tcx) { + span_bug!( + span, + "{:?}: reachable {:?} > nominal {:?}", + def_id, + ev.reachable, + nominal_vis + ); + } } } } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 11b024d7fc9de..cea255713ba86 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -894,7 +894,12 @@ impl<'tcx> DefIdVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'_, 'tcx> _descr: &dyn fmt::Display, ) -> ControlFlow { if let Some(def_id) = def_id.as_local() { - self.ev.update_eff_vis(def_id, self.effective_vis, None, self.level); + // All effective visibilities except `reachable_through_impl_trait` are limited to + // nominal visibility. If any type or trait is leaked farther than that, it will + // produce type privacy errors on any use, so we don't consider it leaked. + let nominal_vis = (self.level != Level::ReachableThroughImplTrait) + .then(|| self.ev.tcx.local_visibility(def_id)); + self.ev.update_eff_vis(def_id, self.effective_vis, nominal_vis, self.level); } ControlFlow::Continue(()) } @@ -1869,10 +1874,9 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { return false; }; - // FIXME: `Level::Reachable` should be taken instead of `Level::Reexported` - let reexported_at_vis = *effective_vis.at_level(Level::Reexported); + let reachable_at_vis = *effective_vis.at_level(Level::Reachable); - if !vis.is_at_least(reexported_at_vis, self.tcx) { + if !vis.is_at_least(reachable_at_vis, self.tcx) { let lint = if self.in_primary_interface { lint::builtin::PRIVATE_INTERFACES } else { @@ -1889,7 +1893,7 @@ impl SearchInterfaceForPrivateItemsVisitor<'_> { tcx: self.tcx, }) .into(), - item_vis_descr: &vis_to_string(self.item_def_id, reexported_at_vis, self.tcx), + item_vis_descr: &vis_to_string(self.item_def_id, reachable_at_vis, self.tcx), ty_span: vis_span, ty_kind: kind, ty_descr: descr.into(), @@ -2278,7 +2282,7 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities { changed: false, }; - visitor.effective_visibilities.check_invariants(tcx, true); + visitor.effective_visibilities.check_invariants(tcx); if visitor.impl_trait_pass { // Underlying types of `impl Trait`s are marked as reachable unconditionally, // so this pass doesn't need to be a part of the fixed point iteration below. @@ -2295,7 +2299,7 @@ fn effective_visibilities(tcx: TyCtxt<'_>, (): ()) -> &EffectiveVisibilities { break; } } - visitor.effective_visibilities.check_invariants(tcx, false); + visitor.effective_visibilities.check_invariants(tcx); let mut check_visitor = TestReachabilityVisitor { tcx, effective_visibilities: &visitor.effective_visibilities }; diff --git a/library/core/src/iter/adapters/flatten.rs b/library/core/src/iter/adapters/flatten.rs index 2568aaf34f3fb..d3e4545635100 100644 --- a/library/core/src/iter/adapters/flatten.rs +++ b/library/core/src/iter/adapters/flatten.rs @@ -310,7 +310,7 @@ where /// Real logic of both `Flatten` and `FlatMap` which simply delegate to /// this type. #[derive(Clone, Debug)] -#[unstable(feature = "trusted_len", issue = "37572")] +#[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))] struct FlattenCompat { iter: Fuse, frontiter: Option, @@ -464,7 +464,7 @@ where } } -#[unstable(feature = "trusted_len", issue = "37572")] +#[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))] impl Iterator for FlattenCompat where I: Iterator>, @@ -579,7 +579,7 @@ where } } -#[unstable(feature = "trusted_len", issue = "37572")] +#[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))] impl DoubleEndedIterator for FlattenCompat where I: DoubleEndedIterator>, @@ -649,7 +649,7 @@ where } } -#[unstable(feature = "trusted_len", issue = "37572")] +#[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))] unsafe impl TrustedLen for FlattenCompat::IntoIter> where @@ -657,7 +657,7 @@ where { } -#[unstable(feature = "trusted_len", issue = "37572")] +#[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))] unsafe impl<'a, const N: usize, I, T> TrustedLen for FlattenCompat::IntoIter> where @@ -665,7 +665,7 @@ where { } -#[unstable(feature = "trusted_len", issue = "37572")] +#[cfg_attr(bootstrap, unstable(feature = "trusted_len", issue = "37572"))] unsafe impl<'a, const N: usize, I, T> TrustedLen for FlattenCompat::IntoIter> where diff --git a/tests/ui/privacy/effective_visibilities_full_priv.rs b/tests/ui/privacy/effective_visibilities_full_priv.rs index cc708917586ff..a26ae3bd12264 100644 --- a/tests/ui/privacy/effective_visibilities_full_priv.rs +++ b/tests/ui/privacy/effective_visibilities_full_priv.rs @@ -6,7 +6,7 @@ struct SemiPriv; mod m { #[rustc_effective_visibility] struct Priv; - //~^ ERROR Direct: pub(self), Reexported: pub(self), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate) + //~^ ERROR not in the table //~| ERROR not in the table #[rustc_effective_visibility] diff --git a/tests/ui/privacy/effective_visibilities_full_priv.stderr b/tests/ui/privacy/effective_visibilities_full_priv.stderr index a856aa20d92c5..29d82e2ee01c1 100644 --- a/tests/ui/privacy/effective_visibilities_full_priv.stderr +++ b/tests/ui/privacy/effective_visibilities_full_priv.stderr @@ -1,4 +1,4 @@ -error: Direct: pub(self), Reexported: pub(self), Reachable: pub(crate), ReachableThroughImplTrait: pub(crate) +error: not in the table --> $DIR/effective_visibilities_full_priv.rs:8:5 | LL | struct Priv; From 98a86ffc9ffbbd8a94ba219b9cddc3d20ba23f4d Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 15 Jun 2023 20:43:20 +0300 Subject: [PATCH 4/4] privacy: Rename some variables for clarity --- compiler/rustc_middle/src/middle/privacy.rs | 6 +++--- compiler/rustc_privacy/src/lib.rs | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/compiler/rustc_middle/src/middle/privacy.rs b/compiler/rustc_middle/src/middle/privacy.rs index a0b7c8de600f8..5baeb1ee0cf7b 100644 --- a/compiler/rustc_middle/src/middle/privacy.rs +++ b/compiler/rustc_middle/src/middle/privacy.rs @@ -215,7 +215,7 @@ impl EffectiveVisibilities { pub fn update( &mut self, id: Id, - nominal_vis: Option, + max_vis: Option, lazy_private_vis: impl FnOnce() -> Visibility, inherited_effective_vis: EffectiveVisibility, level: Level, @@ -239,8 +239,8 @@ impl EffectiveVisibilities { if !(inherited_effective_vis_at_prev_level == inherited_effective_vis_at_level && level != l) { - calculated_effective_vis = if let Some(nominal_vis) = nominal_vis && !nominal_vis.is_at_least(inherited_effective_vis_at_level, tcx) { - nominal_vis + calculated_effective_vis = if let Some(max_vis) = max_vis && !max_vis.is_at_least(inherited_effective_vis_at_level, tcx) { + max_vis } else { inherited_effective_vis_at_level } diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index cea255713ba86..3b8cbab1a6915 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -473,14 +473,14 @@ impl<'tcx> EmbargoVisitor<'tcx> { &mut self, def_id: LocalDefId, inherited_effective_vis: EffectiveVisibility, - nominal_vis: Option, + max_vis: Option, level: Level, ) { let private_vis = ty::Visibility::Restricted(self.tcx.parent_module_from_def_id(def_id)); - if Some(private_vis) != nominal_vis { + if max_vis != Some(private_vis) { self.changed |= self.effective_visibilities.update( def_id, - nominal_vis, + max_vis, || private_vis, inherited_effective_vis, level, @@ -774,9 +774,9 @@ impl<'tcx> Visitor<'tcx> for EmbargoVisitor<'tcx> { for impl_item_ref in impl_.items { let def_id = impl_item_ref.id.owner_id.def_id; - let nominal_vis = + let max_vis = impl_.of_trait.is_none().then(|| self.tcx.local_visibility(def_id)); - self.update_eff_vis(def_id, item_ev, nominal_vis, Level::Direct); + self.update_eff_vis(def_id, item_ev, max_vis, Level::Direct); if let Some(impl_item_ev) = self.get(def_id) { self.reach(def_id, impl_item_ev).generics().predicates().ty(); @@ -897,9 +897,9 @@ impl<'tcx> DefIdVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'_, 'tcx> // All effective visibilities except `reachable_through_impl_trait` are limited to // nominal visibility. If any type or trait is leaked farther than that, it will // produce type privacy errors on any use, so we don't consider it leaked. - let nominal_vis = (self.level != Level::ReachableThroughImplTrait) + let max_vis = (self.level != Level::ReachableThroughImplTrait) .then(|| self.ev.tcx.local_visibility(def_id)); - self.ev.update_eff_vis(def_id, self.effective_vis, nominal_vis, self.level); + self.ev.update_eff_vis(def_id, self.effective_vis, max_vis, self.level); } ControlFlow::Continue(()) }