Skip to content

Commit

Permalink
Rollup merge of #125978 - fmease:cleanup-hir-ty-lowering-consolidate-…
Browse files Browse the repository at this point in the history 
…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.
  • Loading branch information
@workingjubilee
workingjubilee committed Jun 12, 2024
2 parents 969056d + c59a2b2 commit e7b07ea
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 70 deletions.
4 changes: 4 additions & 0 deletions compiler/rustc_hir_analysis/messages.ftl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_hir_analysis/src/errors.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
50 changes: 17 additions & 33 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand All @@ -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 {
Expand Down
91 changes: 58 additions & 33 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand All @@ -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,
Expand All @@ -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(
Expand All @@ -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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/associated-inherent-types/assoc-inherent-private.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
7 changes: 5 additions & 2 deletions tests/ui/traits/item-privacy.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit e7b07ea

Please sign in to comment.