Opaque types should not be HIR items (and thus not be HIR owners)

[camelid]

Summary

• Delete hir::ItemKind::OpaqueTy, add hir::Node::OpaqueTy
• Replace the ItemId in hir::TyKind::OpaqueDef with &'hir OpaqueTy
• Fix the fallout

A WIP version of much of this can be seen in 79a3e7a...dcb9e9a, but it's incomplete. The reason is that many parts of the compiler assume that opaques are items and handle them through implicit codepaths.

Background

Currently, opaque types (e.g., return-position impl Trait) are lowered into a hir::ItemKind::OpaqueTy and then a hir::TyKind::OpaqueDef that references that item's ID. However, because opaques' DefIds are created in rustc_ast_lowering, yet opaques can contain nodes whose DefIds are created in DefCollector (such as anon consts, which may in turn contain arbitrary nested items), this causes an incorrect parenting structure. This issue was uncovered in #128844, and it impeded that change, which delayed creation of defs for all expression-like nodes (anon consts, closures, etc.) until ast_lowering to fix other issues relating to def parents. @BoxyUwU summarized the issue in that PR's thread, reproduced here:

Context for why the Item::OpaqueTy changes since @cjgillot has started looking at this PR:

note: "this PR" indicates "this PR before the changes to opaques" as this was written from before those changes were made

Given the code example:

trait Trait<const N: usize> {}

#[rustfmt::skip]
fn foo() -> impl Trait<{ struct Bar; 1 }> {}

impl Trait<1> for () {}

The free_items for this module are:

free_items: [
  ItemId { owner_id: DefId(0:1 ~ playground_1[9aa8]::{use#0}) },
  ItemId { owner_id: DefId(0:2 ~ playground_1[9aa8]::std) },
  ItemId { owner_id: DefId(0:3 ~ playground_1[9aa8]::Trait) },
  ItemId { owner_id: DefId(0:7 ~ playground_1[9aa8]::foo) },
  ItemId { owner_id: DefId(0:11 ~ playground_1[9aa8]::foo::{opaque#0}) },
  ItemId { owner_id: DefId(0:9 ~ playground_1[9aa8]::foo::{constant#0}::Bar) },
  ItemId { owner_id: DefId(0:5 ~ playground_1[9aa8]::{impl#0}) }
]

so, HirIdValidator gets used on foo, opaque#0 and Bar, but not the anon const

* `foo` it will recurse and call `visit_nested_item` on the opaque which doesn't recurse, and just checks that the parent `DefId`'s hir's owner is `foo`

* `opaque#0`it will recurse and... check the `HirId` of the anon const is right (for some definition of right, unrelated to any of this), then recurse inside it, then visit `Bar` as a nested item and check that: `Bar`'s def parent's hir's owner is `opaque#0`,
  
  * On nightly that def parent is the anon const, and the anon consts hir is indeed owned by opaque#0
  * On this PR where items (that arent nested bodies) that are in anon consts are not parented to the anon const, `Bar`'s def parent is `foo`, so we attempt to check that `foo`'s hir's owner is `opaque#0` which it is not so we ICE

On nightly: The def parent setup for Bar is foo::{constant}::Bar, foo and opaque#0 are hir owners. When we validate opaque's HirId's, it succeeds as we ignore anon consts, and the fact that constant exists means that Bar's parent's hir is owned opaque. On this PR: The def parent setup for bar is foo::Bar, foo and opaque#0 are hir owners. When we validate opaques HirIds, it fails as constant#0 is no longer the parent of Bar so it correctly detects that we have a nested thing in opaque#0 with a parent that is "outside" of opaque in a different hir owner's hir

Conclusion: Opaque types either need to have their DefId created in DefCollector or needs to stop being a hir owner. Or, generally, if something is to be a hir owner node, the DefId for it should be created before all children DefIds or else you cannot create the parent structure correctly.

As mentioned above, the alternative is to create DefIds for opaques ahead-of-time, in DefCollector, but that is unsatisfactory because lowering of opaque types is quite complicated and that logic exists in rustc_ast_lowering. IMO it also makes more intuitive sense to create defs for things that don't themselves participate in resolution, such as anon consts and opaque types, during ast_lowering rather than def collection.

[compiler-errors]

I talked to Boxy and I'd like to take a look at this, unless someone like @cjgillot knows how to fix it and has the time. I've relabeled this as E-hard since it likely requires a lot of patience going through the HIR visitors to ensure they're handling opaques correctly now.

[cjgillot]

I posted a draft here: #129244