Remove obsolete workaround.

[cjgillot]

The regression test for #62649 appears to pass even without the workaround.

[rust-highfive]

r? @Mark-Simulacrum

(rust-highfive has picked a reviewer for you, use r? to override)

[Mark-Simulacrum]

Hm, the comment says "one such bug"... I wonder if we have tests for all of them.

cc @rust-lang/wg-incr-comp

[spastorino]

So I guess we are also saying that #62649 is not a bug anymore? where does #62649 works? nightly, beta, stable? should we close the issue?.

Edit: I see there are attempts to fix the mentioned issue at #75641 and there were heavy changes to incr. comp. anyway not 100% sure when was this issue fixed and if it's already fixed on stable and beta.

[cjgillot]

Long explanation:

This match triggers when we are trying to force the dep-node for hir_owner or hir_owner_nodes with a non-owning def-id (both work exactly the same way).
An invariant of lowering, enforced by HIR indexing is that local_def_id_to_hir_id(id).owner is either id or its parent (there is a debug_assert).
The second branch of the workaround is already checked in the query forcing code, when attempting to reconstruct the query key from the hash.

The first branch of the is more interesting. It triggers when hir_owner is forced with a non-owning def-id. The query computes index_hir().map[id].signature. For non-owning def-ids, HIR indexing builds index_hir().map[id].signature to be Some(..) iff the id is an owner. On the converse, which is the case at hand, hir_owner returns None.

As HIR accesses are built so as never to knowingly access those Nones *. As a consequence, the compilation session which forced this node had a Some(..) value. As a consequence, the query will be red.

* There is an exception for non-exported macros, which I plan to remove because I get bitten every time.

[Mark-Simulacrum]

r? @michaelwoerister perhaps?

[michaelwoerister]

OK, this looks good to me. However, we should use this PR as an opportunity to document the invariants @cjgillot mentions, in particular it would be great to do (at least) the following:

• Add a doc string to rustc_middle::hir::IndexedHir::map that describes what the map contains. I assume that there is an entry for every DefId in the local crate - but since not every DefId will also be a HIR owner we should explain what the corresponding HirOwnerData looks like for non-owning nodes.
• Add a doc string that explains a bit more about the rustc_middle::hir::IndexedHir::parenting field. What keys are expected to be in there, for example.
• Add a doc string to the rustc_middle::hir::HirOwnerData::signature field. When is this None?
• Add a doc string to the rustc_middle::hir::HirOwnerData::with_bodies field. When is this None?

Also I have some questions:

• Why is rustc_middle::hir::HirOwnerData::with_bodies not just called by bodies? The name is a bit unexpected. Is there more to it? If so it would be great to document it.
• Is there ever a case where HirOwnerData::signature is Some but HirOwnerData::with_bodies is None? If not, can we use Option<HirOwnerData> in IndexedHir::map?
• Why is HirOwnerData::with_bodies a mut reference?

[michaelwoerister]

Thanks, @cjgillot! That looks like a nice little improvement overall.

@bors r+ rollup=never

[bors]

📌 Commit 4f8e34c has been approved by michaelwoerister

[bors]

⌛ Testing commit 4f8e34c with merge 91ddf3e...

[bors]

☀️ Test successful - checks-actions
Approved by: michaelwoerister
Pushing 91ddf3e to master...