refactor the AssociatedItem structures

6bb3c12
Opened by Niko Matsakis at 2019-06-13 20:39:14

During the work on https://github.com/rust-lang/rust/pull/40668, there was some discussion about refactoring the ty::AssociatedItem and hir::{Impl,Trait}ItemRef data structures. The precise plan is a bit unclear, so I'm opening this issue to try and discuss and lay it out.

Observations:

  • Although ty::AssociatedItem lives in ty, it has no real dependencies and is based entirely on the HIR, so it would better live in HIR.
    • The existing ImplItemRef and TraitItemRef are basically specialized variants of AssociatedItem, though they add a Span and use local-ids (ImplItemId) rather than a DefId.
  • There is a need for a query that given a DefId gives you back the AssociatedItem (including across crates). The local-crate portion of this query seems like it could live in hir::map quite nicely.
  • Some of this setup is carefully crafted to avoid incorrect incremental dependencies and can change as we progress on the proposed red/green changes (in particular, the current query goes to some lengths to avoid reading from Hir(X) when computing AssociatedItem(X), instead reading from the containing impl/trait; this is because we don't want to require everything that needed even basic information about X to have to change when X changes).

cc @eddyb @cramertj

  1. I think that having item-like things "out of line" makes sense in the HIR, but I'd (eventually perhaps) like to move towards unifying them into one vector, rather than having "items", "impl items", and "trait items" as 3 separate cases. i.e., I'd love to see a single Map<DefId, ItemLike>. To that end, perhaps we should move towards a (local) DefId as the key, instead of the newtype'd NodeId we have now? That would mean that ImplItemRef could just be an AssociatedItem (which includes a DefId) -- and perhaps eventually it would just be a DefId.

    (This raises the point that I've been wanting to make some sort of LocalDefId type -- I'd rather not use DefIndex for this purpose, since then it's unclear if this is a def-id with LOCAL_CRATE or a def-index from some other crate.)

    Niko Matsakis at 2017-03-21 09:40:18

  2. @nikomatsakis is this issue still relevant?

    Steve Klabnik at 2018-09-24 15:50:06

  3. I'd say very much so, and I've been arguing recently that impl Trait should desugar to an item not parented to a module or a block, and that we should extend HIR's notion of item past AST's.

    Eduard-Mihai Burtescu at 2018-09-24 18:03:10