From c59a2b2746c7c22bd684cbd5a5da0affc7d11a52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Le=C3=B3n=20Orell=20Valerian=20Liehr?= Date: Tue, 4 Jun 2024 16:47:23 +0200 Subject: [PATCH] Cleanup: HIR ty lowering: Consolidate assoc item access checking --- compiler/rustc_hir_analysis/messages.ftl | 4 + compiler/rustc_hir_analysis/src/errors.rs | 12 +++ .../src/hir_ty_lowering/bounds.rs | 50 ++++------ .../src/hir_ty_lowering/mod.rs | 91 ++++++++++++------- .../assoc-inherent-private.stderr | 4 +- tests/ui/traits/item-privacy.stderr | 7 +- 6 files changed, 98 insertions(+), 70 deletions(-) diff --git a/compiler/rustc_hir_analysis/messages.ftl b/compiler/rustc_hir_analysis/messages.ftl index 67959d9dfede0..cb9a642ca391b 100644 --- a/compiler/rustc_hir_analysis/messages.ftl +++ b/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 -> diff --git a/compiler/rustc_hir_analysis/src/errors.rs b/compiler/rustc_hir_analysis/src/errors.rs index 8a9d4cd4ac701..8c65bed4de25f 100644 --- a/compiler/rustc_hir_analysis/src/errors.rs +++ b/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> { diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs index b6a1799c03f11..999da63e1766b 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs @@ -292,30 +292,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) @@ -406,10 +391,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), @@ -428,15 +412,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 { diff --git a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs index 54b7f7f36ed2e..34bf8c027b742 100644 --- a/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs +++ b/compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs @@ -1166,8 +1166,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| { @@ -1183,7 +1185,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, @@ -1193,7 +1195,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( @@ -1220,7 +1222,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() { @@ -1264,7 +1270,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 @@ -1351,50 +1357,69 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ { } } - fn probe_assoc_ty(&self, name: Ident, block: HirId, span: Span, scope: DefId) -> Option { - 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 { + 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( diff --git a/tests/ui/associated-inherent-types/assoc-inherent-private.stderr b/tests/ui/associated-inherent-types/assoc-inherent-private.stderr index d67b45dae3fbe..e3802a9f5bc47 100644 --- a/tests/ui/associated-inherent-types/assoc-inherent-private.stderr +++ b/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 diff --git a/tests/ui/traits/item-privacy.stderr b/tests/ui/traits/item-privacy.stderr index d08bb4745bf57..4b5339b64ad59 100644 --- a/tests/ui/traits/item-privacy.stderr +++ b/tests/ui/traits/item-privacy.stderr @@ -195,14 +195,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