Auto merge of #698 - firefighterduck:coinductive-recursive-false-posi…

…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.

book/src/glossary.md

@@ -229,3 +229,15 @@ valuation `A = true, B = false` makes the premise true and the conclusion false.
 ## Valuation
 A valuation is an assignment of values to all variables inside a logical
 formula.
+
+## Fixed-Points
+A fixed-point of a function `f` is a value `x` for which `f(x)=x`.
+Similarly a pre-fixed-point is defined as `x ≤ f(x)`, whereas for a post-fixed-point it holds that `f(x) ≤ x`.
+
+A least fixed-point (lfp) of `f` is the fixed-point `x` of `f` for which all other fixed-points `y` are greater or equal (i.e. if `f(y)=y` then `x ≤ y`).
+Similarly, a greatest fixed-point (gfp) is greater or equal than all other fixed-points.
+If `f` is a function on sets, the least fixed-point is defined as the intersection of all pre-fixed-points, which are then defined as sets `x` for which `x ⊆ f(x)`.
+The greatest fixed-point is in this case the union of all post-fixed-points, respectively.
+
+This simple definition of lfp and gfp can also be lifted to general lattices.
+The results for Chalk goals form such a lattice and, thus, every solver for such goals tries to find such fixed-points.

book/src/recursive/coinduction.md

@@ -1,3 +1,98 @@
 # Coinduction
 
-TBD
+This sub-chapter is meant to describe the current handling of coinductive goals in the recursive solver rather than providing an extensive overview over the theoretical backgrounds and ideas.
+It follows the description in [this GitHub comment](https://github.com/rust-lang/chalk/issues/399#issuecomment-643420016) and the Zulip topic linked there.
+In general, coinductive cycles can arise for well-formedness checking and autotraits.
+Therefore, correctly handling coinductive cycles is necessary to model the Rust trait system in its entirety.
+
+## General Idea
+Coinductive cycles can be handled the same way as inductive cycles described [before](./inductive_cycles.md).
+The only difference is the start value for coinductive goals.
+Whereas inductive goals start with a negative result and are iterated until a least fixed-point is found, coinductive goals start with a positive result (i.e. a unique solution with identity substitution).
+This negative result is then iterated until a greatest fixed-point is reached.
+
+## Mixed co-inductive and inductive Cycles
+As described above, the handling of inductive and coindutive cycles differs only in the start value from which the computation begins.
+Thus, it might seem reasonable to have mixed inductive and coinductive cycles as all goals inside these cycles would be handled the same way anyway.
+Unfortunately, this is not possible for the kind of logic that Chalk is based on (i.e. essentially an extension of co-LP for Hereditary Harrop clauses, cf. [this paper][co-LP]).
+
+There is fundamental difference between results for inductive cycles and results for coinductive cycles of goals.
+An inductive goal is provable if and only if there exists a proof for it consisting of a finite chain of derivations from axioms that are members of the least-fixed point of the underlying logic program.
+On the other hand, coinductive goals are provable if there exists an at most infinite derivation starting from the axioms that proves it (this includes in particular all finite derivations).
+This infinite derivation is then part of the greatest fixed-point of the logic program.
+As infinite derivations are not feasible to compute, it is enough to show that such a derivation contains no contradiction.
+
+A simple example `X :- X.` (with `X` a free variable) is thus not provable by inductive reasoning (the least solution/lfp for this is the empty solution, a failure) but it is provable by coinductive reasoning (the greatest solution/gfp is the universe, i.e. all values).
+
+This difference between inductive and coinductive results becomes a problem when combined in a single cycle.
+Consider a coinductive goal `CG` and an inductive goal `IG`. Now consider the simplest possible mixed cycle:
+```notrust
+CG :- IG
+IG :- CG
+```
+It is apparent, that there can not exist a solution for `IG` as the cyclic dependency prevents a finite proof derivation.
+In contrast to that, `CG` could potentially be provable as the derivation *`CG` if `IG` if `CG` if `IG` ...* is infinite and based only on the two axioms.
+As a result, `CG` would hold whereas `IG` would not hold, creating a contradiction.
+
+The simplest solution to this problem, proposed by Simon et al. in [their paper about co-LP][co-LP], is to disallow mixed inductive and coinductive cycles.
+This approach is also used by Chalk.
+
+## Prevention of Invalid Results
+The problem of invalid results propagated outside of the coinductive cycle is also described in the [Coinduction chapter](../engine/logic/coinduction.md) for the SLG solver alongside the rather complex handling used with it.
+Whereas the SLG solver introduces [special constructs](../engine/logic/coinduction.html#nikos-proposed-solution) to handle coinduction, it is sufficient for the recursive solver to use the same logic for inductive and coinductive cycles.
+The following is a description of how this works in more detail.
+
+### The Problem
+The problem arises if a solution that is purely based on the positive starting value for the coinductive cycle is cached (or tabled in logic programming terms) and as such propagated to other goals that are possibly reliant on this. An example where all clause goals are assumedly coinductive may look like this (cf. the test case `coinduction::coinductive_unsound1`):
+
+```notrust
+C :- C1.
+C :- C2.
+C1 :- C2, C3.
+C2 :- C1.
+```
+The following is a computation to find out whether there exists a type that implements `C`.
+Here the implementation of `C` may be proved by either showing that the type implements `C1` or `C2`.
+* Start proving `C` by trying to prove `C1`:
+    * For `C1` try to prove `C2` and `C3`:
+        * Start with `C2`. For `C2` we need to prove `C1`:
+            * This is a (coinductive) cycle. Assume that `C1` holds, i.e. use the positive start value.
+        * Based on this `C2` also holds. If this case is not handled specifically, the solution for `C2` is cached without a reference to the solution for `C1` on which it depends.
+        * Now try to prove `C3`:
+            * Find that there is no way do so from the given axioms.
+        * Thus, there exists no solution for `C3` and the computation fails. This valid result is cached and lifted back up.
+    * Due to the failure of `C3` there is also no solution for `C1`. This failure is also cached correctly and lifted back up. The cached solution for `C2` has now become invalid as it depends on a positive result for `C1`.
+* As a result of the failure for `C1`, `C` can not be proved from `C1`. Try proving `C` from `C2` instead:
+    * Find the cached result that `C2` has a solution and lift it back up.
+* Due to the solution for `C2`, `C` is also proved with the same solution.
+* Stop with this positive but invalid result for `C`.
+
+### The Solution
+The above example should make it evident that the caching of found solutions in coinductive cycles can lead to invalid results and should therefore be prevented.
+This can be achieved by delaying the caching of all results inside the coinductive cycle until it is clear whether the start of the cycle (i.e. `C1` in the example above) is provable (cf. the handling of inductive cycles [before](./inductive_cycles.md)).
+If the start of the cycle can be proven by the results of the cycle and related subgoals then the assumption about it was correct and thus all results for goals inside the cycle are also valid.
+If, however, the start of the cycle can not be proved, i.e. the initial assumption was false, then a subset of the found solutions for the coinductive cycle may be invalid (i.e. the solution for `C2` in the example).
+
+To remove such invalid results, the cycle is restarted with a negative result for the cycle start.
+With this approach, it is possible to remove all invalid result that would otherwise depend on the disproved cycle assumption.
+To allow for the cycle to be restarted correctly, all nodes in the search graph after the cycle start are deleted.
+
+With this procedure, the example is handled as follows:
+* Start proving `C` with `C1`:
+    * For `C1` prove `C2` and `C3`:
+        * For `C2` prove `C1`:
+            * This is a coinductive cycle. Assume that `C1` holds.
+        * Thus `C2` also holds. Delay the caching of the result about `C2`.
+        * There is no way to prove `C3`. Cache this result and lift the failure up.
+    * Due to the failure of `C3` there is also no solution for `C1`. Set `C1` to a negative result and restart the cycle.
+        * For `C2` prove `C1`:
+            * `C1` has now a negative result.
+        * Thus, `C2` also has a negative result which is not yet cached.
+        * Find the already cached negative result for `C3`.
+    * Nothing changed regarding `C1` (this would indicate a negative cycle which is currently  not allowed) and the negative result for `C1` and `C2` are cached. Lift the negative result for `C1` back up.
+* Start proving `C` with `C2`:
+    * Find negative cached result for `C2`. Lift the result back up.
+* Neither `C1` nor `C2` have a positive result. Stop with the valid disproof of `C`.
+
+
+[co-LP]: https://link.springer.com/chapter/10.1007%2F978-3-540-73420-8_42

book/src/todo.md

@@ -4,4 +4,3 @@ Some topics yet to be written:
 
 - Elaborate on the proof procedure
 - SLG solving – introduce negative reasoning
-- Recursive solver coinduction chapter

chalk-recursive/src/combine.rs

@@ -2,33 +2,7 @@ use chalk_solve::Solution;
 use tracing::debug;
 
 use chalk_ir::interner::Interner;
-use chalk_ir::{ClausePriority, DomainGoal, Fallible, GenericArg, Goal, GoalData};
-
-pub(crate) fn with_priorities_for_goal<I: Interner>(
-    interner: &I,
-    goal: &Goal<I>,
-    a: Fallible<Solution<I>>,
-    prio_a: ClausePriority,
-    b: Fallible<Solution<I>>,
-    prio_b: ClausePriority,
-) -> (Fallible<Solution<I>>, ClausePriority) {
-    let domain_goal = match goal.data(interner) {
-        GoalData::DomainGoal(domain_goal) => domain_goal,
-        _ => {
-            // non-domain goals currently have no priorities, so we always take the new solution here
-            return (b, prio_b);
-        }
-    };
-    match (a, b) {
-        (Ok(a), Ok(b)) => {
-            let (solution, prio) = with_priorities(interner, domain_goal, a, prio_a, b, prio_b);
-            (Ok(solution), prio)
-        }
-        (Ok(solution), Err(_)) => (Ok(solution), prio_a),
-        (Err(_), Ok(solution)) => (Ok(solution), prio_b),
-        (Err(_), Err(e)) => (Err(e), prio_b),
-    }
-}
+use chalk_ir::{ClausePriority, DomainGoal, GenericArg};
 
 pub(super) fn with_priorities<I: Interner>(
     interner: &I,

chalk-recursive/src/recursive.rs

@@ -2,10 +2,10 @@ use crate::search_graph::DepthFirstNumber;
 use crate::search_graph::SearchGraph;
 use crate::solve::{SolveDatabase, SolveIteration};
 use crate::stack::{Stack, StackDepth};
-use crate::{combine, Minimums, UCanonicalGoal};
-use chalk_ir::interner::Interner;
+use crate::{Minimums, UCanonicalGoal};
 use chalk_ir::Fallible;
-use chalk_ir::{Canonical, ConstrainedSubst, Constraints, Goal, InEnvironment, UCanonical};
+use chalk_ir::{interner::Interner, NoSolution};
+use chalk_ir::{Canonical, ConstrainedSubst, Goal, InEnvironment, UCanonical};
 use chalk_solve::{coinductive_goal::IsCoinductive, RustIrDatabase, Solution};
 use rustc_hash::FxHashMap;
 use std::fmt;
@@ -163,18 +163,6 @@ impl<'me, I: Interner> Solver<'me, I> {
                 return *minimums;
             }
 
-            let old_answer = &self.context.search_graph[dfn].solution;
-            let old_prio = self.context.search_graph[dfn].solution_priority;
-
-            let (current_answer, current_prio) = combine::with_priorities_for_goal(
-                self.program.interner(),
-                &canonical_goal.canonical.value.goal,
-                old_answer.clone(),
-                old_prio,
-                current_answer,
-                current_prio,
-            );
-
             // Some of our subgoals depended on us. We need to re-run
             // with the current answer.
             if self.context.search_graph[dfn].solution == current_answer {
@@ -223,24 +211,17 @@ impl<'me, I: Interner> SolveDatabase<I> for Solver<'me, I> {
         if let Some(dfn) = self.context.search_graph.lookup(&goal) {
             // Check if this table is still on the stack.
             if let Some(depth) = self.context.search_graph[dfn].stack_depth {
-                // Is this a coinductive goal? If so, that is success,
-                // so we can return normally. Note that this return is
-                // not tabled.
-                //
-                // XXX how does caching with coinduction work?
-                if self.context.stack.coinductive_cycle_from(depth) {
-                    let value = ConstrainedSubst {
-                        subst: goal.trivial_substitution(self.program.interner()),
-                        constraints: Constraints::empty(self.program.interner()),
-                    };
-                    debug!("applying coinductive semantics");
-                    return Ok(Solution::Unique(Canonical {
-                        value,
-                        binders: goal.canonical.binders,
-                    }));
-                }
-
                 self.context.stack[depth].flag_cycle();
+                // Mixed cycles are not allowed. For more information about this
+                // see the corresponding section in the coinduction chapter:
+                // https://rust-lang.github.io/chalk/book/recursive/coinduction.html#mixed-co-inductive-and-inductive-cycles
+                if self
+                    .context
+                    .stack
+                    .mixed_inductive_coinductive_cycle_from(depth)
+                {
+                    return Err(NoSolution);
+                }
             }
 
             minimums.update_from(self.context.search_graph[dfn].links);
@@ -255,10 +236,15 @@ impl<'me, I: Interner> SolveDatabase<I> for Solver<'me, I> {
             previous_solution
         } else {
             // Otherwise, push the goal onto the stack and create a table.
-            // The initial result for this table is error.
+            // The initial result for this table depends on whether the goal is coinductive.
             let coinductive_goal = goal.is_coinductive(self.program);
             let depth = self.context.stack.push(coinductive_goal);
-            let dfn = self.context.search_graph.insert(&goal, depth);
+            let dfn = self.context.search_graph.insert(
+                &goal,
+                depth,
+                coinductive_goal,
+                self.program.interner(),
+            );
             let subgoal_minimums = self.solve_new_subgoal(goal, depth, dfn);
             self.context.search_graph[dfn].links = subgoal_minimums;
             self.context.search_graph[dfn].stack_depth = None;

chalk-recursive/src/search_graph.rs

@@ -5,7 +5,10 @@ use std::usize;
 
 use super::stack::StackDepth;
 use crate::{Minimums, UCanonicalGoal};
-use chalk_ir::{interner::Interner, ClausePriority, Fallible, NoSolution};
+use chalk_ir::{
+    interner::Interner, Canonical, ClausePriority, ConstrainedSubst, Constraints, Fallible,
+    NoSolution,
+};
 use chalk_solve::Solution;
 use rustc_hash::FxHashMap;
 use tracing::{debug, instrument};
@@ -56,18 +59,32 @@ impl<I: Interner> SearchGraph<I> {
     ///
     /// - stack depth as given
     /// - links set to its own DFN
-    /// - solution is initially `NoSolution`
+    /// - solution is initially an identity substitution for coinductive goals
+    ///   or `NoSolution` for other goals
     pub(crate) fn insert(
         &mut self,
         goal: &UCanonicalGoal<I>,
         stack_depth: StackDepth,
+        coinductive: bool,
+        interner: &I,
     ) -> DepthFirstNumber {
         let dfn = DepthFirstNumber {
             index: self.nodes.len(),
         };
+        let solution = if coinductive {
+            Ok(Solution::Unique(Canonical {
+                value: ConstrainedSubst {
+                    subst: goal.trivial_substitution(interner),
+                    constraints: Constraints::empty(interner),
+                },
+                binders: goal.canonical.binders.clone(),
+            }))
+        } else {
+            Err(NoSolution)
+        };
         let node = Node {
             goal: goal.clone(),
-            solution: Err(NoSolution),
+            solution,
             solution_priority: ClausePriority::High,
             stack_depth: Some(stack_depth),
             links: Minimums { positive: dfn },

chalk-recursive/src/stack.rs

@@ -67,12 +67,18 @@ impl Stack {
         self.entries.pop();
     }
 
-    /// True if all the goals from the top of the stack down to (and
-    /// including) the given depth are coinductive.
-    pub(crate) fn coinductive_cycle_from(&self, depth: StackDepth) -> bool {
-        self.entries[depth.depth..]
+    /// True iff there exist at least one coinductive goal
+    /// and one inductive goal each from the top of the stack
+    /// down to (and including) the given depth.
+    pub(crate) fn mixed_inductive_coinductive_cycle_from(&self, depth: StackDepth) -> bool {
+        let coinductive_count = self.entries[depth.depth..]
             .iter()
-            .all(|entry| entry.coinductive_goal)
+            .filter(|entry| entry.coinductive_goal)
+            .count();
+        let total_count = self.entries.len() - depth.depth;
+        let any_coinductive = coinductive_count != 0;
+        let any_inductive = coinductive_count != total_count;
+        any_coinductive && any_inductive
     }
 }