Rollup merge of #125978 - fmease:cleanup-hir-ty-lowering-consolidate-assoc-item-access-checking, r=davidtwco

Cleanup: HIR ty lowering: Consolidate the places that do assoc item probing & access checking

Use `probe_assoc_item` (for hygienically probing an assoc item and checking if it's accessible wrt. visibility and stability) for assoc item constraints, too, not just for assoc type paths and make the privacy error translatable.

Author: workingjubilee

compiler/rustc_hir_analysis/messages.ftl

@@ -8,6 +8,10 @@ hir_analysis_assoc_item_constraints_not_allowed_here =
     associated item constraints are not allowed here
     .label = associated item constraint not allowed here
 
+hir_analysis_assoc_item_is_private = {$kind} `{$name}` is private
+    .label = private {$kind}
+    .defined_here_label = the {$kind} is defined here
+
 hir_analysis_assoc_item_not_found = associated {$assoc_kind} `{$assoc_name}` not found for `{$ty_param_name}`
 
 hir_analysis_assoc_item_not_found_found_in_other_trait_label = there is {$identically_named ->

compiler/rustc_hir_analysis/src/errors.rs

@@ -55,6 +55,18 @@ pub struct AssocKindMismatchWrapInBracesSugg {
     pub hi: Span,
 }
 
+#[derive(Diagnostic)]
+#[diag(hir_analysis_assoc_item_is_private, code = E0624)]
+pub struct AssocItemIsPrivate {
+    #[primary_span]
+    #[label]
+    pub span: Span,
+    pub kind: &'static str,
+    pub name: Ident,
+    #[label(hir_analysis_defined_here_label)]
+    pub defined_here_label: Span,
+}
+
 #[derive(Diagnostic)]
 #[diag(hir_analysis_assoc_item_not_found, code = E0220)]
 pub struct AssocItemNotFound<'a> {

compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs

@@ -294,30 +294,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             )?
         };
 
-        let (assoc_ident, def_scope) =
-            tcx.adjust_ident_and_get_scope(constraint.ident, candidate.def_id(), hir_ref_id);
-
-        // We have already adjusted the item name above, so compare with `.normalize_to_macros_2_0()`
-        // instead of calling `filter_by_name_and_kind` which would needlessly normalize the
-        // `assoc_ident` again and again.
-        let assoc_item = tcx
-            .associated_items(candidate.def_id())
-            .filter_by_name_unhygienic(assoc_ident.name)
-            .find(|i| i.kind == assoc_kind && i.ident(tcx).normalize_to_macros_2_0() == assoc_ident)
-            .expect("missing associated item");
-
-        if !assoc_item.visibility(tcx).is_accessible_from(def_scope, tcx) {
-            let reported = tcx
-                .dcx()
-                .struct_span_err(
-                    constraint.span,
-                    format!("{} `{}` is private", assoc_item.kind, constraint.ident),
-                )
-                .with_span_label(constraint.span, format!("private {}", assoc_item.kind))
-                .emit();
-            self.set_tainted_by_errors(reported);
-        }
-        tcx.check_stability(assoc_item.def_id, Some(hir_ref_id), constraint.span, None);
+        let assoc_item = self
+            .probe_assoc_item(
+                constraint.ident,
+                assoc_kind,
+                hir_ref_id,
+                constraint.span,
+                candidate.def_id(),
+            )
+            .expect("failed to find associated item");
 
         duplicates
             .entry(assoc_item.def_id)
@@ -404,10 +389,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             // Create the generic arguments for the associated type or constant by joining the
             // parent arguments (the arguments of the trait) and the own arguments (the ones of
             // the associated item itself) and construct an alias type using them.
-            let alias_ty = candidate.map_bound(|trait_ref| {
-                let ident = Ident::new(assoc_item.name, constraint.ident.span);
+            let alias_term = candidate.map_bound(|trait_ref| {
                 let item_segment = hir::PathSegment {
-                    ident,
+                    ident: constraint.ident,
                     hir_id: constraint.hir_id,
                     res: Res::Err,
                     args: Some(constraint.gen_args),
@@ -426,15 +410,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             });
 
             // Provide the resolved type of the associated constant to `type_of(AnonConst)`.
-            if let hir::AssocItemConstraintKind::Equality { term: hir::Term::Const(anon_const) } =
-                constraint.kind
-            {
-                let ty = alias_ty.map_bound(|ty| tcx.type_of(ty.def_id).instantiate(tcx, ty.args));
-                let ty = check_assoc_const_binding_type(tcx, assoc_ident, ty, constraint.hir_id);
+            if let Some(anon_const) = constraint.ct() {
+                let ty = alias_term
+                    .map_bound(|alias| tcx.type_of(alias.def_id).instantiate(tcx, alias.args));
+                let ty =
+                    check_assoc_const_binding_type(tcx, constraint.ident, ty, constraint.hir_id);
                 tcx.feed_anon_const_type(anon_const.def_id, ty::EarlyBinder::bind(ty));
             }
 
-            alias_ty
+            alias_term
         };
 
         match constraint.kind {

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

@@ -1151,8 +1151,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         };
 
         let trait_did = bound.def_id();
-        let assoc_ty_did = self.probe_assoc_ty(assoc_ident, hir_ref_id, span, trait_did).unwrap();
-        let ty = self.lower_assoc_ty(span, assoc_ty_did, assoc_segment, bound);
+        let assoc_ty = self
+            .probe_assoc_item(assoc_ident, ty::AssocKind::Type, hir_ref_id, span, trait_did)
+            .expect("failed to find associated type");
+        let ty = self.lower_assoc_ty(span, assoc_ty.def_id, assoc_segment, bound);
 
         if let Some(variant_def_id) = variant_resolution {
             tcx.node_span_lint(AMBIGUOUS_ASSOCIATED_ITEMS, hir_ref_id, span, |lint| {
@@ -1168,7 +1170,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                 };
 
                 could_refer_to(DefKind::Variant, variant_def_id, "");
-                could_refer_to(DefKind::AssocTy, assoc_ty_did, " also");
+                could_refer_to(DefKind::AssocTy, assoc_ty.def_id, " also");
 
                 lint.span_suggestion(
                     span,
@@ -1178,7 +1180,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
                 );
             });
         }
-        Ok((ty, DefKind::AssocTy, assoc_ty_did))
+        Ok((ty, DefKind::AssocTy, assoc_ty.def_id))
     }
 
     fn probe_inherent_assoc_ty(
@@ -1205,7 +1207,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         let candidates: Vec<_> = tcx
             .inherent_impls(adt_did)?
             .iter()
-            .filter_map(|&impl_| Some((impl_, self.probe_assoc_ty_unchecked(name, block, impl_)?)))
+            .filter_map(|&impl_| {
+                let (item, scope) =
+                    self.probe_assoc_item_unchecked(name, ty::AssocKind::Type, block, impl_)?;
+                Some((impl_, (item.def_id, scope)))
+            })
             .collect();
 
         if candidates.is_empty() {
@@ -1249,7 +1255,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
             },
         )?;
 
-        self.check_assoc_ty(assoc_item, name, def_scope, block, span);
+        self.check_assoc_item(assoc_item, name, def_scope, block, span);
 
         // FIXME(fmease): Currently creating throwaway `parent_args` to please
         // `lower_generic_args_of_assoc_item`. Modify the latter instead (or sth. similar) to
@@ -1336,50 +1342,69 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
         }
     }
 
-    fn probe_assoc_ty(&self, name: Ident, block: HirId, span: Span, scope: DefId) -> Option<DefId> {
-        let (item, def_scope) = self.probe_assoc_ty_unchecked(name, block, scope)?;
-        self.check_assoc_ty(item, name, def_scope, block, span);
+    /// Given name and kind search for the assoc item in the provided scope and check if it's accessible[^1].
+    ///
+    /// [^1]: I.e., accessible in the provided scope wrt. visibility and stability.
+    fn probe_assoc_item(
+        &self,
+        ident: Ident,
+        kind: ty::AssocKind,
+        block: HirId,
+        span: Span,
+        scope: DefId,
+    ) -> Option<ty::AssocItem> {
+        let (item, scope) = self.probe_assoc_item_unchecked(ident, kind, block, scope)?;
+        self.check_assoc_item(item.def_id, ident, scope, block, span);
         Some(item)
     }
 
-    fn probe_assoc_ty_unchecked(
+    /// Given name and kind search for the assoc item in the provided scope
+    /// *without* checking if it's accessible[^1].
+    ///
+    /// [^1]: I.e., accessible in the provided scope wrt. visibility and stability.
+    fn probe_assoc_item_unchecked(
         &self,
-        name: Ident,
+        ident: Ident,
+        kind: ty::AssocKind,
         block: HirId,
         scope: DefId,
-    ) -> Option<(DefId, DefId)> {
+    ) -> Option<(ty::AssocItem, /*scope*/ DefId)> {
         let tcx = self.tcx();
-        let (ident, def_scope) = tcx.adjust_ident_and_get_scope(name, scope, block);
 
+        let (ident, def_scope) = tcx.adjust_ident_and_get_scope(ident, scope, block);
         // We have already adjusted the item name above, so compare with `.normalize_to_macros_2_0()`
         // instead of calling `filter_by_name_and_kind` which would needlessly normalize the
         // `ident` again and again.
-        let item = tcx.associated_items(scope).in_definition_order().find(|i| {
-            i.kind.namespace() == Namespace::TypeNS
-                && i.ident(tcx).normalize_to_macros_2_0() == ident
-        })?;
+        let item = tcx
+            .associated_items(scope)
+            .filter_by_name_unhygienic(ident.name)
+            .find(|i| i.kind == kind && i.ident(tcx).normalize_to_macros_2_0() == ident)?;
 
-        Some((item.def_id, def_scope))
+        Some((*item, def_scope))
     }
 
-    fn check_assoc_ty(&self, item: DefId, name: Ident, def_scope: DefId, block: HirId, span: Span) {
+    /// Check if the given assoc item is accessible in the provided scope wrt. visibility and stability.
+    fn check_assoc_item(
+        &self,
+        item_def_id: DefId,
+        ident: Ident,
+        scope: DefId,
+        block: HirId,
+        span: Span,
+    ) {
         let tcx = self.tcx();
-        let kind = DefKind::AssocTy;
-
-        if !tcx.visibility(item).is_accessible_from(def_scope, tcx) {
-            let kind = tcx.def_kind_descr(kind, item);
-            let msg = format!("{kind} `{name}` is private");
-            let def_span = tcx.def_span(item);
-            let reported = tcx
-                .dcx()
-                .struct_span_err(span, msg)
-                .with_code(E0624)
-                .with_span_label(span, format!("private {kind}"))
-                .with_span_label(def_span, format!("{kind} defined here"))
-                .emit();
+
+        if !tcx.visibility(item_def_id).is_accessible_from(scope, tcx) {
+            let reported = tcx.dcx().emit_err(crate::errors::AssocItemIsPrivate {
+                span,
+                kind: tcx.def_descr(item_def_id),
+                name: ident,
+                defined_here_label: tcx.def_span(item_def_id),
+            });
             self.set_tainted_by_errors(reported);
         }
-        tcx.check_stability(item, Some(block), span, None);
+
+        tcx.check_stability(item_def_id, Some(block), span, None);
     }
 
     fn probe_traits_that_match_assoc_ty(

tests/ui/associated-inherent-types/assoc-inherent-private.stderr

@@ -2,7 +2,7 @@ error[E0624]: associated type `P` is private
   --> $DIR/assoc-inherent-private.rs:10:10
    |
 LL |         type P = ();
-   |         ------ associated type defined here
+   |         ------ the associated type is defined here
 ...
 LL | type U = m::T::P;
    |          ^^^^^^^ private associated type
@@ -11,7 +11,7 @@ error[E0624]: associated type `P` is private
   --> $DIR/assoc-inherent-private.rs:21:10
    |
 LL |             pub(super) type P = bool;
-   |             ----------------- associated type defined here
+   |             ----------------- the associated type is defined here
 ...
 LL | type V = n::n::T::P;
    |          ^^^^^^^^^^ private associated type

tests/ui/traits/item-privacy.stderr

@@ -196,14 +196,17 @@ error[E0624]: associated type `A` is private
   --> $DIR/item-privacy.rs:119:12
    |
 LL |         type A = u8;
-   |         ------ associated type defined here
+   |         ------ the associated type is defined here
 ...
 LL |     let _: T::A;
    |            ^^^^ private associated type
 
-error: associated type `A` is private
+error[E0624]: associated type `A` is private
   --> $DIR/item-privacy.rs:128:9
    |
+LL |         type A = u8;
+   |         ------ the associated type is defined here
+...
 LL |         A = u8,
    |         ^^^^^^ private associated type