Skip to content

Commit c59a2b2

Browse files
committed
Cleanup: HIR ty lowering: Consolidate assoc item access checking
1 parent 9f2d0b3 commit c59a2b2

File tree

6 files changed

+98
-70
lines changed

6 files changed

+98
-70
lines changed

compiler/rustc_hir_analysis/messages.ftl

+4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ hir_analysis_assoc_item_constraints_not_allowed_here =
88
associated item constraints are not allowed here
99
.label = associated item constraint not allowed here
1010
11+
hir_analysis_assoc_item_is_private = {$kind} `{$name}` is private
12+
.label = private {$kind}
13+
.defined_here_label = the {$kind} is defined here
14+
1115
hir_analysis_assoc_item_not_found = associated {$assoc_kind} `{$assoc_name}` not found for `{$ty_param_name}`
1216
1317
hir_analysis_assoc_item_not_found_found_in_other_trait_label = there is {$identically_named ->

compiler/rustc_hir_analysis/src/errors.rs

+12
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,18 @@ pub struct AssocKindMismatchWrapInBracesSugg {
5555
pub hi: Span,
5656
}
5757

58+
#[derive(Diagnostic)]
59+
#[diag(hir_analysis_assoc_item_is_private, code = E0624)]
60+
pub struct AssocItemIsPrivate {
61+
#[primary_span]
62+
#[label]
63+
pub span: Span,
64+
pub kind: &'static str,
65+
pub name: Ident,
66+
#[label(hir_analysis_defined_here_label)]
67+
pub defined_here_label: Span,
68+
}
69+
5870
#[derive(Diagnostic)]
5971
#[diag(hir_analysis_assoc_item_not_found, code = E0220)]
6072
pub struct AssocItemNotFound<'a> {

compiler/rustc_hir_analysis/src/hir_ty_lowering/bounds.rs

+17-33
Original file line numberDiff line numberDiff line change
@@ -292,30 +292,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
292292
)?
293293
};
294294

295-
let (assoc_ident, def_scope) =
296-
tcx.adjust_ident_and_get_scope(constraint.ident, candidate.def_id(), hir_ref_id);
297-
298-
// We have already adjusted the item name above, so compare with `.normalize_to_macros_2_0()`
299-
// instead of calling `filter_by_name_and_kind` which would needlessly normalize the
300-
// `assoc_ident` again and again.
301-
let assoc_item = tcx
302-
.associated_items(candidate.def_id())
303-
.filter_by_name_unhygienic(assoc_ident.name)
304-
.find(|i| i.kind == assoc_kind && i.ident(tcx).normalize_to_macros_2_0() == assoc_ident)
305-
.expect("missing associated item");
306-
307-
if !assoc_item.visibility(tcx).is_accessible_from(def_scope, tcx) {
308-
let reported = tcx
309-
.dcx()
310-
.struct_span_err(
311-
constraint.span,
312-
format!("{} `{}` is private", assoc_item.kind, constraint.ident),
313-
)
314-
.with_span_label(constraint.span, format!("private {}", assoc_item.kind))
315-
.emit();
316-
self.set_tainted_by_errors(reported);
317-
}
318-
tcx.check_stability(assoc_item.def_id, Some(hir_ref_id), constraint.span, None);
295+
let assoc_item = self
296+
.probe_assoc_item(
297+
constraint.ident,
298+
assoc_kind,
299+
hir_ref_id,
300+
constraint.span,
301+
candidate.def_id(),
302+
)
303+
.expect("failed to find associated item");
319304

320305
duplicates
321306
.entry(assoc_item.def_id)
@@ -406,10 +391,9 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
406391
// Create the generic arguments for the associated type or constant by joining the
407392
// parent arguments (the arguments of the trait) and the own arguments (the ones of
408393
// the associated item itself) and construct an alias type using them.
409-
let alias_ty = candidate.map_bound(|trait_ref| {
410-
let ident = Ident::new(assoc_item.name, constraint.ident.span);
394+
let alias_term = candidate.map_bound(|trait_ref| {
411395
let item_segment = hir::PathSegment {
412-
ident,
396+
ident: constraint.ident,
413397
hir_id: constraint.hir_id,
414398
res: Res::Err,
415399
args: Some(constraint.gen_args),
@@ -428,15 +412,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
428412
});
429413

430414
// Provide the resolved type of the associated constant to `type_of(AnonConst)`.
431-
if let hir::AssocItemConstraintKind::Equality { term: hir::Term::Const(anon_const) } =
432-
constraint.kind
433-
{
434-
let ty = alias_ty.map_bound(|ty| tcx.type_of(ty.def_id).instantiate(tcx, ty.args));
435-
let ty = check_assoc_const_binding_type(tcx, assoc_ident, ty, constraint.hir_id);
415+
if let Some(anon_const) = constraint.ct() {
416+
let ty = alias_term
417+
.map_bound(|alias| tcx.type_of(alias.def_id).instantiate(tcx, alias.args));
418+
let ty =
419+
check_assoc_const_binding_type(tcx, constraint.ident, ty, constraint.hir_id);
436420
tcx.feed_anon_const_type(anon_const.def_id, ty::EarlyBinder::bind(ty));
437421
}
438422

439-
alias_ty
423+
alias_term
440424
};
441425

442426
match constraint.kind {

compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs

+58-33
Original file line numberDiff line numberDiff line change
@@ -1166,8 +1166,10 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
11661166
};
11671167

11681168
let trait_did = bound.def_id();
1169-
let assoc_ty_did = self.probe_assoc_ty(assoc_ident, hir_ref_id, span, trait_did).unwrap();
1170-
let ty = self.lower_assoc_ty(span, assoc_ty_did, assoc_segment, bound);
1169+
let assoc_ty = self
1170+
.probe_assoc_item(assoc_ident, ty::AssocKind::Type, hir_ref_id, span, trait_did)
1171+
.expect("failed to find associated type");
1172+
let ty = self.lower_assoc_ty(span, assoc_ty.def_id, assoc_segment, bound);
11711173

11721174
if let Some(variant_def_id) = variant_resolution {
11731175
tcx.node_span_lint(AMBIGUOUS_ASSOCIATED_ITEMS, hir_ref_id, span, |lint| {
@@ -1183,7 +1185,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
11831185
};
11841186

11851187
could_refer_to(DefKind::Variant, variant_def_id, "");
1186-
could_refer_to(DefKind::AssocTy, assoc_ty_did, " also");
1188+
could_refer_to(DefKind::AssocTy, assoc_ty.def_id, " also");
11871189

11881190
lint.span_suggestion(
11891191
span,
@@ -1193,7 +1195,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
11931195
);
11941196
});
11951197
}
1196-
Ok((ty, DefKind::AssocTy, assoc_ty_did))
1198+
Ok((ty, DefKind::AssocTy, assoc_ty.def_id))
11971199
}
11981200

11991201
fn probe_inherent_assoc_ty(
@@ -1220,7 +1222,11 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
12201222
let candidates: Vec<_> = tcx
12211223
.inherent_impls(adt_did)?
12221224
.iter()
1223-
.filter_map(|&impl_| Some((impl_, self.probe_assoc_ty_unchecked(name, block, impl_)?)))
1225+
.filter_map(|&impl_| {
1226+
let (item, scope) =
1227+
self.probe_assoc_item_unchecked(name, ty::AssocKind::Type, block, impl_)?;
1228+
Some((impl_, (item.def_id, scope)))
1229+
})
12241230
.collect();
12251231

12261232
if candidates.is_empty() {
@@ -1264,7 +1270,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
12641270
},
12651271
)?;
12661272

1267-
self.check_assoc_ty(assoc_item, name, def_scope, block, span);
1273+
self.check_assoc_item(assoc_item, name, def_scope, block, span);
12681274

12691275
// FIXME(fmease): Currently creating throwaway `parent_args` to please
12701276
// `lower_generic_args_of_assoc_item`. Modify the latter instead (or sth. similar) to
@@ -1351,50 +1357,69 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
13511357
}
13521358
}
13531359

1354-
fn probe_assoc_ty(&self, name: Ident, block: HirId, span: Span, scope: DefId) -> Option<DefId> {
1355-
let (item, def_scope) = self.probe_assoc_ty_unchecked(name, block, scope)?;
1356-
self.check_assoc_ty(item, name, def_scope, block, span);
1360+
/// Given name and kind search for the assoc item in the provided scope and check if it's accessible[^1].
1361+
///
1362+
/// [^1]: I.e., accessible in the provided scope wrt. visibility and stability.
1363+
fn probe_assoc_item(
1364+
&self,
1365+
ident: Ident,
1366+
kind: ty::AssocKind,
1367+
block: HirId,
1368+
span: Span,
1369+
scope: DefId,
1370+
) -> Option<ty::AssocItem> {
1371+
let (item, scope) = self.probe_assoc_item_unchecked(ident, kind, block, scope)?;
1372+
self.check_assoc_item(item.def_id, ident, scope, block, span);
13571373
Some(item)
13581374
}
13591375

1360-
fn probe_assoc_ty_unchecked(
1376+
/// Given name and kind search for the assoc item in the provided scope
1377+
/// *without* checking if it's accessible[^1].
1378+
///
1379+
/// [^1]: I.e., accessible in the provided scope wrt. visibility and stability.
1380+
fn probe_assoc_item_unchecked(
13611381
&self,
1362-
name: Ident,
1382+
ident: Ident,
1383+
kind: ty::AssocKind,
13631384
block: HirId,
13641385
scope: DefId,
1365-
) -> Option<(DefId, DefId)> {
1386+
) -> Option<(ty::AssocItem, /*scope*/ DefId)> {
13661387
let tcx = self.tcx();
1367-
let (ident, def_scope) = tcx.adjust_ident_and_get_scope(name, scope, block);
13681388

1389+
let (ident, def_scope) = tcx.adjust_ident_and_get_scope(ident, scope, block);
13691390
// We have already adjusted the item name above, so compare with `.normalize_to_macros_2_0()`
13701391
// instead of calling `filter_by_name_and_kind` which would needlessly normalize the
13711392
// `ident` again and again.
1372-
let item = tcx.associated_items(scope).in_definition_order().find(|i| {
1373-
i.kind.namespace() == Namespace::TypeNS
1374-
&& i.ident(tcx).normalize_to_macros_2_0() == ident
1375-
})?;
1393+
let item = tcx
1394+
.associated_items(scope)
1395+
.filter_by_name_unhygienic(ident.name)
1396+
.find(|i| i.kind == kind && i.ident(tcx).normalize_to_macros_2_0() == ident)?;
13761397

1377-
Some((item.def_id, def_scope))
1398+
Some((*item, def_scope))
13781399
}
13791400

1380-
fn check_assoc_ty(&self, item: DefId, name: Ident, def_scope: DefId, block: HirId, span: Span) {
1401+
/// Check if the given assoc item is accessible in the provided scope wrt. visibility and stability.
1402+
fn check_assoc_item(
1403+
&self,
1404+
item_def_id: DefId,
1405+
ident: Ident,
1406+
scope: DefId,
1407+
block: HirId,
1408+
span: Span,
1409+
) {
13811410
let tcx = self.tcx();
1382-
let kind = DefKind::AssocTy;
1383-
1384-
if !tcx.visibility(item).is_accessible_from(def_scope, tcx) {
1385-
let kind = tcx.def_kind_descr(kind, item);
1386-
let msg = format!("{kind} `{name}` is private");
1387-
let def_span = tcx.def_span(item);
1388-
let reported = tcx
1389-
.dcx()
1390-
.struct_span_err(span, msg)
1391-
.with_code(E0624)
1392-
.with_span_label(span, format!("private {kind}"))
1393-
.with_span_label(def_span, format!("{kind} defined here"))
1394-
.emit();
1411+
1412+
if !tcx.visibility(item_def_id).is_accessible_from(scope, tcx) {
1413+
let reported = tcx.dcx().emit_err(crate::errors::AssocItemIsPrivate {
1414+
span,
1415+
kind: tcx.def_descr(item_def_id),
1416+
name: ident,
1417+
defined_here_label: tcx.def_span(item_def_id),
1418+
});
13951419
self.set_tainted_by_errors(reported);
13961420
}
1397-
tcx.check_stability(item, Some(block), span, None);
1421+
1422+
tcx.check_stability(item_def_id, Some(block), span, None);
13981423
}
13991424

14001425
fn probe_traits_that_match_assoc_ty(

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error[E0624]: associated type `P` is private
22
--> $DIR/assoc-inherent-private.rs:10:10
33
|
44
LL | type P = ();
5-
| ------ associated type defined here
5+
| ------ the associated type is defined here
66
...
77
LL | type U = m::T::P;
88
| ^^^^^^^ private associated type
@@ -11,7 +11,7 @@ error[E0624]: associated type `P` is private
1111
--> $DIR/assoc-inherent-private.rs:21:10
1212
|
1313
LL | pub(super) type P = bool;
14-
| ----------------- associated type defined here
14+
| ----------------- the associated type is defined here
1515
...
1616
LL | type V = n::n::T::P;
1717
| ^^^^^^^^^^ private associated type

tests/ui/traits/item-privacy.stderr

+5-2
Original file line numberDiff line numberDiff line change
@@ -195,14 +195,17 @@ error[E0624]: associated type `A` is private
195195
--> $DIR/item-privacy.rs:119:12
196196
|
197197
LL | type A = u8;
198-
| ------ associated type defined here
198+
| ------ the associated type is defined here
199199
...
200200
LL | let _: T::A;
201201
| ^^^^ private associated type
202202

203-
error: associated type `A` is private
203+
error[E0624]: associated type `A` is private
204204
--> $DIR/item-privacy.rs:128:9
205205
|
206+
LL | type A = u8;
207+
| ------ the associated type is defined here
208+
...
206209
LL | A = u8,
207210
| ^^^^^^ private associated type
208211

0 commit comments

Comments
 (0)