Trait selection provisional cache incorrectly handles DepNodes, leading to ICE #92987
Assignees
Labels
C-bug
Category: This is a bug.
I-ICE
Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️
T-compiler
Relevant to the compiler team, which will review and decide on the PR/issue.
Comments
znx3p0
added
C-bug
|
This is a new incremental compilation issue, distinct from the previous 'called Option::unwrap' issues. cc @cjgillot |
|
Thanks for the great reproducer - I was able to reproduce this by first building commit |
|
This is caused by the way that we handle The current query system is set up so that we will never try to force a query with a stale If a query 'A' obtains a When we actually invoke a query during the execution of some query's body, we will try to find a matching However, anon dep nodes break this assumption. Specifically, when we create a Inside a (recursive) call to As a result, trying to mark a The fact that we will never legitimately try to force a stale I'll need to think about how we can adjust anon dep nodes to fix this issue. Ideally, we would just remove the concept of anon dep nodes entirely - however, I'm pretty sure that it's the only way to make the global evaluation cache work with incremental compilation. |
|
I've determined the root cause of the issue - we're failing to keep track of the created rust/compiler/rustc_trait_selection/src/traits/select/mod.rs Lines 765 to 766 in ee5d8d3 When we later read from the evaluation cache, we will generate a read of that rust/compiler/rustc_query_system/src/cache.rs Lines 49 to 52 in ee5d8d3 This ensures that any queries using a cached result will still get all of the dependency edges created during the initial uncached execution (e.g. a call to However, when we store a result in the provisional cache (due to an auto-trait cycle), we don't store the generated rust/compiler/rustc_trait_selection/src/traits/select/mod.rs Lines 776 to 785 in ee5d8d3 When we later move entires from the provisional cache to the 'real' evaluation cache, we insert them using a different rust/compiler/rustc_trait_selection/src/traits/select/mod.rs Lines 768 to 775 in ee5d8d3 In the provided reproducer, we first store I've confirmed that storing the provisional result into the evaluation cache using its original |
Aaron1011
changed the title
Option::unwrap() on a None value', /rustc/5531927e8af9b99ad923af4c827c91038bca51ee/compiler/rustc_hir/src/definitions.rs:452:14
|
Minimal reproduction script:
#[cfg(first)]
struct CycleOne(Box<CycleTwo>);
#[cfg(second)]
enum CycleOne {
Variant(Box<CycleTwo>)
}
struct CycleTwo(CycleOne);
fn assert_send<T: Send>() {}
fn bar() {
assert_send::<CycleOne>();
assert_send::<CycleTwo>();
}
set -euxo pipefail
rm -r foo
rustc repro.rs --crate-type lib -C incremental=foo --cfg first
rustc repro.rs --crate-type lib -C incremental=foo --cfg secondproduces: |
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jan 25, 2022
…ichaelwoerister Properly track `DepNode`s in trait evaluation provisional cache Fixes rust-lang#92987 During evaluation of an auto trait predicate, we may encounter a cycle. This causes us to store the evaluation result in a special 'provisional cache;. If we later end up determining that the type can legitimately implement the auto trait despite the cycle, we remove the entry from the provisional cache, and insert it into the evaluation cache. Additionally, trait evaluation creates a special anonymous `DepNode`. All queries invoked during the predicate evaluation are added as outoging dependency edges from the `DepNode`. This `DepNode` is then store in the evaluation cache - if a different query ends up reading from the cache entry, it will also perform a read of the stored `DepNode`. As a result, the cached evaluation will still end up (transitively) incurring all of the same dependencies that it would if it actually performed the uncached evaluation (e.g. a call to `type_of` to determine constituent types). Previously, we did not correctly handle the interaction between the provisional cache and the created `DepNode`. Storing an evaluation result in the provisional cache would cause us to lose the `DepNode` created during the evaluation. If we later moved the entry from the provisional cache to the evaluation cache, we would use the `DepNode` associated with the evaluation that caused us to 'complete' the cycle, not the evaluatoon where we first discovered the cycle. As a result, future reads from the evaluation cache would miss some incremental compilation dependencies that would have otherwise been added if the evaluation was *not* cached. Under the right circumstances, this could lead to us trying to force a query with a no-longer-existing `DefPathHash`, since we were missing the (red) dependency edge that would have caused us to bail out before attempting forcing. This commit makes the provisional cache store the `DepNode` create during the provisional evaluation. When we move an entry from the provisional cache to the evaluation cache, we create a *new* `DepNode` that has dependencies going to *both* of the evaluation `DepNodes` we have available. This ensures that cached reads will incur all of the necessary dependency edges.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this issue
Jan 25, 2022
…ichaelwoerister Properly track `DepNode`s in trait evaluation provisional cache Fixes rust-lang#92987 During evaluation of an auto trait predicate, we may encounter a cycle. This causes us to store the evaluation result in a special 'provisional cache;. If we later end up determining that the type can legitimately implement the auto trait despite the cycle, we remove the entry from the provisional cache, and insert it into the evaluation cache. Additionally, trait evaluation creates a special anonymous `DepNode`. All queries invoked during the predicate evaluation are added as outoging dependency edges from the `DepNode`. This `DepNode` is then store in the evaluation cache - if a different query ends up reading from the cache entry, it will also perform a read of the stored `DepNode`. As a result, the cached evaluation will still end up (transitively) incurring all of the same dependencies that it would if it actually performed the uncached evaluation (e.g. a call to `type_of` to determine constituent types). Previously, we did not correctly handle the interaction between the provisional cache and the created `DepNode`. Storing an evaluation result in the provisional cache would cause us to lose the `DepNode` created during the evaluation. If we later moved the entry from the provisional cache to the evaluation cache, we would use the `DepNode` associated with the evaluation that caused us to 'complete' the cycle, not the evaluatoon where we first discovered the cycle. As a result, future reads from the evaluation cache would miss some incremental compilation dependencies that would have otherwise been added if the evaluation was *not* cached. Under the right circumstances, this could lead to us trying to force a query with a no-longer-existing `DefPathHash`, since we were missing the (red) dependency edge that would have caused us to bail out before attempting forcing. This commit makes the provisional cache store the `DepNode` create during the provisional evaluation. When we move an entry from the provisional cache to the evaluation cache, we create a *new* `DepNode` that has dependencies going to *both* of the evaluation `DepNodes` we have available. This ensures that cached reads will incur all of the necessary dependency edges.
This was referenced Feb 26, 2023
Closed
This was referenced Feb 26, 2023
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Code
Please clone this repo and build it.
Meta
rustc --version --verbose:Error output
Backtrace
The text was updated successfully, but these errors were encountered: