Skip to content

Commit

Permalink
Cleanup: HIR ty lowering: Consolidate assoc item access checking
Browse files Browse the repository at this point in the history
  • Loading branch information
fmease committed Jun 4, 2024
1 parent 9f2d0b3 commit 69a05d7
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
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
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
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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),
Expand All @@ -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(|ty| tcx.type_of(ty.def_id).instantiate(tcx, ty.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
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand All @@ -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,
Expand All @@ -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(
Expand All @@ -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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1351,50 +1357,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] in the scope.
///
/// [^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
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
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down

0 comments on commit 69a05d7

Please sign in to comment.