Repository navigation
Describe forcing dep nodes and query key recovery - #3075
Closed
isHeSatoshi wants to merge 1 commit into
Closed
isHeSatoshi wants to merge 1 commit into
isHeSatoshi wants to merge 1 commit into
Conversation
The chapter mentions that `try_mark_green` re-runs a query from a dep node, and the query modifiers section refers to a node's key type being "recoverable", but nothing explains what recovery is or why it can fail. Explain that re-running a query from its dep node is called forcing, that it needs the query key to be reconstructible from a hash that was never meant to be reversible, and that a failure is handled by failing try_mark_green, which costs reuse rather than correctness. Then describe the fingerprint styles, which key types are recoverable and how, and why returning `None` is always permitted. The `no_force` modifier now says what it gives up and points here.
This comment was marked as low quality.
This comment was marked as low quality.
Collaborator
|
Thanks for the PR. If you have write access, feel free to merge this PR if it does not need reviews. You can request a review using |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1285.
The chapter says that
try_mark_greenre-runs a query from a dep node, and the query modifiers section saysno_forceprevents "forcing" a node "even if the query's key type is recoverable", but neither term is explained anywhere. This adds a section covering both, placed just before the modifiers section so theno_forcebullet can point at it.What the section says
TyCtxt::try_force_from_dep_node, and the pseudocode calls itrun_query_for.DepNodeis aDepKindplus a key fingerprint, which is a stable hash, and the persisted dep graph stores nodes but not the keys they were built from.try_mark_greenreturnsNone, and the root query is re-evaluated. That costs reuse, not correctness.KeyFingerprintStyleand its four values, the key types that are recoverable and how, and that returningNoneis always allowed.On the issue's specifics
The issue says recovery is "currently only possible when the query ID can be mapped to a
DefPathHash(this is the case forCrateNum,LocalDefId, andDefId)". That was written in January 2022 and is now incomplete in two ways, so the section reflects the current code rather than the issue text:OwnerId,ModIdandLocalModIdare recoverable too.(DefId, DefId)is explicitlyOpaque.HirIdhas its ownKeyFingerprintStyle::HirId, recovered by splitting the fingerprint back into aDefPathHashlocal hash and anItemLocalId. So aDefPathHashis no longer the only route back to a key.KeyFingerprintStyle::Unitcovers keys that are()or equivalent.Where the claims come from
Read out of
rust-lang/ruston master rather than recalled:compiler/rustc_middle/src/dep_graph/mod.rs: theKeyFingerprintStyleenum, its per-variant doc comments, andis_maybe_recoverable.compiler/rustc_middle/src/dep_graph/dep_node_key.rs: theDepNodeKeytrait, the blanketOpaqueimplementation, and each concretetry_recover_key, includingHirId's fingerprint split.compiler/rustc_middle/src/dep_graph/dep_node.rs: the design comment onforce_from_dep_node_fnexplaining why forcing is expected to fail from time to time and thattry_mark_greenfailing is how that is handled.compiler/rustc_middle/src/dep_graph/graph.rs:try_mark_greencallingtry_force_from_dep_nodeand returningNonewhen it fails.compiler/rustc_query_impl/src/execution.rs:force_query_dep_nodereturningfalsewhentry_recover_keyyieldsNone.Checks
#tymethod.anchors that exist on theDepNodeKeypage.RUSTC_BOOTSTRAP=1 cargo run --release --manifest-path ci/sembr/Cargo.toml srcreports the file as compliant.#forcing-dep-nodes-and-recovering-query-keysand#query-modifiers.