[ty] use Type::Divergent to avoid panic in infinitely-nested-tuple implicit attribute (#20333)

## Summary

Use `Type::Divergent` to avoid "too many iterations" panic on an
infinitely-nested tuple in an implicit instance attribute.

The regression here is from checking all tuple elements to see if they
contain a Divergent type. It's 5% on one project, 1% on another, and
zero on the rest. I spent some time looking into eliminating this
regression by tracking a flag on inference results to note if they could
possibly contain any Divergent type, but this doesn't really work --
there are too many different ways a type containing a Divergent type
could enter an inference result. Still thinking about whether there are
other ways to reduce this. One option is if we see certain kinds of
non-atomic types that are commonly expensive to check for Divergent, we
could make `has_divergent_type` a Salsa query on those types.

## Test Plan

Added mdtest.

Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

Cargo.lock

@@ -3430,7 +3430,7 @@ checksum = "28d3b2b1366ec20994f1fd18c3c594f05c5dd4bc44d8bb0c1c632c8d6829481f"
 [[package]]
 name = "salsa"
 version = "0.23.0"
-source = "git+https://github.com/salsa-rs/salsa.git?rev=a3ffa22cb26756473d56f867aedec3fd907c4dd9#a3ffa22cb26756473d56f867aedec3fd907c4dd9"
+source = "git+https://github.com/salsa-rs/salsa.git?rev=3713cd7eb30821c0c086591832dd6f59f2af7fe7#3713cd7eb30821c0c086591832dd6f59f2af7fe7"
 dependencies = [
  "boxcar",
  "compact_str",
@@ -3454,12 +3454,12 @@ dependencies = [
 [[package]]
 name = "salsa-macro-rules"
 version = "0.23.0"
-source = "git+https://github.com/salsa-rs/salsa.git?rev=a3ffa22cb26756473d56f867aedec3fd907c4dd9#a3ffa22cb26756473d56f867aedec3fd907c4dd9"
+source = "git+https://github.com/salsa-rs/salsa.git?rev=3713cd7eb30821c0c086591832dd6f59f2af7fe7#3713cd7eb30821c0c086591832dd6f59f2af7fe7"
 
 [[package]]
 name = "salsa-macros"
 version = "0.23.0"
-source = "git+https://github.com/salsa-rs/salsa.git?rev=a3ffa22cb26756473d56f867aedec3fd907c4dd9#a3ffa22cb26756473d56f867aedec3fd907c4dd9"
+source = "git+https://github.com/salsa-rs/salsa.git?rev=3713cd7eb30821c0c086591832dd6f59f2af7fe7#3713cd7eb30821c0c086591832dd6f59f2af7fe7"
 dependencies = [
  "proc-macro2",
  "quote",

Cargo.toml

@@ -143,7 +143,7 @@ regex-automata = { version = "0.4.9" }
 rustc-hash = { version = "2.0.0" }
 rustc-stable-hash = { version = "0.1.2" }
 # When updating salsa, make sure to also update the revision in `fuzz/Cargo.toml`
-salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "a3ffa22cb26756473d56f867aedec3fd907c4dd9", default-features = false, features = [
+salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "3713cd7eb30821c0c086591832dd6f59f2af7fe7", default-features = false, features = [
     "compact_str",
     "macros",
     "salsa_unstable",

crates/ty_python_semantic/resources/mdtest/attributes.md

@@ -2419,6 +2419,16 @@ reveal_type(Answer.NO.value)  # revealed: Any
 reveal_type(Answer.__members__)  # revealed: MappingProxyType[str, Unknown]
 ```
 
+## Divergent inferred implicit instance attribute types
+
+```py
+class C:
+    def f(self, other: "C"):
+        self.x = (other.x, 1)
+
+reveal_type(C().x)  # revealed: Unknown | tuple[Divergent, Literal[1]]
+```
+
 ## References
 
 Some of the tests in the *Class and instance variables* section draw inspiration from

crates/ty_python_semantic/src/types.rs

@@ -762,6 +762,10 @@ impl<'db> Type<'db> {
         Self::Dynamic(DynamicType::Unknown)
     }
 
+    pub(crate) fn divergent(scope: ScopeId<'db>) -> Self {
+        Self::Dynamic(DynamicType::Divergent(DivergentType { scope }))
+    }
+
     pub(crate) fn object(db: &'db dyn Db) -> Self {
         KnownClass::Object.to_instance(db)
     }
@@ -6516,7 +6520,6 @@ impl<'db> Type<'db> {
         }
     }
 
-    #[allow(unused)]
     pub(super) fn has_divergent_type(self, db: &'db dyn Db, div: Type<'db>) -> bool {
         any_over_type(db, self, &|ty| match ty {
             Type::Dynamic(DynamicType::Divergent(_)) => ty == div,

crates/ty_python_semantic/src/types/infer.rs

@@ -85,7 +85,7 @@ fn scope_cycle_recover<'db>(
 }
 
 fn scope_cycle_initial<'db>(_db: &'db dyn Db, scope: ScopeId<'db>) -> ScopeInference<'db> {
-    ScopeInference::cycle_fallback(scope)
+    ScopeInference::cycle_initial(scope)
 }
 
 /// Infer all types for a [`Definition`] (including sub-expressions).
@@ -123,7 +123,7 @@ fn definition_cycle_initial<'db>(
     db: &'db dyn Db,
     definition: Definition<'db>,
 ) -> DefinitionInference<'db> {
-    DefinitionInference::cycle_fallback(definition.scope(db))
+    DefinitionInference::cycle_initial(definition.scope(db))
 }
 
 /// Infer types for all deferred type expressions in a [`Definition`].
@@ -164,7 +164,7 @@ fn deferred_cycle_initial<'db>(
     db: &'db dyn Db,
     definition: Definition<'db>,
 ) -> DefinitionInference<'db> {
-    DefinitionInference::cycle_fallback(definition.scope(db))
+    DefinitionInference::cycle_initial(definition.scope(db))
 }
 
 /// Infer all types for an [`Expression`] (including sub-expressions).
@@ -192,20 +192,29 @@ pub(crate) fn infer_expression_types<'db>(
         .finish_expression()
 }
 
+/// How many fixpoint iterations to allow before falling back to Divergent type.
+const ITERATIONS_BEFORE_FALLBACK: u32 = 10;
+
 fn expression_cycle_recover<'db>(
-    _db: &'db dyn Db,
+    db: &'db dyn Db,
     _value: &ExpressionInference<'db>,
-    _count: u32,
+    count: u32,
-    _expression: Expression<'db>,
+    expression: Expression<'db>,
 ) -> salsa::CycleRecoveryAction<ExpressionInference<'db>> {
-    salsa::CycleRecoveryAction::Iterate
+    if count == ITERATIONS_BEFORE_FALLBACK {
+        salsa::CycleRecoveryAction::Fallback(ExpressionInference::cycle_fallback(
+            expression.scope(db),
+        ))
+    } else {
+        salsa::CycleRecoveryAction::Iterate
+    }
 }
 
 fn expression_cycle_initial<'db>(
     db: &'db dyn Db,
     expression: Expression<'db>,
 ) -> ExpressionInference<'db> {
-    ExpressionInference::cycle_fallback(expression.scope(db))
+    ExpressionInference::cycle_initial(expression.scope(db))
 }
 
 /// Infers the type of an `expression` that is guaranteed to be in the same file as the calling query.
@@ -324,7 +333,7 @@ fn unpack_cycle_recover<'db>(
 }
 
 fn unpack_cycle_initial<'db>(_db: &'db dyn Db, _unpack: Unpack<'db>) -> UnpackResult<'db> {
-    UnpackResult::cycle_fallback(Type::Never)
+    UnpackResult::cycle_initial(Type::Never)
 }
 
 /// Returns the type of the nearest enclosing class for the given scope.
@@ -378,6 +387,35 @@ impl<'db> InferenceRegion<'db> {
     }
 }
 
+#[derive(Debug, Clone, Copy, Eq, PartialEq, get_size2::GetSize, salsa::Update)]
+enum CycleRecovery<'db> {
+    /// An initial-value for fixpoint iteration; all types are `Type::Never`.
+    Initial,
+    /// A divergence-fallback value for fixpoint iteration; all types are `Divergent`.
+    Divergent(ScopeId<'db>),
+}
+
+impl<'db> CycleRecovery<'db> {
+    fn merge(self, other: Option<CycleRecovery<'db>>) -> Self {
+        if let Some(other) = other {
+            match (self, other) {
+                // It's important here that we keep the scope of `self` if merging two `Divergent`.
+                (Self::Divergent(scope), _) | (_, Self::Divergent(scope)) => Self::Divergent(scope),
+                _ => Self::Initial,
+            }
+        } else {
+            self
+        }
+    }
+
+    fn fallback_type(self) -> Type<'db> {
+        match self {
+            Self::Initial => Type::Never,
+            Self::Divergent(scope) => Type::divergent(scope),
+        }
+    }
+}
+
 /// The inferred types for a scope region.
 #[derive(Debug, Eq, PartialEq, salsa::Update, get_size2::GetSize)]
 pub(crate) struct ScopeInference<'db> {
@@ -385,27 +423,25 @@ pub(crate) struct ScopeInference<'db> {
     expressions: FxHashMap<ExpressionNodeKey, Type<'db>>,
 
     /// The extra data that is only present for few inference regions.
-    extra: Option<Box<ScopeInferenceExtra>>,
+    extra: Option<Box<ScopeInferenceExtra<'db>>>,
 }
 
 #[derive(Debug, Eq, PartialEq, get_size2::GetSize, salsa::Update, Default)]
-struct ScopeInferenceExtra {
+struct ScopeInferenceExtra<'db> {
-    /// The fallback type for missing expressions/bindings/declarations.
+    /// Is this a cycle-recovery inference result, and if so, what kind?
-    ///
+    cycle_recovery: Option<CycleRecovery<'db>>,
-    /// This is used only when constructing a cycle-recovery `TypeInference`.
-    cycle_fallback: bool,
 
     /// The diagnostics for this region.
     diagnostics: TypeCheckDiagnostics,
 }
 
 impl<'db> ScopeInference<'db> {
-    fn cycle_fallback(scope: ScopeId<'db>) -> Self {
+    fn cycle_initial(scope: ScopeId<'db>) -> Self {
         let _ = scope;
 
         Self {
             extra: Some(Box::new(ScopeInferenceExtra {
-                cycle_fallback: true,
+                cycle_recovery: Some(CycleRecovery::Initial),
                 ..ScopeInferenceExtra::default()
             })),
             expressions: FxHashMap::default(),
@@ -431,14 +467,10 @@ impl<'db> ScopeInference<'db> {
             .or_else(|| self.fallback_type())
     }
 
-    fn is_cycle_callback(&self) -> bool {
+    fn fallback_type(&self) -> Option<Type<'db>> {
         self.extra
             .as_ref()
-            .is_some_and(|extra| extra.cycle_fallback)
+            .and_then(|extra| extra.cycle_recovery.map(CycleRecovery::fallback_type))
-    }
-
-    fn fallback_type(&self) -> Option<Type<'db>> {
-        self.is_cycle_callback().then_some(Type::Never)
     }
 }
 
@@ -471,10 +503,8 @@ pub(crate) struct DefinitionInference<'db> {
 
 #[derive(Debug, Eq, PartialEq, get_size2::GetSize, salsa::Update, Default)]
 struct DefinitionInferenceExtra<'db> {
-    /// The fallback type for missing expressions/bindings/declarations.
+    /// Is this a cycle-recovery inference result, and if so, what kind?
-    ///
+    cycle_recovery: Option<CycleRecovery<'db>>,
-    /// This is used only when constructing a cycle-recovery `TypeInference`.
-    cycle_fallback: bool,
 
     /// The definitions that are deferred.
     deferred: Box<[Definition<'db>]>,
@@ -487,7 +517,7 @@ struct DefinitionInferenceExtra<'db> {
 }
 
 impl<'db> DefinitionInference<'db> {
-    fn cycle_fallback(scope: ScopeId<'db>) -> Self {
+    fn cycle_initial(scope: ScopeId<'db>) -> Self {
         let _ = scope;
 
         Self {
@@ -497,7 +527,7 @@ impl<'db> DefinitionInference<'db> {
             #[cfg(debug_assertions)]
             scope,
             extra: Some(Box::new(DefinitionInferenceExtra {
-                cycle_fallback: true,
+                cycle_recovery: Some(CycleRecovery::Initial),
                 ..DefinitionInferenceExtra::default()
             })),
         }
@@ -566,14 +596,10 @@ impl<'db> DefinitionInference<'db> {
         self.declarations.iter().map(|(_, qualifiers)| *qualifiers)
     }
 
-    fn is_cycle_callback(&self) -> bool {
+    fn fallback_type(&self) -> Option<Type<'db>> {
         self.extra
             .as_ref()
-            .is_some_and(|extra| extra.cycle_fallback)
+            .and_then(|extra| extra.cycle_recovery.map(CycleRecovery::fallback_type))
-    }
-
-    fn fallback_type(&self) -> Option<Type<'db>> {
-        self.is_cycle_callback().then_some(Type::Never)
     }
 
     pub(crate) fn undecorated_type(&self) -> Option<Type<'db>> {
@@ -605,21 +631,34 @@ struct ExpressionInferenceExtra<'db> {
     /// The diagnostics for this region.
     diagnostics: TypeCheckDiagnostics,
 
-    /// `true` if this region is part of a cycle-recovery `TypeInference`.
+    /// Is this a cycle recovery inference result, and if so, what kind?
-    ///
+    cycle_recovery: Option<CycleRecovery<'db>>,
-    /// Falls back to `Type::Never` if an expression is missing.
-    cycle_fallback: bool,
 
     /// `true` if all places in this expression are definitely bound
     all_definitely_bound: bool,
 }
 
 impl<'db> ExpressionInference<'db> {
+    fn cycle_initial(scope: ScopeId<'db>) -> Self {
+        let _ = scope;
+        Self {
+            extra: Some(Box::new(ExpressionInferenceExtra {
+                cycle_recovery: Some(CycleRecovery::Initial),
+                all_definitely_bound: true,
+                ..ExpressionInferenceExtra::default()
+            })),
+            expressions: FxHashMap::default(),
+
+            #[cfg(debug_assertions)]
+            scope,
+        }
+    }
+
     fn cycle_fallback(scope: ScopeId<'db>) -> Self {
         let _ = scope;
         Self {
             extra: Some(Box::new(ExpressionInferenceExtra {
-                cycle_fallback: true,
+                cycle_recovery: Some(CycleRecovery::Divergent(scope)),
                 all_definitely_bound: true,
                 ..ExpressionInferenceExtra::default()
             })),
@@ -645,14 +684,10 @@ impl<'db> ExpressionInference<'db> {
             .unwrap_or_else(Type::unknown)
     }
 
-    fn is_cycle_callback(&self) -> bool {
+    fn fallback_type(&self) -> Option<Type<'db>> {
         self.extra
             .as_ref()
-            .is_some_and(|extra| extra.cycle_fallback)
+            .and_then(|extra| extra.cycle_recovery.map(CycleRecovery::fallback_type))
-    }
-
-    fn fallback_type(&self) -> Option<Type<'db>> {
-        self.is_cycle_callback().then_some(Type::Never)
     }
 
     /// Returns true if all places in this expression are definitely bound.

crates/ty_python_semantic/src/types/infer/builder.rs

@@ -9,10 +9,10 @@ use ruff_text_size::{Ranged, TextRange};
 use rustc_hash::{FxHashMap, FxHashSet};
 
 use super::{
-    DefinitionInference, DefinitionInferenceExtra, ExpressionInference, ExpressionInferenceExtra,
+    CycleRecovery, DefinitionInference, DefinitionInferenceExtra, ExpressionInference,
-    InferenceRegion, ScopeInference, ScopeInferenceExtra, infer_deferred_types,
+    ExpressionInferenceExtra, InferenceRegion, ScopeInference, ScopeInferenceExtra,
-    infer_definition_types, infer_expression_types, infer_same_file_expression_type,
+    infer_deferred_types, infer_definition_types, infer_expression_types,
-    infer_scope_types, infer_unpack_types,
+    infer_same_file_expression_type, infer_scope_types, infer_unpack_types,
 };
 use crate::module_name::{ModuleName, ModuleNameResolutionError};
 use crate::module_resolver::{
@@ -251,10 +251,8 @@ pub(super) struct TypeInferenceBuilder<'db, 'ast> {
     /// For function definitions, the undecorated type of the function.
     undecorated_type: Option<Type<'db>>,
 
-    /// The fallback type for missing expressions/bindings/declarations.
+    /// Did we merge in a sub-region with a cycle-recovery fallback, and if so, what kind?
-    ///
+    cycle_recovery: Option<CycleRecovery<'db>>,
-    /// This is used only when constructing a cycle-recovery `TypeInference`.
-    cycle_fallback: bool,
 
     /// `true` if all places in this expression are definitely bound
     all_definitely_bound: bool,
@@ -290,11 +288,22 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             typevar_binding_context: None,
             deferred: VecSet::default(),
             undecorated_type: None,
-            cycle_fallback: false,
+            cycle_recovery: None,
             all_definitely_bound: true,
         }
     }
 
+    fn extend_cycle_recovery(&mut self, other_recovery: Option<CycleRecovery<'db>>) {
+        match &mut self.cycle_recovery {
+            Some(recovery) => *recovery = recovery.merge(other_recovery),
+            recovery @ None => *recovery = other_recovery,
+        }
+    }
+
+    fn fallback_type(&self) -> Option<Type<'db>> {
+        self.cycle_recovery.map(CycleRecovery::fallback_type)
+    }
+
     fn extend_definition(&mut self, inference: &DefinitionInference<'db>) {
         #[cfg(debug_assertions)]
         assert_eq!(self.scope, inference.scope);
@@ -307,7 +316,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         }
 
         if let Some(extra) = &inference.extra {
-            self.cycle_fallback |= extra.cycle_fallback;
+            self.extend_cycle_recovery(extra.cycle_recovery);
             self.context.extend(&extra.diagnostics);
             self.deferred.extend(extra.deferred.iter().copied());
         }
@@ -325,7 +334,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
 
         if let Some(extra) = &inference.extra {
             self.context.extend(&extra.diagnostics);
-            self.cycle_fallback |= extra.cycle_fallback;
+            self.extend_cycle_recovery(extra.cycle_recovery);
 
             if !matches!(self.region, InferenceRegion::Scope(..)) {
                 self.bindings.extend(extra.bindings.iter().copied());
@@ -399,7 +408,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         self.expressions
             .get(&expr.into())
             .copied()
-            .or(self.cycle_fallback.then_some(Type::Never))
+            .or(self.fallback_type())
     }
 
     /// Get the type of an expression from any scope in the same file.
@@ -5189,12 +5198,18 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             parenthesized: _,
         } = tuple;
 
-        // Collecting all elements is necessary to infer all sub-expressions even if some
+        let db = self.db();
-        // element types are `Never` (which leads `from_elements` to return early without
+        let divergent = Type::divergent(self.scope());
-        // consuming the whole iterator).
+        let element_types = elts.iter().map(|element| {
-        let element_types: Vec<_> = elts.iter().map(|elt| self.infer_expression(elt)).collect();
+            let element_type = self.infer_expression(element);
+            if element_type.has_divergent_type(self.db(), divergent) {
+                divergent
+            } else {
+                element_type
+            }
+        });
 
-        Type::heterogeneous_tuple(self.db(), element_types)
+        Type::heterogeneous_tuple(db, element_types)
     }
 
     fn infer_list_expression(&mut self, list: &ast::ExprList) -> Type<'db> {
@@ -8808,7 +8823,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             bindings,
             declarations,
             deferred,
-            cycle_fallback,
+            cycle_recovery,
             all_definitely_bound,
 
             // Ignored; only relevant to definition regions
@@ -8836,7 +8851,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         );
 
         let extra =
-            (cycle_fallback || !bindings.is_empty() || !diagnostics.is_empty() || !all_definitely_bound).then(|| {
+            (cycle_recovery.is_some() || !bindings.is_empty() || !diagnostics.is_empty() || !all_definitely_bound).then(|| {
                 if bindings.len() > 20 {
                     tracing::debug!(
                         "Inferred expression region `{:?}` contains {} bindings. Lookups by linear scan might be slow.",
@@ -8848,7 +8863,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
                 Box::new(ExpressionInferenceExtra {
                     bindings: bindings.into_boxed_slice(),
                     diagnostics,
-                    cycle_fallback,
+                    cycle_recovery,
                     all_definitely_bound,
                 })
             });
@@ -8873,7 +8888,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             bindings,
             declarations,
             deferred,
-            cycle_fallback,
+            cycle_recovery,
             undecorated_type,
             all_definitely_bound: _,
             // builder only state
@@ -8889,12 +8904,12 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         let diagnostics = context.finish();
 
         let extra = (!diagnostics.is_empty()
-            || cycle_fallback
+            || cycle_recovery.is_some()
             || undecorated_type.is_some()
             || !deferred.is_empty())
         .then(|| {
             Box::new(DefinitionInferenceExtra {
-                cycle_fallback,
+                cycle_recovery,
                 deferred: deferred.into_boxed_slice(),
                 diagnostics,
                 undecorated_type,
@@ -8936,7 +8951,7 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
             context,
             mut expressions,
             scope,
-            cycle_fallback,
+            cycle_recovery,
 
             // Ignored, because scope types are never extended into other scopes.
             deferred: _,
@@ -8959,9 +8974,9 @@ impl<'db, 'ast> TypeInferenceBuilder<'db, 'ast> {
         let _ = scope;
         let diagnostics = context.finish();
 
-        let extra = (!diagnostics.is_empty() || cycle_fallback).then(|| {
+        let extra = (!diagnostics.is_empty() || cycle_recovery.is_some()).then(|| {
             Box::new(ScopeInferenceExtra {
-                cycle_fallback,
+                cycle_recovery,
                 diagnostics,
             })
         });

crates/ty_python_semantic/src/types/unpacker.rs

@@ -224,7 +224,7 @@ impl<'db> UnpackResult<'db> {
         &self.diagnostics
     }
 
-    pub(crate) fn cycle_fallback(cycle_fallback_type: Type<'db>) -> Self {
+    pub(crate) fn cycle_initial(cycle_fallback_type: Type<'db>) -> Self {
         Self {
             targets: FxHashMap::default(),
             diagnostics: TypeCheckDiagnostics::default(),

fuzz/Cargo.toml

@@ -30,7 +30,7 @@ ty_python_semantic = { path = "../crates/ty_python_semantic" }
 ty_vendored = { path = "../crates/ty_vendored" }
 
 libfuzzer-sys = { git = "https://github.com/rust-fuzz/libfuzzer", default-features = false }
-salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "a3ffa22cb26756473d56f867aedec3fd907c4dd9", default-features = false, features = [
+salsa = { git = "https://github.com/salsa-rs/salsa.git", rev = "3713cd7eb30821c0c086591832dd6f59f2af7fe7", default-features = false, features = [
     "compact_str",
     "macros",
     "salsa_unstable",