Rollup merge of rust-lang#155226 - aerooneqq:delegation-hir-crate-items-revert, r=petrochenkov

delegation: revert execution of hir_crate_items before delayed lowering

This PR reverts rust-lang#154368, as after weekend consideration I don't think that it is a correct way of fixing cycles during delayed lowering:

- The number of ICEs were reported, fixing them would require to develop solution from rust-lang#154368 but I am afraid that it would lead to cancer code growing,
- The [memory regression](rust-lang#154368 (comment)) for rustdoc was reported, it can be fixed with moving `tcx.force_delayed_owners_lowering` call earlier, but it is already bad that this is call is now required everywhere, before rust-lang#154368 AST was dropped before `hir_crate_items` automatically and users of `rustc` API did not have to think about it.

I will try to come up with a more robust solution leaving rust-lang#154368 as a last resort if nothing else will work.

Re-opens rust-lang#154169.
Fixes rust-lang#155125. Fixes rust-lang#155127. Fixes rust-lang#155128. Fixes rust-lang#155164. Fixes rust-lang#155202.
Part of rust-lang#118212.

r? @petrochenkov

Author: JonathanBrouwer

compiler/rustc_ast_lowering/src/delegation.rs

@@ -433,7 +433,6 @@ impl<'hir, R: ResolverAstLoweringExt<'hir>> LoweringContext<'_, 'hir, R> {
             // also nested delegations may need to access information about this code (#154332),
             // so it is better to leave this code as opposed to bodies of extern functions,
             // which are completely erased from existence.
-            // FIXME(fn_delegation): fix `help` in error message (see `inner-attr.stderr`)
             if param_count == 0
                 && let Some(block) = block
             {

compiler/rustc_ast_lowering/src/item.rs

@@ -38,7 +38,7 @@ pub(super) enum Owners<'a, 'hir> {
 }
 
 impl<'hir> Owners<'_, 'hir> {
-    pub(super) fn get_or_insert_mut(&mut self, def_id: LocalDefId) -> &mut hir::MaybeOwner<'hir> {
+    fn get_or_insert_mut(&mut self, def_id: LocalDefId) -> &mut hir::MaybeOwner<'hir> {
         match self {
             Owners::IndexVec(index_vec) => {
                 index_vec.ensure_contains_elem(def_id, || hir::MaybeOwner::Phantom)

compiler/rustc_ast_lowering/src/lib.rs

@@ -39,7 +39,6 @@ use std::mem;
 use std::sync::Arc;
 
 use rustc_ast::node_id::NodeMap;
-use rustc_ast::visit::AssocCtxt;
 use rustc_ast::{self as ast, *};
 use rustc_attr_parsing::{AttributeParser, Late, OmitDoc};
 use rustc_data_structures::fingerprint::Fingerprint;
@@ -634,29 +633,13 @@ 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() {
-        let delayed_owner_kind = match &ast_index[def_id] {
-            AstOwner::Item(Item { kind: ItemKind::Delegation(_), .. }) => {
-                Some(hir::DelayedOwnerKind::Item)
+        match &ast_index[def_id] {
+            AstOwner::Item(Item { kind: ItemKind::Delegation { .. }, .. })
+            | AstOwner::AssocItem(Item { kind: AssocItemKind::Delegation { .. }, .. }, _) => {
+                delayed_ids.insert(def_id);
             }
-            AstOwner::AssocItem(Item { kind: AssocItemKind::Delegation(_), .. }, ctx) => {
-                Some(match ctx {
-                    AssocCtxt::Trait => hir::DelayedOwnerKind::TraitItem,
-                    AssocCtxt::Impl { .. } => hir::DelayedOwnerKind::ImplItem,
-                })
-            }
-            _ => None,
+            _ => lowerer.lower_node(def_id),
         };
-
-        if let Some(kind) = delayed_owner_kind {
-            delayed_ids.insert(def_id);
-
-            let owner = lowerer.owners.get_or_insert_mut(def_id);
-            if let hir::MaybeOwner::Phantom = owner {
-                *owner = hir::MaybeOwner::Delayed(kind)
-            }
-        } else {
-            lowerer.lower_node(def_id);
-        }
     }
 
     // Don't hash unless necessary, because it's expensive.

compiler/rustc_hir/src/hir.rs

@@ -1641,18 +1641,10 @@ impl<'tcx> OwnerInfo<'tcx> {
     }
 }
 
-#[derive(Copy, Clone, Debug, HashStable_Generic)]
-pub enum DelayedOwnerKind {
-    Item,
-    ImplItem,
-    TraitItem,
-}
-
 #[derive(Copy, Clone, Debug, HashStable_Generic)]
 pub enum MaybeOwner<'tcx> {
     Owner(&'tcx OwnerInfo<'tcx>),
     NonOwner(HirId),
-    Delayed(DelayedOwnerKind),
     /// Used as a placeholder for unused LocalDefId.
     Phantom,
 }
@@ -1661,19 +1653,12 @@ impl<'tcx> MaybeOwner<'tcx> {
     pub fn as_owner(self) -> Option<&'tcx OwnerInfo<'tcx>> {
         match self {
             MaybeOwner::Owner(i) => Some(i),
-            _ => None,
+            MaybeOwner::NonOwner(_) | MaybeOwner::Phantom => None,
         }
     }
 
     pub fn unwrap(self) -> &'tcx OwnerInfo<'tcx> {
-        self.as_owner().unwrap_or_else(|| panic!("not a HIR owner"))
-    }
-
-    pub fn expect_delayed(self) -> DelayedOwnerKind {
-        match self {
-            MaybeOwner::Delayed(delayed_owner) => delayed_owner,
-            _ => panic!("not a delayed owner"),
-        }
+        self.as_owner().unwrap_or_else(|| panic!("Not a HIR owner"))
     }
 }
 

compiler/rustc_hir/src/intravisit.rs

@@ -226,11 +226,6 @@ pub trait Visitor<'v>: Sized {
     /// or `ControlFlow<T>`.
     type Result: VisitorResult = ();
 
-    #[inline]
-    fn visit_if_delayed(&self, _: LocalDefId) -> bool {
-        true
-    }
-
     /// If `type NestedFilter` is set to visit nested items, this method
     /// must also be overridden to provide a map to retrieve nested items.
     fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
@@ -249,23 +244,18 @@ pub trait Visitor<'v>: Sized {
     /// this method is if you want a nested pattern but cannot supply a
     /// `TyCtxt`; see `maybe_tcx` for advice.
     fn visit_nested_item(&mut self, id: ItemId) -> Self::Result {
-        if self.should_visit_maybe_delayed_inter(id.owner_id.def_id) {
+        if Self::NestedFilter::INTER {
             let item = self.maybe_tcx().hir_item(id);
             try_visit!(self.visit_item(item));
         }
         Self::Result::output()
     }
 
-    // Now delayed owners are only delegations, which are either item, trait item or impl item.
-    fn should_visit_maybe_delayed_inter(&mut self, id: LocalDefId) -> bool {
-        Self::NestedFilter::INTER && self.visit_if_delayed(id)
-    }
-
     /// Like `visit_nested_item()`, but for trait items. See
     /// `visit_nested_item()` for advice on when to override this
     /// method.
     fn visit_nested_trait_item(&mut self, id: TraitItemId) -> Self::Result {
-        if self.should_visit_maybe_delayed_inter(id.owner_id.def_id) {
+        if Self::NestedFilter::INTER {
             let item = self.maybe_tcx().hir_trait_item(id);
             try_visit!(self.visit_trait_item(item));
         }
@@ -276,7 +266,7 @@ pub trait Visitor<'v>: Sized {
     /// `visit_nested_item()` for advice on when to override this
     /// method.
     fn visit_nested_impl_item(&mut self, id: ImplItemId) -> Self::Result {
-        if self.should_visit_maybe_delayed_inter(id.owner_id.def_id) {
+        if Self::NestedFilter::INTER {
             let item = self.maybe_tcx().hir_impl_item(id);
             try_visit!(self.visit_impl_item(item));
         }

compiler/rustc_interface/src/passes.rs

@@ -1054,10 +1054,6 @@ pub fn emit_delayed_lints(tcx: TyCtxt<'_>) {
 /// Runs all analyses that we guarantee to run, even if errors were reported in earlier analyses.
 /// This function never fails.
 fn run_required_analyses(tcx: TyCtxt<'_>) {
-    // Forces all delayed owners to be lowered and drops AST crate after it.
-    // Also refetches hir_crate_items to prevent multiple threads from blocking on it later.
-    tcx.force_delayed_owners_lowering();
-
     if tcx.sess.opts.unstable_opts.input_stats {
         rustc_passes::input_stats::print_hir_stats(tcx);
     }
@@ -1066,6 +1062,11 @@ fn run_required_analyses(tcx: TyCtxt<'_>) {
     #[cfg(all(not(doc), debug_assertions))]
     rustc_passes::hir_id_validator::check_crate(tcx);
 
+    // Prefetch this to prevent multiple threads from blocking on it later.
+    // This is needed since the `hir_id_validator::check_crate` call above is not guaranteed
+    // to use `hir_crate_items`.
+    tcx.ensure_done().hir_crate_items(());
+
     let sess = tcx.sess;
     sess.time("misc_checking_1", || {
         par_fns(&mut [

compiler/rustc_middle/src/hir/map.rs

@@ -5,10 +5,9 @@
 use rustc_abi::ExternAbi;
 use rustc_ast::visit::{VisitorResult, walk_list};
 use rustc_data_structures::fingerprint::Fingerprint;
-use rustc_data_structures::fx::FxIndexSet;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::svh::Svh;
-use rustc_data_structures::sync::{DynSend, DynSync, par_for_each_in, try_par_for_each_in};
+use rustc_data_structures::sync::{DynSend, DynSync, par_for_each_in, spawn, try_par_for_each_in};
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, LOCAL_CRATE, LocalDefId, LocalModDefId};
 use rustc_hir::definitions::{DefKey, DefPath, DefPathHash};
@@ -1246,7 +1245,25 @@ pub(super) fn hir_module_items(tcx: TyCtxt<'_>, module_id: LocalModDefId) -> Mod
     }
 }
 
+fn force_delayed_owners_lowering(tcx: TyCtxt<'_>) {
+    let krate = tcx.hir_crate(());
+    for &id in &krate.delayed_ids {
+        tcx.ensure_done().lower_delayed_owner(id);
+    }
+
+    let (_, krate) = krate.delayed_resolver.steal();
+    let prof = tcx.sess.prof.clone();
+
+    // Drop AST to free memory. It can be expensive so try to drop it on a separate thread.
+    spawn(move || {
+        let _timer = prof.verbose_generic_activity("drop_ast");
+        drop(krate);
+    });
+}
+
 pub(crate) fn hir_crate_items(tcx: TyCtxt<'_>, _: ()) -> ModuleItems {
+    force_delayed_owners_lowering(tcx);
+
     let mut collector = ItemCollector::new(tcx, true);
 
     // A "crate collector" and "module collector" start at a
@@ -1307,12 +1324,11 @@ struct ItemCollector<'tcx> {
     nested_bodies: Vec<LocalDefId>,
     delayed_lint_items: Vec<OwnerId>,
     eiis: Vec<LocalDefId>,
-    delayed_ids: Option<&'tcx FxIndexSet<LocalDefId>>,
 }
 
 impl<'tcx> ItemCollector<'tcx> {
     fn new(tcx: TyCtxt<'tcx>, crate_collector: bool) -> ItemCollector<'tcx> {
-        let mut collector = ItemCollector {
+        ItemCollector {
             crate_collector,
             tcx,
             submodules: Vec::default(),
@@ -1325,46 +1341,13 @@ impl<'tcx> ItemCollector<'tcx> {
             nested_bodies: Vec::default(),
             delayed_lint_items: Vec::default(),
             eiis: Vec::default(),
-            delayed_ids: None,
-        };
-
-        if crate_collector {
-            let krate = tcx.hir_crate(());
-            collector.delayed_ids = Some(&krate.delayed_ids);
-
-            let delayed_kinds =
-                krate.delayed_ids.iter().copied().map(|id| (id, krate.owners[id].expect_delayed()));
-
-            // FIXME(fn_delegation): need to add delayed lints, eiis
-            for (def_id, kind) in delayed_kinds {
-                let owner_id = OwnerId { def_id };
-
-                match kind {
-                    DelayedOwnerKind::Item => collector.items.push(ItemId { owner_id }),
-                    DelayedOwnerKind::ImplItem => {
-                        collector.impl_items.push(ImplItemId { owner_id })
-                    }
-                    DelayedOwnerKind::TraitItem => {
-                        collector.trait_items.push(TraitItemId { owner_id })
-                    }
-                };
-
-                collector.body_owners.push(def_id);
-            }
         }
-
-        collector
     }
 }
 
 impl<'hir> Visitor<'hir> for ItemCollector<'hir> {
     type NestedFilter = nested_filter::All;
 
-    #[inline]
-    fn visit_if_delayed(&self, def_id: LocalDefId) -> bool {
-        !self.crate_collector || self.delayed_ids.is_none_or(|ids| !ids.contains(&def_id))
-    }
-
     fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
         self.tcx
     }

compiler/rustc_middle/src/hir/mod.rs

@@ -14,7 +14,7 @@ use rustc_data_structures::fx::FxIndexSet;
 use rustc_data_structures::sorted_map::SortedMap;
 use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
 use rustc_data_structures::steal::Steal;
-use rustc_data_structures::sync::{DynSend, DynSync, spawn, try_par_for_each_in};
+use rustc_data_structures::sync::{DynSend, DynSync, try_par_for_each_in};
 use rustc_hir::def::{DefKind, Res};
 use rustc_hir::def_id::{DefId, LocalDefId, LocalModDefId};
 use rustc_hir::lints::DelayedLint;
@@ -64,8 +64,7 @@ impl<'hir> Crate<'hir> {
         // which is greater than delayed LocalDefId, we use IndexVec for owners,
         // so we will call ensure_contains_elem which will grow it.
         if let Some(owner) = self.owners.get(def_id)
-            && (self.delayed_ids.is_empty()
-                || !matches!(owner, MaybeOwner::Phantom | MaybeOwner::Delayed(_)))
+            && (self.delayed_ids.is_empty() || !matches!(owner, MaybeOwner::Phantom))
         {
             return *owner;
         }
@@ -208,24 +207,6 @@ impl ModuleItems {
 }
 
 impl<'tcx> TyCtxt<'tcx> {
-    pub fn force_delayed_owners_lowering(self) {
-        let krate = self.hir_crate(());
-        self.ensure_done().hir_crate_items(());
-
-        for &id in &krate.delayed_ids {
-            self.ensure_done().lower_delayed_owner(id);
-        }
-
-        let (_, krate) = krate.delayed_resolver.steal();
-        let prof = self.sess.prof.clone();
-
-        // Drop AST to free memory. It can be expensive so try to drop it on a separate thread.
-        spawn(move || {
-            let _timer = prof.verbose_generic_activity("drop_ast");
-            drop(krate);
-        });
-    }
-
     pub fn parent_module(self, id: HirId) -> LocalModDefId {
         if !id.is_owner() && self.def_kind(id.owner) == DefKind::Mod {
             LocalModDefId::new_unchecked(id.owner.def_id)
@@ -494,8 +475,7 @@ pub fn provide(providers: &mut Providers) {
     providers.local_def_id_to_hir_id = |tcx, def_id| match tcx.hir_crate(()).owner(tcx, def_id) {
         MaybeOwner::Owner(_) => HirId::make_owner(def_id),
         MaybeOwner::NonOwner(hir_id) => hir_id,
-        MaybeOwner::Phantom => bug!("no HirId for {:?}", def_id),
-        MaybeOwner::Delayed(_) => bug!("delayed owner should be lowered {:?}", def_id),
+        MaybeOwner::Phantom => bug!("No HirId for {:?}", def_id),
     };
     providers.opt_hir_owner_nodes =
         |tcx, id| tcx.hir_crate(()).owner(tcx, id).as_owner().map(|i| &i.nodes);

src/librustdoc/lib.rs

@@ -904,7 +904,6 @@ fn main_args(early_dcx: &mut EarlyDiagCtxt, at_args: &[String]) {
                 return;
             }
 
-            tcx.force_delayed_owners_lowering();
             rustc_interface::passes::emit_delayed_lints(tcx);
 
             if render_opts.dep_info().is_some() {

tests/ui/delegation/correct_body_owner_parent_found_in_diagnostics.stderr

@@ -43,6 +43,15 @@ help: consider introducing lifetime `'a` here
 LL | impl<'a> Trait for Z {
    |     ++++
 
+error[E0599]: no associated function or constant named `new` found for struct `InvariantRef<'a, T>` in the current scope
+  --> $DIR/correct_body_owner_parent_found_in_diagnostics.rs:9:41
+   |
+LL | pub struct InvariantRef<'a, T: ?Sized>(&'a T, PhantomData<&'a mut &'a T>);
+   | -------------------------------------- associated function or constant `new` not found for this struct
+...
+LL |     pub const NEW: Self = InvariantRef::new(&());
+   |                                         ^^^ associated function or constant not found in `InvariantRef<'_, _>`
+
 error[E0277]: the trait bound `u8: Trait` is not satisfied
   --> $DIR/correct_body_owner_parent_found_in_diagnostics.rs:22:12
    |
@@ -110,15 +119,6 @@ LL |     reuse <u8 as Trait>::{foo, bar, meh} { &const { InvariantRef::<'a>::NEW
             found struct `InvariantRef<'_, ()>`
    = note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
 
-error[E0599]: no associated function or constant named `new` found for struct `InvariantRef<'a, T>` in the current scope
-  --> $DIR/correct_body_owner_parent_found_in_diagnostics.rs:9:41
-   |
-LL | pub struct InvariantRef<'a, T: ?Sized>(&'a T, PhantomData<&'a mut &'a T>);
-   | -------------------------------------- associated function or constant `new` not found for this struct
-...
-LL |     pub const NEW: Self = InvariantRef::new(&());
-   |                                         ^^^ associated function or constant not found in `InvariantRef<'_, _>`
-
 error: aborting due to 10 previous errors
 
 Some errors have detailed explanations: E0261, E0277, E0308, E0599.