From d326aed46f6fdc741fe78f29c948a4c358150094 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Thu, 15 Jun 2023 15:47:38 +0300 Subject: [PATCH 01/10] 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 02/10] 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 03/10] 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 04/10] 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(()) } From 042f6052abfd6a8557192a8962d47b25fa61b021 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Fri, 23 Jun 2023 16:04:35 +0000 Subject: [PATCH 05/10] Add some tests around where bounds on associated items and their lack of effect on impls --- tests/ui/traits/auxiliary/trivial3.rs | 1 + tests/ui/traits/auxiliary/trivial4.rs | 3 +++ tests/ui/traits/trivial_impl.rs | 18 ++++++++++++++++ tests/ui/traits/trivial_impl.stderr | 14 ++++++++++++ tests/ui/traits/trivial_impl2.rs | 13 ++++++++++++ tests/ui/traits/trivial_impl2.stderr | 14 ++++++++++++ tests/ui/traits/trivial_impl3.rs | 19 +++++++++++++++++ tests/ui/traits/trivial_impl3.stderr | 14 ++++++++++++ tests/ui/traits/trivial_impl4.rs | 21 ++++++++++++++++++ tests/ui/traits/trivial_impl4.stderr | 14 ++++++++++++ tests/ui/traits/trivial_impl_sized.rs | 26 +++++++++++++++++++++++ tests/ui/traits/trivial_impl_sized.stderr | 25 ++++++++++++++++++++++ 12 files changed, 182 insertions(+) create mode 100644 tests/ui/traits/auxiliary/trivial3.rs create mode 100644 tests/ui/traits/auxiliary/trivial4.rs create mode 100644 tests/ui/traits/trivial_impl.rs create mode 100644 tests/ui/traits/trivial_impl.stderr create mode 100644 tests/ui/traits/trivial_impl2.rs create mode 100644 tests/ui/traits/trivial_impl2.stderr create mode 100644 tests/ui/traits/trivial_impl3.rs create mode 100644 tests/ui/traits/trivial_impl3.stderr create mode 100644 tests/ui/traits/trivial_impl4.rs create mode 100644 tests/ui/traits/trivial_impl4.stderr create mode 100644 tests/ui/traits/trivial_impl_sized.rs create mode 100644 tests/ui/traits/trivial_impl_sized.stderr diff --git a/tests/ui/traits/auxiliary/trivial3.rs b/tests/ui/traits/auxiliary/trivial3.rs new file mode 100644 index 0000000000000..0a47fdc74d721 --- /dev/null +++ b/tests/ui/traits/auxiliary/trivial3.rs @@ -0,0 +1 @@ +pub trait Trait {} diff --git a/tests/ui/traits/auxiliary/trivial4.rs b/tests/ui/traits/auxiliary/trivial4.rs new file mode 100644 index 0000000000000..a527d1a9526d9 --- /dev/null +++ b/tests/ui/traits/auxiliary/trivial4.rs @@ -0,0 +1,3 @@ +pub trait Trait {} + +impl Trait for () {} diff --git a/tests/ui/traits/trivial_impl.rs b/tests/ui/traits/trivial_impl.rs new file mode 100644 index 0000000000000..6ac8c744bc455 --- /dev/null +++ b/tests/ui/traits/trivial_impl.rs @@ -0,0 +1,18 @@ +//! This test checks that we do need to implement +//! all members, even if their where bounds only hold +//! due to other impls. + +trait Foo { + fn foo() + where + Self: Foo<()>; +} + +impl Foo<()> for () { + fn foo() {} +} + +impl Foo for () {} +//~^ ERROR: not all trait items implemented, missing: `foo` + +fn main() {} diff --git a/tests/ui/traits/trivial_impl.stderr b/tests/ui/traits/trivial_impl.stderr new file mode 100644 index 0000000000000..4b29b55bea13d --- /dev/null +++ b/tests/ui/traits/trivial_impl.stderr @@ -0,0 +1,14 @@ +error[E0046]: not all trait items implemented, missing: `foo` + --> $DIR/trivial_impl.rs:15:1 + | +LL | / fn foo() +LL | | where +LL | | Self: Foo<()>; + | |______________________- `foo` from trait +... +LL | impl Foo for () {} + | ^^^^^^^^^^^^^^^^^^^^ missing `foo` in implementation + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0046`. diff --git a/tests/ui/traits/trivial_impl2.rs b/tests/ui/traits/trivial_impl2.rs new file mode 100644 index 0000000000000..be58096007f2b --- /dev/null +++ b/tests/ui/traits/trivial_impl2.rs @@ -0,0 +1,13 @@ +//! This test checks that we currently need to implement +//! members, even if their where bounds don't hold for the impl type. + +trait Foo { + fn foo() + where + Self: Foo<()>; +} + +impl Foo for () {} +//~^ ERROR: not all trait items implemented, missing: `foo` + +fn main() {} diff --git a/tests/ui/traits/trivial_impl2.stderr b/tests/ui/traits/trivial_impl2.stderr new file mode 100644 index 0000000000000..04c05df0616d1 --- /dev/null +++ b/tests/ui/traits/trivial_impl2.stderr @@ -0,0 +1,14 @@ +error[E0046]: not all trait items implemented, missing: `foo` + --> $DIR/trivial_impl2.rs:10:1 + | +LL | / fn foo() +LL | | where +LL | | Self: Foo<()>; + | |______________________- `foo` from trait +... +LL | impl Foo for () {} + | ^^^^^^^^^^^^^^^^^^^^ missing `foo` in implementation + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0046`. diff --git a/tests/ui/traits/trivial_impl3.rs b/tests/ui/traits/trivial_impl3.rs new file mode 100644 index 0000000000000..714f643bc994c --- /dev/null +++ b/tests/ui/traits/trivial_impl3.rs @@ -0,0 +1,19 @@ +//! Check that we don't break orphan rules. +//! The dependency may add an impl for `u8` later, +//! which would break this crate. We want to avoid adding +//! more ways in which adding an impl can be a breaking change. + +// aux-build:trivial3.rs + +extern crate trivial3; + +pub trait Foo { + fn foo() + where + Self: trivial3::Trait; +} + +impl Foo for u8 {} +//~^ ERROR not all trait items implemented, missing: `foo` + +fn main() {} diff --git a/tests/ui/traits/trivial_impl3.stderr b/tests/ui/traits/trivial_impl3.stderr new file mode 100644 index 0000000000000..dfb39d6ce159c --- /dev/null +++ b/tests/ui/traits/trivial_impl3.stderr @@ -0,0 +1,14 @@ +error[E0046]: not all trait items implemented, missing: `foo` + --> $DIR/trivial_impl3.rs:16:1 + | +LL | / fn foo() +LL | | where +LL | | Self: trivial3::Trait; + | |______________________________- `foo` from trait +... +LL | impl Foo for u8 {} + | ^^^^^^^^^^^^^^^ missing `foo` in implementation + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0046`. diff --git a/tests/ui/traits/trivial_impl4.rs b/tests/ui/traits/trivial_impl4.rs new file mode 100644 index 0000000000000..518f159c1fb65 --- /dev/null +++ b/tests/ui/traits/trivial_impl4.rs @@ -0,0 +1,21 @@ +//! Check that we don't break orphan rules. +//! The dependency may add an impl for `u8` later, +//! which would break this crate. We want to avoid adding +//! more ways in which adding an impl can be a breaking change. +//! This test differs from `trivial_impl3` because there actually +//! exists any impl for `Trait`, which has an effect on coherence. + +// aux-build:trivial4.rs + +extern crate trivial4; + +pub trait Foo { + fn foo() + where + Self: trivial4::Trait; +} + +impl Foo for u8 {} +//~^ ERROR not all trait items implemented, missing: `foo` + +fn main() {} diff --git a/tests/ui/traits/trivial_impl4.stderr b/tests/ui/traits/trivial_impl4.stderr new file mode 100644 index 0000000000000..04b29ed77340d --- /dev/null +++ b/tests/ui/traits/trivial_impl4.stderr @@ -0,0 +1,14 @@ +error[E0046]: not all trait items implemented, missing: `foo` + --> $DIR/trivial_impl4.rs:18:1 + | +LL | / fn foo() +LL | | where +LL | | Self: trivial4::Trait; + | |______________________________- `foo` from trait +... +LL | impl Foo for u8 {} + | ^^^^^^^^^^^^^^^ missing `foo` in implementation + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0046`. diff --git a/tests/ui/traits/trivial_impl_sized.rs b/tests/ui/traits/trivial_impl_sized.rs new file mode 100644 index 0000000000000..501a3405090e6 --- /dev/null +++ b/tests/ui/traits/trivial_impl_sized.rs @@ -0,0 +1,26 @@ +//! This test checks that we currently need to implement +//! members, even if their where bounds don't hold for the impl type. + +trait Foo { + fn foo() + where + Self: Sized; +} + +impl Foo for () { + fn foo() {} +} + +// Must not be allowed +impl Foo for i32 {} +//~^ ERROR: not all trait items implemented, missing: `foo` + +// Should be allowed +impl Foo for dyn std::fmt::Debug {} +//~^ ERROR: not all trait items implemented, missing: `foo` + +impl Foo for dyn std::fmt::Display { + fn foo() {} +} + +fn main() {} diff --git a/tests/ui/traits/trivial_impl_sized.stderr b/tests/ui/traits/trivial_impl_sized.stderr new file mode 100644 index 0000000000000..ebf6dfc9dd2df --- /dev/null +++ b/tests/ui/traits/trivial_impl_sized.stderr @@ -0,0 +1,25 @@ +error[E0046]: not all trait items implemented, missing: `foo` + --> $DIR/trivial_impl_sized.rs:15:1 + | +LL | / fn foo() +LL | | where +LL | | Self: Sized; + | |____________________- `foo` from trait +... +LL | impl Foo for i32 {} + | ^^^^^^^^^^^^^^^^ missing `foo` in implementation + +error[E0046]: not all trait items implemented, missing: `foo` + --> $DIR/trivial_impl_sized.rs:19:1 + | +LL | / fn foo() +LL | | where +LL | | Self: Sized; + | |____________________- `foo` from trait +... +LL | impl Foo for dyn std::fmt::Debug {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ missing `foo` in implementation + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0046`. From 48544c1b12b50ab5fd6a772060c17bdb8b5a32d2 Mon Sep 17 00:00:00 2001 From: Rageking8 Date: Mon, 26 Jun 2023 20:40:06 +0800 Subject: [PATCH 06/10] Make `rustc_on_unimplemented` std-agnostic --- library/core/src/convert/mod.rs | 2 +- library/core/src/iter/traits/iterator.rs | 8 ++--- library/core/src/marker.rs | 28 +++++++-------- library/core/src/ops/index.rs | 2 +- library/core/src/ops/try_trait.rs | 45 +++++++++++++++++++----- library/core/src/slice/index.rs | 5 ++- 6 files changed, 60 insertions(+), 30 deletions(-) diff --git a/library/core/src/convert/mod.rs b/library/core/src/convert/mod.rs index 799085e9a8307..ff5a4c913b7ed 100644 --- a/library/core/src/convert/mod.rs +++ b/library/core/src/convert/mod.rs @@ -532,7 +532,7 @@ pub trait Into: Sized { #[rustc_diagnostic_item = "From"] #[stable(feature = "rust1", since = "1.0.0")] #[rustc_on_unimplemented(on( - all(_Self = "&str", T = "std::string::String"), + all(_Self = "&str", any(T = "alloc::string::String", T = "std::string::String")), note = "to coerce a `{T}` into a `{Self}`, use `&*` as a prefix", ))] pub trait From: Sized { diff --git a/library/core/src/iter/traits/iterator.rs b/library/core/src/iter/traits/iterator.rs index dabfce1447483..988352283084b 100644 --- a/library/core/src/iter/traits/iterator.rs +++ b/library/core/src/iter/traits/iterator.rs @@ -26,13 +26,13 @@ fn _assert_is_object_safe(_: &dyn Iterator) {} #[stable(feature = "rust1", since = "1.0.0")] #[rustc_on_unimplemented( on( - _Self = "std::ops::RangeTo", + any(_Self = "core::ops::RangeTo", _Self = "std::ops::RangeTo"), label = "if you meant to iterate until a value, add a starting value", note = "`..end` is a `RangeTo`, which cannot be iterated on; you might have meant to have a \ bounded `Range`: `0..end`" ), on( - _Self = "std::ops::RangeToInclusive", + any(_Self = "core::ops::RangeToInclusive", _Self = "std::ops::RangeToInclusive"), label = "if you meant to iterate until a value (including it), add a starting value", note = "`..=end` is a `RangeToInclusive`, which cannot be iterated on; you might have meant \ to have a bounded `RangeInclusive`: `0..=end`" @@ -43,7 +43,7 @@ fn _assert_is_object_safe(_: &dyn Iterator) {} ), on(_Self = "&[]", label = "`{Self}` is not an iterator; try calling `.iter()`"), on( - _Self = "std::vec::Vec", + any(_Self = "alloc::vec::Vec", _Self = "std::vec::Vec"), label = "`{Self}` is not an iterator; try calling `.into_iter()` or `.iter()`" ), on( @@ -51,7 +51,7 @@ fn _assert_is_object_safe(_: &dyn Iterator) {} label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`" ), on( - _Self = "std::string::String", + any(_Self = "alloc::string::String", _Self = "std::string::String"), label = "`{Self}` is not an iterator; try calling `.chars()` or `.bytes()`" ), on( diff --git a/library/core/src/marker.rs b/library/core/src/marker.rs index 760e58276fc9f..e251015dd8600 100644 --- a/library/core/src/marker.rs +++ b/library/core/src/marker.rs @@ -575,59 +575,59 @@ impl Copy for &T {} #[lang = "sync"] #[rustc_on_unimplemented( on( - _Self = "std::cell::OnceCell", + any(_Self = "core::cell:OnceCell", _Self = "std::cell::OnceCell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::OnceLock` instead" ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU8` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU16` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU32` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicU64` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicUsize` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI8` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI16` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI32` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicI64` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicIsize` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` or `std::sync::atomic::AtomicBool` instead", ), on( - _Self = "std::cell::Cell", + any(_Self = "core::cell::Cell", _Self = "std::cell::Cell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock`", ), on( - _Self = "std::cell::RefCell", + any(_Self = "core::cell::RefCell", _Self = "std::cell::RefCell"), note = "if you want to do aliasing and mutation between multiple threads, use `std::sync::RwLock` instead", ), message = "`{Self}` cannot be shared between threads safely", diff --git a/library/core/src/ops/index.rs b/library/core/src/ops/index.rs index 1f1784ec9b255..f4649be54d56f 100644 --- a/library/core/src/ops/index.rs +++ b/library/core/src/ops/index.rs @@ -153,7 +153,7 @@ see chapter in The Book " ), on( - _Self = "std::string::String", + any(_Self = "alloc::string::String", _Self = "std::string::String"), note = "you can use `.chars().nth()` or `.bytes().nth()` see chapter in The Book " ), diff --git a/library/core/src/ops/try_trait.rs b/library/core/src/ops/try_trait.rs index b4f69d0b21309..17625daccbcf9 100644 --- a/library/core/src/ops/try_trait.rs +++ b/library/core/src/ops/try_trait.rs @@ -226,8 +226,14 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "std::result::Result", - R = "std::option::Option" + any( + _Self = "core::result::Result", + _Self = "std::result::Result", + ), + any( + R = "core::option::Option", + R = "std::option::Option", + ) ), message = "the `?` operator can only be used on `Result`s, not `Option`s, \ in {ItemContext} that returns `Result`", @@ -237,7 +243,10 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "std::result::Result", + any( + _Self = "core::result::Result", + _Self = "std::result::Result", + ) ), // There's a special error message in the trait selection code for // `From` in `?`, so this is not shown for result-in-result errors, @@ -250,8 +259,14 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "std::option::Option", - R = "std::result::Result", + any( + _Self = "core::option::Option", + _Self = "std::option::Option", + ), + any( + R = "core::result::Result", + R = "std::result::Result", + ) ), message = "the `?` operator can only be used on `Option`s, not `Result`s, \ in {ItemContext} that returns `Option`", @@ -261,7 +276,10 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "std::option::Option", + any( + _Self = "core::option::Option", + _Self = "std::option::Option", + ) ), // `Option`-in-`Option` always works, as there's only one possible // residual, so this can also be phrased strongly. @@ -273,8 +291,14 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "std::ops::ControlFlow", - R = "std::ops::ControlFlow", + any( + _Self = "core::ops::ControlFlow", + _Self = "std::ops::ControlFlow", + ), + any( + R = "core::ops::ControlFlow", + R = "std::ops::ControlFlow", + ) ), message = "the `?` operator in {ItemContext} that returns `ControlFlow` \ can only be used on other `ControlFlow`s (with the same Break type)", @@ -285,7 +309,10 @@ pub trait Try: FromResidual { on( all( from_desugaring = "QuestionMark", - _Self = "std::ops::ControlFlow", + any( + _Self = "core::ops::ControlFlow", + _Self = "std::ops::ControlFlow", + ) // `R` is not a `ControlFlow`, as that case was matched previously ), message = "the `?` operator can only be used on `ControlFlow`s \ diff --git a/library/core/src/slice/index.rs b/library/core/src/slice/index.rs index 6ef9f9c95e843..e1e3bcc05e74b 100644 --- a/library/core/src/slice/index.rs +++ b/library/core/src/slice/index.rs @@ -152,7 +152,10 @@ mod private_slice_index { #[rustc_on_unimplemented( on(T = "str", label = "string indices are ranges of `usize`",), on( - all(any(T = "str", T = "&str", T = "std::string::String"), _Self = "{integer}"), + all( + any(T = "str", T = "&str", T = "alloc::string::String", T = "std::string::String"), + _Self = "{integer}" + ), note = "you can use `.chars().nth()` or `.bytes().nth()`\n\ for more information, see chapter 8 in The Book: \ " From 5abeb801b888a0475a87c03b3428d9cd503a0e74 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 28 Jun 2023 14:56:21 -0700 Subject: [PATCH 07/10] syle-guide: Clarify let-else further Give some additional examples with multi-line patterns. Make it clearer to go on to the next case if the conditions aren't met. --- src/doc/style-guide/src/statements.md | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) diff --git a/src/doc/style-guide/src/statements.md b/src/doc/style-guide/src/statements.md index 9bc521e1c7ba8..a5cd6da10616d 100644 --- a/src/doc/style-guide/src/statements.md +++ b/src/doc/style-guide/src/statements.md @@ -159,8 +159,8 @@ before the `else`. If the initializer expression is multi-line, the `else` keyword and opening brace of the block (i.e. `else {`) should be put on the same line as the end of -the initializer expression, with a space between them, if all the following are -true: +the initializer expression, with a space between them, if and only if all the +following are true: * The initializer expression ends with one or more closing parentheses, square brackets, and/or braces @@ -209,6 +209,28 @@ fn main() { else { return; }; + + let LongStructName(AnotherStruct { + multi, + line, + pattern, + }) = slice.as_ref() + else { + return; + }; + + let LongStructName(AnotherStruct { + multi, + line, + pattern, + }) = multi_line_function_call( + arg1, + arg2, + arg3, + arg4, + ) else { + return; + }; } ``` From 5e83ddd27946a74f9ab3390977b99c32a365b7a0 Mon Sep 17 00:00:00 2001 From: Lukas Markeffsky <@> Date: Wed, 28 Jun 2023 23:16:39 +0200 Subject: [PATCH 08/10] don't suggest `move` for borrows that aren't closures --- .../src/diagnostics/conflict_errors.rs | 27 ++++++++++--------- tests/ui/closures/issue-113087.rs | 11 ++++++++ tests/ui/closures/issue-113087.stderr | 16 +++++++++++ 3 files changed, 41 insertions(+), 13 deletions(-) create mode 100644 tests/ui/closures/issue-113087.rs create mode 100644 tests/ui/closures/issue-113087.stderr diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 39b05829888c3..a068f2d69268b 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -1730,18 +1730,19 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { ( Some(name), BorrowExplanation::UsedLater(LaterUseKind::ClosureCapture, var_or_use_span, _), - ) => self.report_escaping_closure_capture( - borrow_spans, - borrow_span, - &RegionName { - name: self.synthesize_region_name(), - source: RegionNameSource::Static, - }, - ConstraintCategory::CallArgument(None), - var_or_use_span, - &format!("`{}`", name), - "block", - ), + ) if borrow_spans.for_generator() || borrow_spans.for_closure() => self + .report_escaping_closure_capture( + borrow_spans, + borrow_span, + &RegionName { + name: self.synthesize_region_name(), + source: RegionNameSource::Static, + }, + ConstraintCategory::CallArgument(None), + var_or_use_span, + &format!("`{}`", name), + "block", + ), ( Some(name), BorrowExplanation::MustBeValidFor { @@ -1754,7 +1755,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { span, .. }, - ) if borrow_spans.for_generator() | borrow_spans.for_closure() => self + ) if borrow_spans.for_generator() || borrow_spans.for_closure() => self .report_escaping_closure_capture( borrow_spans, borrow_span, diff --git a/tests/ui/closures/issue-113087.rs b/tests/ui/closures/issue-113087.rs new file mode 100644 index 0000000000000..a4edc2f2f0b5d --- /dev/null +++ b/tests/ui/closures/issue-113087.rs @@ -0,0 +1,11 @@ +fn some_fn<'a>(_: &'a i32, _: impl FnOnce(&'a i32)) {} + +fn main() { + let some_closure = |_| {}; + + for a in [1] { + some_fn(&a, |c| { //~ ERROR does not live long enough + some_closure(c); + }); + } +} diff --git a/tests/ui/closures/issue-113087.stderr b/tests/ui/closures/issue-113087.stderr new file mode 100644 index 0000000000000..8ccef4a54f5d6 --- /dev/null +++ b/tests/ui/closures/issue-113087.stderr @@ -0,0 +1,16 @@ +error[E0597]: `a` does not live long enough + --> $DIR/issue-113087.rs:7:17 + | +LL | for a in [1] { + | - binding `a` declared here +LL | some_fn(&a, |c| { + | ^^ borrowed value does not live long enough +LL | some_closure(c); + | ------------ borrow later captured here by closure +LL | }); +LL | } + | - `a` dropped here while still borrowed + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0597`. From 4cc80651b84665c92851f95a07e9443675c281f5 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 28 Jun 2023 15:35:05 -0700 Subject: [PATCH 09/10] style-guide: Add an example of formatting a multi-line attribute We already say to format attributes like functions, but we didn't have an example of formatting a multi-line attribute. --- src/doc/style-guide/src/README.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/doc/style-guide/src/README.md b/src/doc/style-guide/src/README.md index 75013bb3df980..b8aa64ba14f6f 100644 --- a/src/doc/style-guide/src/README.md +++ b/src/doc/style-guide/src/README.md @@ -186,6 +186,11 @@ For attributes with argument lists, format like functions. ```rust #[repr(C)] #[foo(foo, bar)] +#[long_multi_line_attribute( + split, + across, + lines, +)] struct CRepr { #![repr(C)] x: f32, From 025dd3aef08d3674be596caf2ce2f260d00871c0 Mon Sep 17 00:00:00 2001 From: Josh Triplett Date: Wed, 28 Jun 2023 17:10:03 -0700 Subject: [PATCH 10/10] style-guide: Narrow guidance about references and dereferencing The style guide advises "prefer dereferencing to taking references", but doesn't give guidance on when that "preference" should get overridden by other considerations. Give an example of when it's fine to ignore that advice. --- src/doc/style-guide/src/expressions.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/doc/style-guide/src/expressions.md b/src/doc/style-guide/src/expressions.md index 8271b42da4c70..f5e37b6c46f69 100644 --- a/src/doc/style-guide/src/expressions.md +++ b/src/doc/style-guide/src/expressions.md @@ -288,7 +288,8 @@ and other assignment operators such as `+=` or `*=`). For comparison operators, because for `T op U`, `&T op &U` is also implemented: if you have `t: &T`, and `u: U`, prefer `*t op u` to `t op &u`. In general, -within expressions, prefer dereferencing to taking references. +within expressions, prefer dereferencing to taking references, unless necessary +(e.g. to avoid an unnecessarily expensive operation). Use parentheses liberally, do not necessarily elide them due to precedence. Tools should not automatically insert or remove parentheses. Do not use spaces