delegation: split resolution and lowering#157296
Conversation
rustbot
added
S-waiting-on-review
aerooneqq
changed the title
| } | ||
|
|
||
| impl Delegation { | ||
| pub fn span(&self) -> Span { |
There was a problem hiding this comment.
| pub fn span(&self) -> Span { | |
| pub fn last_segment_span(&self) -> Span { |
Nit: the name is too generic, this is some very specific span inside the delegation, not the whole delegation span.
| mid_hir::Crate::new(owners, delayed_ids, delayed_resolver, opt_hir_hash) | ||
| } | ||
|
|
||
| fn opt_delegation<'a>(ast_owner: &'a AstOwner<'a>) -> Option<&'a Delegation> { |
There was a problem hiding this comment.
This could be a method on AstOwner.
Also could rename this to just delegation because there's no non-opt version.
| match ast_owner { | ||
| AstOwner::Item(Item { kind: ItemKind::Delegation(d), .. }) | ||
| | AstOwner::AssocItem(Item { kind: AssocItemKind::Delegation(d), .. }, _) => { | ||
| Some(d.as_ref()) |
There was a problem hiding this comment.
| let (resolver, ast_crate) = &*krate.delayed_resolver.borrow(); | ||
|
|
||
| // FIXME!!!(fn_delegation): make ast index lifetime same as resolver, | ||
| // as it is too bad to reindex whole crate on each delegation lowering. |
There was a problem hiding this comment.
Sigh, we are doing it again.
| } | ||
| } | ||
|
|
||
| pub fn delegations_resolutions(tcx: TyCtxt<'_>, _: ()) -> FxIndexMap<LocalDefId, Option<DefId>> { |
There was a problem hiding this comment.
| pub fn delegations_resolutions(tcx: TyCtxt<'_>, _: ()) -> FxIndexMap<LocalDefId, Option<DefId>> { | |
| pub fn delegations_resolutions(tcx: TyCtxt<'_>, _: ()) -> FxIndexMap<LocalDefId, Result<DefId, ErrorGuaranteed>> { |
Otherwise it's confusing why the Nones are in the table at all.
Can we get the semantics "not in the table - not delegation, Err in the table - unsuccessfully resolved delegation" here?
| let span = delegation.span(); | ||
|
|
||
| let res_id = resolver | ||
| .delegation_info(def_id) |
There was a problem hiding this comment.
Same with delegation_infos, can we keep Result<..., ErrorGuaranteed>s in them?
Otherwise it's not clear what not being in the table means.
| } | ||
| } | ||
|
|
||
| pub fn delegations_resolutions(tcx: TyCtxt<'_>, _: ()) -> FxIndexMap<LocalDefId, Option<DefId>> { |
There was a problem hiding this comment.
It's probably better to move this function to its old place in delegation.rs.
| result | ||
| } | ||
|
|
||
| fn resolve_delegation(tcx: TyCtxt<'_>, mut def_id: DefId, span: Span) -> Option<DefId> { |
There was a problem hiding this comment.
| fn resolve_delegation(tcx: TyCtxt<'_>, mut def_id: DefId, span: Span) -> Option<DefId> { | |
| fn resolve_delegation(tcx: TyCtxt<'_>, mut def_id: DefId, span: Span) -> Result<DefId, ErrorGuaranteed> { |
Same as above.
| } | ||
|
|
||
| fn force_delayed_owners_lowering(tcx: TyCtxt<'_>) { | ||
| tcx.ensure_done().delegations_resolutions(()); |
There was a problem hiding this comment.
Is it necessary to ensure this here?
What happens if delegations_resolutions is first called from some lower_delayed_owner below?
petrochenkov
added
S-waiting-on-author
3 participants
This PR splits delegation's AST -> HIR lowering and its resolution. Now we resolve all delegations and then lower them. This should have benefits:
For delegation: do not always generate first argument #156798, where it will be convenient to insert diagnostics about specifying target expressions for glob reuses of only static functions (thedelegations_resolutionsmap will contain information whether to lower or delete target expression)Part of #118212.
r? @petrochenkov