-
Notifications
You must be signed in to change notification settings - Fork 182
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
coinduction semantics in the recursive solver #399
Comments
@rustbot claim |
So I described in high-level terms the way I think we should implement this. It builds on the way that the recursive solver handles cycles now. How inductive solver worksThe basic structure for handling cycles in the recursive solver is "iterate until we reach a fixed point". For inductive cycles, I think the mental model is that we are "discovering more and more answers". Therefore we have a kind of lattice that works like this (values begin at the first entry and move towards the final one):
We start out with the result of error (i.e., no answers) and then we try to solve the goal via all the "routes" available to us (i.e., applying each possible program clause). Each clause might have back an answer in the form above. If for example we have two clauses, and we get back Error from one and If multiple clauses give back answers, though, things can be a bit more complicated. If the answers are "incompatible", then the result is just ambiguous: so if our two clauses gave back There are also cases where neither is more general than the other. For example, consider two answers like But sometimes you have cases where there are two answers, but one of them is an "instantiation" of the other. The easiest to see is that one answer might be Change the starting value for coinductive goalsSo to make this work for coinductive goals like (say) The actual process remains unchanged otherwise: we start with a different starting value, but then we look for solutions from the various clauses and combine them in the same way, in order to find the most general solution. Then we check whether we've reached a fixed point and, if not, we iterate again. (If we ever reach an ambiguous result, we can stop immediately, and indeed we must do so for various obscure reasons.) Mixed inductive/co-inductive cyclesOne weird case to be careful of are mixed inductive/co-inductive cycles. I think the correct handling there is that if we see a cycle that terminates on a co-inductive goal, but there are inductive goals on the stack, then this is always an error, regardless of what the saved value is. This means for example that if you have something like impl<X: Debug> Send for Foo<X> { }
impl<X> Debug for Foo<X> where Foo<X>: Send { } then we would get an error. This is because we get a stack like
I would like to write a better argument for what's going on here, but for now I'll leave it at this. (Question: should we always fail on a 'mixed stack' regardless of which is the 'head goal'?) Edits in the code that are neededI think we need to do two things. First, we need to remove this logic: chalk/chalk-solve/src/recursive.rs Lines 152 to 159 in 1781c43
That logic combines the "old saved answer" with the "new answer we found from the clauses". But this isn't really what we want to do. We actually just want to save the new answer if it's different and then iterate, we don't need to combine with the result from a previous iteration. On a quick test locally, I found that removing this logic had no ill-effects. Second, we need to change the starting value for a given goal depending on whether it is coinductive or not. That starting value is set automatically when the search graph node is created here: chalk/chalk-solve/src/recursive.rs Line 245 in 1781c43
Third, we would I think want to change the |
I started exploring this in https://github.com/nikomatsakis/chalk-ndm/tree/recursive-coinductive -- just the first step I mentioned above. Could be a fine starting point for someone else. |
Perfect. I will pick it up then ! Thanks a lot for the help! |
…tives, r=nikomatsakis Coinduction handling for recursive solver This PR is meant to address the issue of invalid result in coinductive cycles as described in #399 . It also fixes the two coinduction tests related to that issue (i.e. coinductive_unsound1 and coinductive_multicycle4). As such it is an improved version of #683 as it uses "solution0" described [here](https://hackmd.io/2nm3xPJ1TTGc2r4iiWi4Lg) and discussed [here](https://zulip-archive.rust-lang.org/144729wgtraits/71232coinduction.html) to handle more complicated coinductive cycles.
The recursive solver introduced by @flodiebold in #351 isn't handling coinduction correctly. A number of the tests have "FIXME" results. (See comment.)
This issue has been assigned to @zaharidichev via this comment.
The text was updated successfully, but these errors were encountered: