change definitely non-productive cycles to error

Author: lcnr

Diff for: compiler/rustc_next_trait_solver/src/solve/assembly/mod.rs

@@ -740,7 +740,7 @@ where
                 // We skip the goal itself as that one would cycle.
                 let predicate: I::Predicate = trait_ref.upcast(cx);
                 ecx.add_goals(
-                    GoalSource::Misc,
+                    GoalSource::CoherenceUnknowableSuper,
                     elaborate::elaborate(cx, [predicate])
                         .skip(1)
                         .map(|predicate| goal.with(cx, predicate)),

Diff for: compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

@@ -262,13 +262,31 @@ where
         self.delegate.typing_mode()
     }
 
+    #[instrument(level = "trace", skip(self), ret)]
     pub(super) fn step_kind_for_source(&self, source: GoalSource) -> PathKind {
-        match (self.current_goal_kind, source) {
-            (
-                CurrentGoalKind::CoinductiveTrait,
-                GoalSource::ImplWhereBound | GoalSource::NormalizeImplWhereBound,
-            ) => PathKind::Coinductive,
-            _ => PathKind::Inductive,
+        match source {
+            GoalSource::NormalizeGoal(path_kind) => path_kind,
+            GoalSource::ImplWhereBound => {
+                // We currently only consider a cycle coinductive if it steps
+                // into a where-clause of a coinductive trait.
+                //
+                // We probably want to make all traits coinductive in the future,
+                // so we treat cycles involving their where-clauses as ambiguous.
+                if let CurrentGoalKind::CoinductiveTrait = self.current_goal_kind {
+                    PathKind::Coinductive
+                } else {
+                    PathKind::Unknown
+                }
+            }
+            // We treat checking super trait bounds for unknowable candidates as
+            // unknown. Treating them is inductive would cause be unsound as it causes
+            // tests/ui/traits/next-solver/coherence/ambiguity-causes-visitor-hang.rs
+            // to compile.
+            GoalSource::CoherenceUnknowableSuper => PathKind::Unknown,
+            GoalSource::Misc
+            | GoalSource::AliasBoundConstCondition
+            | GoalSource::AliasWellFormed
+            | GoalSource::InstantiateHigherRanked => PathKind::Inductive,
         }
     }
 
@@ -1120,14 +1138,11 @@ where
         for_goal_source: GoalSource,
         param_env: I::ParamEnv,
     ) -> Self {
-        let normalization_goal_source = match for_goal_source {
-            GoalSource::ImplWhereBound => GoalSource::NormalizeImplWhereBound,
-            _ => GoalSource::Misc,
-        };
+        let step_kind = ecx.step_kind_for_source(for_goal_source);
         ReplaceAliasWithInfer {
             ecx,
             param_env,
-            normalization_goal_source,
+            normalization_goal_source: GoalSource::NormalizeGoal(step_kind),
             cache: Default::default(),
         }
     }

Diff for: compiler/rustc_next_trait_solver/src/solve/search_graph.rs

@@ -3,7 +3,7 @@ use std::marker::PhantomData;
 
 use rustc_type_ir::Interner;
 use rustc_type_ir::search_graph::{self, PathKind};
-use rustc_type_ir::solve::{CanonicalInput, Certainty, QueryResult};
+use rustc_type_ir::solve::{CanonicalInput, Certainty, NoSolution, QueryResult};
 
 use super::inspect::ProofTreeBuilder;
 use super::{FIXPOINT_STEP_LIMIT, has_no_inference_or_external_constraints};
@@ -47,7 +47,8 @@ where
     ) -> QueryResult<I> {
         match kind {
             PathKind::Coinductive => response_no_constraints(cx, input, Certainty::Yes),
-            PathKind::Inductive => response_no_constraints(cx, input, Certainty::overflow(false)),
+            PathKind::Unknown => response_no_constraints(cx, input, Certainty::overflow(false)),
+            PathKind::Inductive => Err(NoSolution),
         }
     }
 
@@ -57,12 +58,7 @@ where
         input: CanonicalInput<I>,
         result: QueryResult<I>,
     ) -> bool {
-        match kind {
-            PathKind::Coinductive => response_no_constraints(cx, input, Certainty::Yes) == result,
-            PathKind::Inductive => {
-                response_no_constraints(cx, input, Certainty::overflow(false)) == result
-            }
-        }
+        Self::initial_provisional_result(cx, kind, input) == result
     }
 
     fn on_stack_overflow(

Diff for: compiler/rustc_trait_selection/src/solve/fulfill/derive_errors.rs

@@ -438,9 +438,13 @@ impl<'tcx> ProofTreeVisitor<'tcx> for BestObligation<'tcx> {
 
             let obligation;
             match (child_mode, nested_goal.source()) {
+                // We shouldn't really care about coherence unknowable candidates
+                // as we never report fulfillment errors involving them. However, this
+                // is still reachable when building the fulfillment errors.
+                (_, GoalSource::CoherenceUnknowableSuper) => continue,
                 (
                     ChildMode::Trait(_) | ChildMode::Host(_),
-                    GoalSource::Misc | GoalSource::NormalizeImplWhereBound,
+                    GoalSource::Misc | GoalSource::NormalizeGoal(_),
                 ) => {
                     continue;
                 }

Diff for: compiler/rustc_type_ir/src/search_graph/mod.rs

@@ -13,12 +13,15 @@
 /// behavior as long as the resulting behavior is still correct.
 use std::cmp::Ordering;
 use std::collections::BTreeMap;
+use std::collections::hash_map::Entry;
 use std::fmt::Debug;
 use std::hash::Hash;
 use std::marker::PhantomData;
 
 use derive_where::derive_where;
 use rustc_index::{Idx, IndexVec};
+#[cfg(feature = "nightly")]
+use rustc_macros::{HashStable_NoContext, TyDecodable, TyEncodable};
 use tracing::debug;
 
 use crate::data_structures::HashMap;
@@ -109,20 +112,29 @@ pub trait Delegate {
 /// In the initial iteration of a cycle, we do not yet have a provisional
 /// result. In the case we return an initial provisional result depending
 /// on the kind of cycle.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
+#[cfg_attr(feature = "nightly", derive(TyDecodable, TyEncodable, HashStable_NoContext))]
 pub enum PathKind {
-    Coinductive,
+    /// A path consisting of only inductive/unproductive steps.
     Inductive,
+    /// A path which is not be coinductive right now but we may want
+    /// to change of them to be so in the future. We return an ambiguous
+    /// result in this case to prevent people from relying on this.
+    Unknown,
+    /// A path with at least one coinductive step. Such cycles hold.
+    Coinductive,
 }
+
 impl PathKind {
     /// Returns the path kind when merging `self` with `rest`.
     ///
     /// Given an inductive path `self` and a coinductive path `rest`,
     /// the path `self -> rest` would be coinductive.
     fn extend(self, rest: PathKind) -> PathKind {
-        match self {
-            PathKind::Coinductive => PathKind::Coinductive,
-            PathKind::Inductive => rest,
+        match (self, rest) {
+            (PathKind::Coinductive, _) | (_, PathKind::Coinductive) => PathKind::Coinductive,
+            (PathKind::Unknown, _) | (_, PathKind::Unknown) => PathKind::Unknown,
+            (PathKind::Inductive, PathKind::Inductive) => PathKind::Inductive,
         }
     }
 }
@@ -156,9 +168,6 @@ impl UsageKind {
             }
         }
     }
-    fn and_merge(&mut self, other: impl Into<Self>) {
-        *self = self.merge(other);
-    }
 }
 
 /// For each goal we track whether the paths from this goal
@@ -294,14 +303,68 @@ impl CycleHeads {
 
             let path_from_entry = match step_kind {
                 PathKind::Coinductive => AllPathsToHeadCoinductive::Yes,
-                PathKind::Inductive => path_from_entry,
+                PathKind::Unknown | PathKind::Inductive => path_from_entry,
             };
 
             self.insert(head, path_from_entry);
         }
     }
 }
 
+bitflags::bitflags! {
+    /// Tracks how nested goals have been accessed. This is necessary to disable
+    /// global cache entries if computing them would otherwise result in a cycle or
+    /// access a provisional cache entry.
+    #[derive(Debug, Clone, Copy)]
+    pub struct PathsToNested: u8 {
+        /// The initial value when adding a goal to its own nested goals.
+        const EMPTY                      = 1 << 0;
+        const INDUCTIVE                  = 1 << 1;
+        const UNKNOWN                    = 1 << 2;
+        const COINDUCTIVE                = 1 << 3;
+    }
+}
+impl From<PathKind> for PathsToNested {
+    fn from(path: PathKind) -> PathsToNested {
+        match path {
+            PathKind::Inductive => PathsToNested::INDUCTIVE,
+            PathKind::Unknown => PathsToNested::UNKNOWN,
+            PathKind::Coinductive => PathsToNested::COINDUCTIVE,
+        }
+    }
+}
+impl PathsToNested {
+    #[must_use]
+    fn extend_with(mut self, path: PathKind) -> Self {
+        match path {
+            PathKind::Inductive => {
+                if self.intersects(PathsToNested::EMPTY) {
+                    self.remove(PathsToNested::EMPTY);
+                    self.insert(PathsToNested::INDUCTIVE);
+                }
+            }
+            PathKind::Unknown => {
+                if self.intersects(PathsToNested::EMPTY | PathsToNested::INDUCTIVE) {
+                    self.remove(PathsToNested::EMPTY | PathsToNested::INDUCTIVE);
+                    self.insert(PathsToNested::UNKNOWN);
+                }
+            }
+            PathKind::Coinductive => {
+                if self.intersects(
+                    PathsToNested::EMPTY | PathsToNested::INDUCTIVE | PathsToNested::UNKNOWN,
+                ) {
+                    self.remove(
+                        PathsToNested::EMPTY | PathsToNested::INDUCTIVE | PathsToNested::UNKNOWN,
+                    );
+                    self.insert(PathsToNested::COINDUCTIVE);
+                }
+            }
+        }
+
+        self
+    }
+}
+
 /// The nested goals of each stack entry and the path from the
 /// stack entry to that nested goal.
 ///
@@ -319,15 +382,18 @@ impl CycleHeads {
 /// results from a the cycle BAB depending on the cycle root.
 #[derive_where(Debug, Default, Clone; X: Cx)]
 struct NestedGoals<X: Cx> {
-    nested_goals: HashMap<X::Input, UsageKind>,
+    nested_goals: HashMap<X::Input, PathsToNested>,
 }
 impl<X: Cx> NestedGoals<X> {
     fn is_empty(&self) -> bool {
         self.nested_goals.is_empty()
     }
 
-    fn insert(&mut self, input: X::Input, path_from_entry: UsageKind) {
-        self.nested_goals.entry(input).or_insert(path_from_entry).and_merge(path_from_entry);
+    fn insert(&mut self, input: X::Input, paths_to_nested: PathsToNested) {
+        match self.nested_goals.entry(input) {
+            Entry::Occupied(mut entry) => *entry.get_mut() |= paths_to_nested,
+            Entry::Vacant(entry) => drop(entry.insert(paths_to_nested)),
+        }
     }
 
     /// Adds the nested goals of a nested goal, given that the path `step_kind` from this goal
@@ -338,18 +404,15 @@ impl<X: Cx> NestedGoals<X> {
     /// the same as for the child.
     fn extend_from_child(&mut self, step_kind: PathKind, nested_goals: &NestedGoals<X>) {
         #[allow(rustc::potential_query_instability)]
-        for (input, path_from_entry) in nested_goals.iter() {
-            let path_from_entry = match step_kind {
-                PathKind::Coinductive => UsageKind::Single(PathKind::Coinductive),
-                PathKind::Inductive => path_from_entry,
-            };
-            self.insert(input, path_from_entry);
+        for (input, paths_to_nested) in nested_goals.iter() {
+            let paths_to_nested = paths_to_nested.extend_with(step_kind);
+            self.insert(input, paths_to_nested);
         }
     }
 
     #[cfg_attr(feature = "nightly", rustc_lint_query_instability)]
     #[allow(rustc::potential_query_instability)]
-    fn iter(&self) -> impl Iterator<Item = (X::Input, UsageKind)> + '_ {
+    fn iter(&self) -> impl Iterator<Item = (X::Input, PathsToNested)> + '_ {
         self.nested_goals.iter().map(|(i, p)| (*i, *p))
     }
 
@@ -487,7 +550,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
             // goals as this change may cause them to now depend on additional
             // goals, resulting in new cycles. See the dev-guide for examples.
             if parent_depends_on_cycle {
-                parent.nested_goals.insert(parent.input, UsageKind::Single(PathKind::Inductive))
+                parent.nested_goals.insert(parent.input, PathsToNested::EMPTY);
             }
         }
     }
@@ -663,7 +726,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
             //
             // We must therefore not use the global cache entry for `B` in that case.
             // See tests/ui/traits/next-solver/cycles/hidden-by-overflow.rs
-            last.nested_goals.insert(last.input, UsageKind::Single(PathKind::Inductive));
+            last.nested_goals.insert(last.input, PathsToNested::EMPTY);
         }
 
         debug!("encountered stack overflow");
@@ -745,16 +808,11 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
 
                 // We now care about the path from the next highest cycle head to the
                 // provisional cache entry.
-                match path_from_head {
-                    PathKind::Coinductive => {}
-                    PathKind::Inductive => {
-                        *path_from_head = Self::cycle_path_kind(
-                            &self.stack,
-                            stack_entry.step_kind_from_parent,
-                            head,
-                        )
-                    }
-                }
+                *path_from_head = path_from_head.extend(Self::cycle_path_kind(
+                    &self.stack,
+                    stack_entry.step_kind_from_parent,
+                    head,
+                ));
                 // Mutate the result of the provisional cache entry in case we did
                 // not reach a fixpoint.
                 *result = mutate_result(input, *result);
@@ -854,7 +912,7 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
             for &ProvisionalCacheEntry {
                 encountered_overflow,
                 ref heads,
-                path_from_head,
+                path_from_head: head_to_provisional,
                 result: _,
             } in entries.iter()
             {
@@ -866,24 +924,19 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
 
                 // A provisional cache entry only applies if the path from its highest head
                 // matches the path when encountering the goal.
+                //
+                // We check if any of the paths taken while computing the global goal
+                // would end up with an applicable provisional cache entry.
                 let head = heads.highest_cycle_head();
-                let full_path = match Self::cycle_path_kind(stack, step_kind_from_parent, head) {
-                    PathKind::Coinductive => UsageKind::Single(PathKind::Coinductive),
-                    PathKind::Inductive => path_from_global_entry,
-                };
-
-                match (full_path, path_from_head) {
-                    (UsageKind::Mixed, _)
-                    | (UsageKind::Single(PathKind::Coinductive), PathKind::Coinductive)
-                    | (UsageKind::Single(PathKind::Inductive), PathKind::Inductive) => {
-                        debug!(
-                            ?full_path,
-                            ?path_from_head,
-                            "cache entry not applicable due to matching paths"
-                        );
-                        return false;
-                    }
-                    _ => debug!(?full_path, ?path_from_head, "paths don't match"),
+                let head_to_curr = Self::cycle_path_kind(stack, step_kind_from_parent, head);
+                let full_paths = path_from_global_entry.extend_with(head_to_curr);
+                if full_paths.contains(head_to_provisional.into()) {
+                    debug!(
+                        ?full_paths,
+                        ?head_to_provisional,
+                        "cache entry not applicable due to matching paths"
+                    );
+                    return false;
                 }
             }
         }
@@ -982,8 +1035,8 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
         let last = &mut self.stack[last_index];
         last.reached_depth = last.reached_depth.max(next_index);
 
-        last.nested_goals.insert(input, UsageKind::Single(step_kind_from_parent));
-        last.nested_goals.insert(last.input, UsageKind::Single(PathKind::Inductive));
+        last.nested_goals.insert(input, step_kind_from_parent.into());
+        last.nested_goals.insert(last.input, PathsToNested::EMPTY);
         if last_index != head {
             last.heads.insert(head, step_kind_from_parent);
         }