Address review comments

Author: aerooneqq

compiler/rustc_ast/src/ast.rs

@@ -3922,7 +3922,7 @@ pub struct Delegation {
 }
 
 impl Delegation {
-    pub fn span(&self) -> Span {
+    pub fn last_segment_span(&self) -> Span {
         self.path.segments.last().unwrap().ident.span
     }
 }

compiler/rustc_ast_lowering/src/delegation.rs

@@ -44,17 +44,21 @@ use hir::{BodyId, HirId};
 use rustc_abi::ExternAbi;
 use rustc_ast as ast;
 use rustc_ast::*;
+use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
 use rustc_hir::attrs::{AttributeKind, InlineAttr};
-use rustc_hir::def_id::DefId;
+use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::{self as hir, FnDeclFlags};
 use rustc_middle::span_bug;
-use rustc_middle::ty::Asyncness;
+use rustc_middle::ty::{Asyncness, TyCtxt};
 use rustc_span::symbol::kw;
-use rustc_span::{Ident, Span, Symbol};
+use rustc_span::{ErrorGuaranteed, Ident, Span, Symbol};
+use smallvec::SmallVec;
 
 use crate::delegation::generics::{GenericsGenerationResult, GenericsGenerationResults};
+use crate::errors::CycleInDelegationSignatureResolution;
 use crate::{
     AllowReturnTypeNotation, ImplTraitContext, ImplTraitPosition, LoweringContext, ParamMode,
+    ResolverAstLoweringExt, index_crate,
 };
 
 mod generics;
@@ -100,6 +104,66 @@ static ATTRS_ADDITIONS: &[AttrAdditionInfo] = &[
     },
 ];
 
+pub fn delegations_resolutions(
+    tcx: TyCtxt<'_>,
+    _: (),
+) -> FxIndexMap<LocalDefId, Result<DefId, ErrorGuaranteed>> {
+    let krate = tcx.hir_crate(());
+
+    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.
+    let ast_index = index_crate(resolver, ast_crate);
+
+    let mut result = FxIndexMap::<LocalDefId, Result<DefId, ErrorGuaranteed>>::default();
+
+    for &def_id in &krate.delayed_ids {
+        let delegation = ast_index[def_id].delegation().expect("processing delegations");
+        let span = delegation.last_segment_span();
+
+        if let Some(res_result) = resolver.delegation_info(def_id) {
+            let res = res_result.map(|info| check_for_cycles(tcx, info.resolution_id, span));
+            result.insert(def_id, res.flatten());
+        }
+    }
+
+    result
+}
+
+fn check_for_cycles(
+    tcx: TyCtxt<'_>,
+    mut def_id: DefId,
+    span: Span,
+) -> Result<DefId, ErrorGuaranteed> {
+    let mut visited: FxHashSet<DefId> = Default::default();
+    let mut path: SmallVec<[DefId; 1]> = Default::default();
+
+    let (resolver, _) = &*tcx.hir_crate(()).delayed_resolver.borrow();
+
+    loop {
+        visited.insert(def_id);
+
+        path.push(def_id);
+
+        // If def_id is in local crate and it corresponds to another delegation
+        // it means that we refer to another delegation as a callee, so in order to obtain
+        // a signature DefId we obtain NodeId of the callee delegation and try to get signature from it.
+        if let Some(local_id) = def_id.as_local()
+            && let Some(Ok(delegation_info)) = resolver.delegation_info(local_id)
+        {
+            def_id = delegation_info.resolution_id;
+            if visited.contains(&def_id) {
+                // We encountered a cycle in the resolution, or delegation callee refers to non-existent
+                // entity, in this case emit an error.
+                return Err(tcx.dcx().emit_err(CycleInDelegationSignatureResolution { span }));
+            }
+        } else {
+            return Ok(path[0]);
+        }
+    }
+}
+
 impl<'hir> LoweringContext<'_, 'hir> {
     fn is_method(&self, def_id: DefId, span: Span) -> bool {
         match self.tcx.def_kind(def_id) {
@@ -114,13 +178,12 @@ impl<'hir> LoweringContext<'_, 'hir> {
         delegation: &Delegation,
         item_id: NodeId,
     ) -> DelegationResults<'hir> {
-        let span = self.lower_span(delegation.span());
+        let span = self.lower_span(delegation.last_segment_span());
 
-        let sig_id =
-            self.tcx.delegations_resolutions(()).get(&self.owner.def_id).copied().flatten();
+        let sig_id = self.tcx.delegations_resolutions(()).get(&self.owner.def_id).copied();
 
         // Delegation can be unresolved in illegal places such as function bodies in extern blocks (see #151356)
-        let Some(sig_id) = sig_id else {
+        let Some(Ok(sig_id)) = sig_id else {
             self.dcx().span_delayed_bug(
                 span,
                 format!("LoweringContext: the delegation {:?} is unresolved", item_id),

compiler/rustc_ast_lowering/src/errors.rs

@@ -522,13 +522,6 @@ pub(crate) struct UnionWithDefault {
     pub span: Span,
 }
 
-#[derive(Diagnostic)]
-#[diag("failed to resolve delegation callee")]
-pub(crate) struct UnresolvedDelegationCallee {
-    #[primary_span]
-    pub span: Span,
-}
-
 #[derive(Diagnostic)]
 #[diag("encountered a cycle during delegation signature resolution")]
 pub(crate) struct CycleInDelegationSignatureResolution {

compiler/rustc_ast_lowering/src/lib.rs

@@ -43,14 +43,14 @@ use rustc_ast::visit::Visitor;
 use rustc_ast::{self as ast, *};
 use rustc_attr_parsing::{AttributeParser, OmitDoc, Recovery, ShouldEmit};
 use rustc_data_structures::fingerprint::Fingerprint;
-use rustc_data_structures::fx::{FxHashSet, FxIndexMap, FxIndexSet};
+use rustc_data_structures::fx::FxIndexSet;
 use rustc_data_structures::sorted_map::SortedMap;
 use rustc_data_structures::stable_hash::{StableHash, StableHasher};
 use rustc_data_structures::steal::Steal;
 use rustc_data_structures::tagged_ptr::TaggedRef;
 use rustc_errors::{DiagArgFromDisplay, DiagCtxtHandle};
 use rustc_hir::def::{DefKind, LifetimeRes, Namespace, PartialRes, PerNS, Res};
-use rustc_hir::def_id::{CRATE_DEF_ID, DefId, LOCAL_CRATE, LocalDefId};
+use rustc_hir::def_id::{CRATE_DEF_ID, LOCAL_CRATE, LocalDefId};
 use rustc_hir::definitions::PerParentDisambiguatorState;
 use rustc_hir::lints::DelayedLint;
 use rustc_hir::{
@@ -64,15 +64,12 @@ use rustc_middle::span_bug;
 use rustc_middle::ty::{DelegationInfo, PerOwnerResolverData, ResolverAstLowering, TyCtxt};
 use rustc_session::errors::add_feature_diagnostics;
 use rustc_span::symbol::{Ident, Symbol, kw, sym};
-use rustc_span::{DUMMY_SP, DesugaringKind, Span};
+use rustc_span::{DUMMY_SP, DesugaringKind, ErrorGuaranteed, Span};
 use smallvec::SmallVec;
 use thin_vec::ThinVec;
 use tracing::{debug, instrument, trace};
 
-use crate::errors::{
-    AssocTyParentheses, AssocTyParenthesesSub, CycleInDelegationSignatureResolution,
-    MisplacedImplTrait, UnresolvedDelegationCallee,
-};
+use crate::errors::{AssocTyParentheses, AssocTyParenthesesSub, MisplacedImplTrait};
 use crate::item::Owners;
 
 macro_rules! arena_vec {
@@ -84,7 +81,7 @@ macro_rules! arena_vec {
 mod asm;
 mod block;
 mod contract;
-mod delegation;
+pub mod delegation;
 mod errors;
 mod expr;
 mod format;
@@ -307,8 +304,8 @@ impl<'tcx> ResolverAstLowering<'tcx> {
         self.extra_lifetime_params_map.get(&id).map_or(&[], |v| &v[..])
     }
 
-    fn delegation_info(&self, id: LocalDefId) -> Option<&DelegationInfo> {
-        self.delegation_infos.get(&id)
+    fn delegation_info(&self, id: LocalDefId) -> Option<Result<DelegationInfo, ErrorGuaranteed>> {
+        self.delegation_infos.get(&id).copied()
     }
 
     fn owner_def_id(&self, id: NodeId) -> LocalDefId {
@@ -441,6 +438,16 @@ enum AstOwner<'a> {
     ForeignItem(&'a ast::ForeignItem),
 }
 
+impl AstOwner<'_> {
+    fn delegation(&self) -> Option<&ast::Delegation> {
+        match self {
+            AstOwner::Item(Item { kind: ItemKind::Delegation(d), .. })
+            | AstOwner::AssocItem(Item { kind: AssocItemKind::Delegation(d), .. }, _) => Some(d),
+            _ => None,
+        }
+    }
+}
+
 #[derive(Copy, Clone, Debug)]
 enum TryBlockScope {
     /// There isn't a `try` block, so a `?` will use `return`.
@@ -544,7 +551,7 @@ pub fn lower_to_hir(tcx: TyCtxt<'_>, (): ()) -> mid_hir::Crate<'_> {
     let mut delayed_ids: FxIndexSet<LocalDefId> = Default::default();
 
     for def_id in ast_index.indices() {
-        if opt_delegation(&ast_index[def_id]).is_some() {
+        if ast_index[def_id].delegation().is_some() {
             delayed_ids.insert(def_id);
         } else {
             lowerer.lower_node(def_id);
@@ -559,16 +566,6 @@ pub fn lower_to_hir(tcx: TyCtxt<'_>, (): ()) -> mid_hir::Crate<'_> {
     mid_hir::Crate::new(owners, delayed_ids, delayed_resolver, opt_hir_hash)
 }
 
-fn opt_delegation<'a>(ast_owner: &'a AstOwner<'a>) -> Option<&'a Delegation> {
-    match ast_owner {
-        AstOwner::Item(Item { kind: ItemKind::Delegation(d), .. })
-        | AstOwner::AssocItem(Item { kind: AssocItemKind::Delegation(d), .. }, _) => {
-            Some(d.as_ref())
-        }
-        _ => None,
-    }
-}
-
 /// Lowers an AST owner corresponding to `def_id`, now only delegations are lowered this way.
 pub fn lower_delayed_owner(tcx: TyCtxt<'_>, def_id: LocalDefId) {
     let krate = tcx.hir_crate(());
@@ -594,65 +591,6 @@ pub fn lower_delayed_owner(tcx: TyCtxt<'_>, def_id: LocalDefId) {
     }
 }
 
-pub fn delegations_resolutions(tcx: TyCtxt<'_>, _: ()) -> FxIndexMap<LocalDefId, Option<DefId>> {
-    let krate = tcx.hir_crate(());
-
-    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.
-    let ast_index = index_crate(resolver, ast_crate);
-
-    let mut result = FxIndexMap::<LocalDefId, Option<DefId>>::default();
-
-    for &def_id in &krate.delayed_ids {
-        let delegation = opt_delegation(&ast_index[def_id]).expect("processing delegations");
-        let span = delegation.span();
-
-        let res_id = resolver
-            .delegation_info(def_id)
-            .and_then(|info| resolve_delegation(tcx, info.resolution_id, span));
-
-        result.insert(def_id, res_id);
-    }
-
-    result
-}
-
-fn resolve_delegation(tcx: TyCtxt<'_>, mut def_id: DefId, span: Span) -> Option<DefId> {
-    let mut visited: FxHashSet<DefId> = Default::default();
-    let mut path: SmallVec<[DefId; 1]> = Default::default();
-
-    let (resolver, _) = &*tcx.hir_crate(()).delayed_resolver.borrow();
-
-    loop {
-        visited.insert(def_id);
-
-        path.push(def_id);
-
-        // If def_id is in local crate and it corresponds to another delegation
-        // it means that we refer to another delegation as a callee, so in order to obtain
-        // a signature DefId we obtain NodeId of the callee delegation and try to get signature from it.
-        if let Some(local_id) = def_id.as_local()
-            && let Some(delegation_info) = resolver.delegation_info(local_id)
-        {
-            def_id = delegation_info.resolution_id;
-            if visited.contains(&def_id) {
-                // We encountered a cycle in the resolution, or delegation callee refers to non-existent
-                // entity, in this case emit an error.
-                match visited.len() {
-                    1 => tcx.dcx().emit_err(UnresolvedDelegationCallee { span }),
-                    _ => tcx.dcx().emit_err(CycleInDelegationSignatureResolution { span }),
-                };
-
-                return None;
-            }
-        } else {
-            return Some(path[0]);
-        }
-    }
-}
-
 #[derive(Copy, Clone, PartialEq, Debug)]
 enum ParamMode {
     /// Any path in a type context.

compiler/rustc_interface/src/passes.rs

@@ -879,7 +879,8 @@ pub static DEFAULT_QUERY_PROVIDERS: LazyLock<Providers> = LazyLock::new(|| {
     providers.queries.analysis = analysis;
     providers.queries.hir_crate = rustc_ast_lowering::lower_to_hir;
     providers.queries.lower_delayed_owner = rustc_ast_lowering::lower_delayed_owner;
-    providers.queries.delegations_resolutions = rustc_ast_lowering::delegations_resolutions;
+    providers.queries.delegations_resolutions =
+        rustc_ast_lowering::delegation::delegations_resolutions;
     // `hir_delayed_owner` is fed during `lower_delayed_owner`, by default it returns phantom,
     // as if this query was not fed it means that `MaybeOwner` does not exist for provided LocalDefId.
     providers.queries.hir_delayed_owner = |_, _| MaybeOwner::Phantom;

compiler/rustc_middle/src/hir/map.rs

@@ -1277,8 +1277,6 @@ pub(super) fn hir_module_items(tcx: TyCtxt<'_>, module_id: LocalModDefId) -> Mod
 }
 
 fn force_delayed_owners_lowering(tcx: TyCtxt<'_>) {
-    tcx.ensure_done().delegations_resolutions(());
-
     let krate = tcx.hir_crate(());
     for &id in &krate.delayed_ids {
         tcx.ensure_done().lower_delayed_owner(id);

compiler/rustc_middle/src/queries.rs

@@ -2065,7 +2065,7 @@ rustc_queries! {
         desc { "inheriting delegation signature" }
     }
 
-    query delegations_resolutions(_: ()) -> &'tcx FxIndexMap<LocalDefId, Option<DefId>> {
+    query delegations_resolutions(_: ()) -> &'tcx FxIndexMap<LocalDefId, Result<DefId, ErrorGuaranteed>> {
         arena_cache
         eval_always
         desc { "getting delegations resolutions" }

compiler/rustc_middle/src/ty/mod.rs

@@ -257,12 +257,12 @@ pub struct ResolverAstLowering<'tcx> {
     pub lint_buffer: Steal<LintBuffer>,
 
     // Information about delegations which is used when handling recursive delegations
-    pub delegation_infos: LocalDefIdMap<DelegationInfo>,
+    pub delegation_infos: LocalDefIdMap<Result<DelegationInfo, ErrorGuaranteed>>,
 
     pub disambiguators: LocalDefIdMap<Steal<PerParentDisambiguatorState>>,
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone, Copy)]
 pub struct DelegationInfo {
     // `DefId` (either the resolution at delegation.id or item_id in case of a trait impl) for signature resolution,
     // for details see https://github.com/rust-lang/rust/issues/118212#issuecomment-2160686914

compiler/rustc_resolve/src/errors.rs

@@ -1776,3 +1776,10 @@ pub(crate) enum UnusedImportsSugg {
         num_to_remove: usize,
     },
 }
+
+#[derive(Diagnostic)]
+#[diag("failed to resolve delegation callee")]
+pub(crate) struct UnresolvedDelegationCallee {
+    #[primary_span]
+    pub span: Span,
+}

compiler/rustc_resolve/src/late.rs

@@ -39,6 +39,7 @@ use smallvec::{SmallVec, smallvec};
 use thin_vec::ThinVec;
 use tracing::{debug, instrument, trace};
 
+use crate::errors::UnresolvedDelegationCallee;
 use crate::{
     BindingError, BindingKey, Decl, DelegationFnSig, Finalize, IdentKey, LateDecl, LocalModule,
     Module, ModuleOrUniformRoot, ParentScope, PathResult, Res, ResolutionError, Resolver, Segment,
@@ -3905,19 +3906,27 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
             .partial_res_map
             .get(&resolution_id)
             .and_then(|r| r.expect_full_res().opt_def_id());
-        if let Some(resolution_id) = def_id {
-            self.r
-                .delegation_infos
-                .insert(self.r.current_owner.def_id, DelegationInfo { resolution_id });
+
+        let resolution = if let Some(resolution_id) = def_id
+            // If there is a single unresolved `reuse foo` in a mod it resolves to itself,
+            // in this case emit `UnresolvedDelegationCallee` error instead of cycle error.
+            && resolution_id != self.r.current_owner.def_id.to_def_id()
+        {
+            Ok(DelegationInfo { resolution_id })
         } else {
             self.r.tcx.dcx().span_delayed_bug(
                 delegation.path.span,
                 format!(
                     "LoweringContext: couldn't resolve node {resolution_id:?} in delegation item",
                 ),
             );
+
+            let err = UnresolvedDelegationCallee { span: delegation.last_segment_span() };
+            Err(self.r.tcx.dcx().emit_err(err))
         };
 
+        self.r.delegation_infos.insert(self.r.current_owner.def_id, resolution);
+
         let Some(body) = &delegation.body else { return };
         self.with_rib(ValueNS, RibKind::FnOrCoroutine, |this| {
             let ident = Ident::new(kw::SelfLower, body.span.normalize_to_macro_rules());