Limit obligation processing depth#153338
Conversation
Related to rust-lang#152857 It's possible for `ObligationForest::process_cycles` to miss more complex cycles and this can lead to an endless fixpoint loop since it never reaches a settled state. It's desirable to adjust `find_cycles_from_node` to catch the cycle in rust-lang#152857. At the same time the nature of the code risks an endless loop by some other unexpected or unhandled cycle, so this PR aims to provide a basic limit to the depth of the loop. The value of the constant was derived to be ~20x the highest value seen when compiling rustc itself. The top values are: ``` ┌───────┐ │ 10852 │ │ 10852 │ │ 10433 │ │ 10433 │ │ 10433 │ │ 10433 │ │ 8498 │ │ 8498 │ │ 6361 │ │ 6361 │ └───────┘ ```
rustbot
added
S-waiting-on-review
|
r? @fee1-dead rustbot has assigned @fee1-dead. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| // It's possible for nodes to recursively add obligations in an endless cycle, | ||
| // for example https://github.com/rust-lang/rust/issues/152857. | ||
| self.error_at(index); | ||
| return outcome; |
There was a problem hiding this comment.
Currently this creates an ICE, which strictly speaking is still preferable to an endless loop but I'd prefer to issue a better error something similar to the other overflow errors, but not sure what's the best way to do that here.
|
@rustbot reroll |
|
Does it add new obligations at each step in the cycle? If so, we should make sure that we increase the |
|
r? lcnr |
|
@rustbot author |
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
Do you mean #152857 ? If so I'm not sure I fully understand how that suggestion relates to this proposed change. As described in the PR "At the same time the nature of the code risks an endless loop by some other unexpected or unhandled cycle, so this PR aims to provide a basic limit to the depth of the loop." |
|
The reason this keeps on looping is that we set
|
6 participants
Related to #152857
It's possible for
ObligationForest::process_cyclesto miss more complex cycles and this can lead to an endless fixpoint loop since it never reaches a settled state. It's desirable to adjustfind_cycles_from_nodeto catch the cycle in#152857. At the same time the nature of the code risks an endless loop by some other unexpected or unhandled cycle, so this PR aims to provide a basic limit to the depth of the loop.
The value of the constant was derived to be ~20x the highest value seen when compiling rustc itself. The top 10 values out of ~70 million invocations are: