Auto merge of #113146 - matthiaskrgr:rollup-bxtr51e, r=matthiaskrgr

Rollup of 7 pull requests

Successful merges:

 - #112670 (privacy: Type privacy lints fixes and cleanups)
 - #112929 (Test that we require implementing trait items whose bounds don't hold in the current impl)
 - #113054 (Make `rustc_on_unimplemented` std-agnostic)
 - #113137 (don't suggest `move` for borrows that aren't closures)
 - #113139 (style-guide: Clarify let-else further)
 - #113140 (style-guide: Add an example of formatting a multi-line attribute)
 - #113143 (style-guide: Narrow guidance about references and dereferencing)

r? `@ghost`
`@rustbot` modify labels: rollup

Author: bors

Diff for: 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 for: compiler/rustc_feature/src/active.rs

@@ -543,6 +543,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 for: 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 for: compiler/rustc_lint_defs/src/builtin.rs

@@ -4264,6 +4264,7 @@ declare_lint! {
     /// ### Example
     ///
     /// ```rust,compile_fail
+    /// # #![feature(type_privacy_lints)]
     /// # #![allow(unused)]
     /// # #![allow(private_in_public)]
     /// #![deny(private_interfaces)]
@@ -4288,6 +4289,7 @@ declare_lint! {
     pub PRIVATE_INTERFACES,
     Allow,
     "private type in primary interface of an item",
+    @feature_gate = sym::type_privacy_lints;
 }
 
 declare_lint! {
@@ -4298,6 +4300,7 @@ declare_lint! {
     /// ### Example
     ///
     /// ```rust,compile_fail
+    /// # #![feature(type_privacy_lints)]
     /// # #![allow(private_in_public)]
     /// # #![allow(unused)]
     /// #![deny(private_bounds)]
@@ -4317,7 +4320,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! {
@@ -4327,6 +4331,7 @@ declare_lint! {
     /// ### Example
     ///
     /// ```rust,compile_fail
+    /// # #![feature(type_privacy_lints)]
     /// # #![allow(unused)]
     /// #![deny(unnameable_types)]
     /// mod m {
@@ -4345,5 +4350,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 for: 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
+                    );
+                }
             }
         }
     }
@@ -212,7 +215,7 @@ impl<Id: Eq + Hash> EffectiveVisibilities<Id> {
     pub fn update(
         &mut self,
         id: Id,
-        nominal_vis: Option<Visibility>,
+        max_vis: Option<Visibility>,
         lazy_private_vis: impl FnOnce() -> Visibility,
         inherited_effective_vis: EffectiveVisibility,
         level: Level,
@@ -236,8 +239,8 @@ impl<Id: Eq + Hash> EffectiveVisibilities<Id> {
                 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
                     }