fix: bugs generating unintended circular references

crates/erg_compiler/context/compare.rs

@@ -1093,6 +1093,11 @@ impl Context {
 
     /// Returns union of two types (`A or B`).
     /// If `A` and `B` have a subtype relationship, it is equal to `max(A, B)`.
+    /// ```erg
+    /// union(Nat, Int) == Int
+    /// union(Int, Str) == Int or Str
+    /// union(?T(<: Str), ?U(<: Int)) == ?T or ?U
+    /// ```
     pub(crate) fn union(&self, lhs: &Type, rhs: &Type) -> Type {
         if lhs == rhs {
             return lhs.clone();

crates/erg_compiler/context/eval.rs

@@ -1242,7 +1242,7 @@ impl Context {
             let (sub, sup) = fv.get_subsup().unwrap();
             if self.is_trait(&sup) && !self.trait_impl_exists(&sub, &sup) {
                 // link to `Never` to prevent double errors from being reported
-                fv.link(&Never);
+                lhs.link(&Never);
                 let sub = if cfg!(feature = "debug") {
                     sub
                 } else {
@@ -1682,8 +1682,8 @@ impl Context {
                 }
             }
         }
-        if let TyParam::FreeVar(lhs) = &lhs {
+        if let TyParam::FreeVar(fv) = &lhs {
-            if let Some((sub, sup)) = lhs.get_subsup() {
+            if let Some((sub, sup)) = fv.get_subsup() {
                 if self.is_trait(&sup) && !self.trait_impl_exists(&sub, &sup) {
                     // to prevent double error reporting
                     lhs.link(&TyParam::t(Never));
@@ -1781,8 +1781,8 @@ impl Context {
                             err
                         })
                         .ok()?;
-                    for (param, arg) in instance.typarams().into_iter().zip(args.into_iter()) {
+                    for (param, arg) in instance.typarams().iter().zip(args.iter()) {
-                        self.sub_unify_tp(&arg, &param, None, &(), false).ok()?;
+                        self.sub_unify_tp(arg, param, None, &(), false).ok()?;
                     }
                     let ty_obj = if self.is_class(&instance) {
                         ValueObj::builtin_class(instance)

crates/erg_compiler/context/generalize.rs

@@ -147,8 +147,9 @@ impl Generalizer {
                     // |Int <: T <: Int| T -> T ==> Int -> Int
                     if sub == sup {
                         let t = self.generalize_t(sub, uninit);
-                        fv.forced_link(&t);
+                        let res = FreeVar(fv);
-                        FreeVar(fv)
+                        res.forced_link(&t);
+                        res
                     } else if sup != Obj
                         && !self.qnames.contains(&fv.unbound_name().unwrap())
                         && self.variance == Contravariant
@@ -546,7 +547,7 @@ impl<'c, 'q, 'l, L: Locational> Dereferencer<'c, 'q, 'l, L> {
                             Ok(ty)
                         }
                         Err(errs) => {
-                            fv.link(&Never);
+                            Type::FreeVar(fv).link(&Never);
                             Err(errs)
                         }
                     }

crates/erg_compiler/context/inquire.rs

@@ -1295,7 +1295,7 @@ impl Context {
                     let non_default_params = pos_args.iter().map(|a| anon(a.expr.t())).collect();
                     let subr_t = subr_t(kind, non_default_params, None, vec![], ret_t);
                     self.occur(&subr_t, instance, obj)?;
-                    fv.link(&subr_t);
+                    instance.link(&subr_t);
                     Ok(SubstituteResult::Ok)
                 }
             }

crates/erg_compiler/context/instantiate.rs

@@ -140,8 +140,8 @@ impl TyVarCache {
         } else if let Some(inst) = self.typaram_instances.get(name) {
             if let Ok(inst) = <&Type>::try_from(inst) {
                 self.update_tyvar(inst, tv, ctx);
-            } else if let TyParam::FreeVar(fv) = inst {
+            } else if let TyParam::FreeVar(_fv) = inst {
-                fv.link(&TyParam::t(tv.clone()));
+                inst.link(&TyParam::t(tv.clone()));
             } else {
                 unreachable!()
             }
@@ -155,8 +155,8 @@ impl TyVarCache {
         // T<inst> is uninitialized
         // T<inst>.link(T<tv>);
         // T <: Eq(T <: Eq(T <: ...))
-        let inst = enum_unwrap!(inst, Type::FreeVar);
+        let free_inst = enum_unwrap!(inst, Type::FreeVar);
-        if inst.constraint_is_uninited() {
+        if free_inst.constraint_is_uninited() {
             inst.link(tv);
         } else {
             // inst: ?T(<: Int) => old_sub: Never, old_sup: Int
@@ -165,14 +165,14 @@ impl TyVarCache {
             // inst: ?T(:> Str)
             // tv: ?T(:> Nat)
             // => ?T(:> Nat or Str)
-            let (old_sub, old_sup) = inst.get_subsup().unwrap();
+            let (old_sub, old_sup) = free_inst.get_subsup().unwrap();
             let tv = enum_unwrap!(tv, Type::FreeVar);
             let (new_sub, new_sup) = tv.get_subsup().unwrap();
             let new_constraint = Constraint::new_sandwiched(
                 ctx.union(&old_sub, &new_sub),
                 ctx.intersection(&old_sup, &new_sup),
             );
-            inst.update_constraint(new_constraint, true);
+            free_inst.update_constraint(new_constraint, true);
         }
     }
 
@@ -192,15 +192,15 @@ impl TyVarCache {
     }
 
     fn update_typaram(&self, inst: &TyParam, tp: &TyParam, ctx: &Context) {
-        let inst = enum_unwrap!(inst, TyParam::FreeVar);
+        let free_inst = enum_unwrap!(inst, TyParam::FreeVar);
-        if inst.constraint_is_uninited() {
+        if free_inst.constraint_is_uninited() {
             inst.link(tp);
         } else {
-            let old_type = inst.get_type().unwrap();
+            let old_type = free_inst.get_type().unwrap();
             let tv = enum_unwrap!(tp, TyParam::FreeVar);
             let new_type = tv.get_type().unwrap();
             let new_constraint = Constraint::new_type_of(ctx.intersection(&old_type, &new_type));
-            inst.update_constraint(new_constraint, true);
+            free_inst.update_constraint(new_constraint, true);
         }
     }
 

crates/erg_compiler/context/register.rs

@@ -946,8 +946,8 @@ impl Context {
         let Some((_, vi)) = self.get_var_info(ident.inspect()) else {
             return Ok(());
         };
-        if let Some(fv) = vi.t.as_free() {
+        if let Some(_fv) = vi.t.as_free() {
-            fv.link(&typ);
+            vi.t.link(&typ);
         }
         res.map(|_| ())
     }

crates/erg_compiler/context/unify.rs

@@ -280,10 +280,10 @@ impl Context {
             {
                 if sub_fv.level().unwrap() > sup_fv.level().unwrap() {
                     if !sub_fv.is_generalized() {
-                        sub_fv.link(maybe_sup);
+                        maybe_sub.link(maybe_sup);
                     }
                 } else if !sup_fv.is_generalized() {
-                    sup_fv.link(maybe_sub);
+                    maybe_sup.link(maybe_sub);
                 }
                 Ok(())
             }
@@ -307,7 +307,7 @@ impl Context {
                             sub_fv.update_constraint(new_constraint, false);
                         }
                     } else {
-                        sub_fv.link(sup_tp);
+                        maybe_sub.link(sup_tp);
                     }
                     Ok(())
                 } else if allow_divergence
@@ -315,7 +315,7 @@ impl Context {
                         || self.eq_tp(sup_tp, &TyParam::value(NegInf)))
                     && self.subtype_of(&fv_t, &mono("Num"))
                 {
-                    sub_fv.link(sup_tp);
+                    maybe_sub.link(sup_tp);
                     Ok(())
                 } else {
                     Err(TyCheckErrors::from(TyCheckError::unreachable(
@@ -345,7 +345,7 @@ impl Context {
                             sup_fv.update_constraint(new_constraint, false);
                         }
                     } else {
-                        sup_fv.link(sub_tp);
+                        maybe_sup.link(sub_tp);
                     }
                     Ok(())
                 } else if allow_divergence
@@ -353,7 +353,7 @@ impl Context {
                         || self.eq_tp(sub_tp, &TyParam::value(NegInf)))
                     && self.subtype_of(&fv_t, &mono("Num"))
                 {
-                    sup_fv.link(sub_tp);
+                    maybe_sup.link(sub_tp);
                     Ok(())
                 } else {
                     Err(TyCheckErrors::from(TyCheckError::unreachable(
@@ -738,21 +738,21 @@ impl Context {
                         .cmp(&sup_fv.level().unwrap_or(GENERIC_LEVEL))
                     {
                         std::cmp::Ordering::Less => {
-                            sub_fv.link(&union);
+                            maybe_sub.link(&union);
-                            sup_fv.link(maybe_sub);
+                            maybe_sup.link(maybe_sub);
                         }
                         std::cmp::Ordering::Greater => {
-                            sup_fv.link(&union);
+                            maybe_sup.link(&union);
-                            sub_fv.link(maybe_sup);
+                            maybe_sub.link(maybe_sup);
                         }
                         std::cmp::Ordering::Equal => {
                             // choose named one
                             if sup_fv.is_named_unbound() {
-                                sup_fv.link(&union);
+                                maybe_sup.link(&union);
-                                sub_fv.link(maybe_sup);
+                                maybe_sub.link(maybe_sup);
                             } else {
-                                sub_fv.link(&union);
+                                maybe_sub.link(&union);
-                                sup_fv.link(maybe_sub);
+                                maybe_sup.link(maybe_sub);
                             }
                         }
                     }
@@ -765,20 +765,20 @@ impl Context {
                     {
                         std::cmp::Ordering::Less => {
                             sub_fv.update_constraint(new_constraint, false);
-                            sup_fv.link(maybe_sub);
+                            maybe_sup.link(maybe_sub);
                         }
                         std::cmp::Ordering::Greater => {
                             sup_fv.update_constraint(new_constraint, false);
-                            sub_fv.link(maybe_sup);
+                            maybe_sub.link(maybe_sup);
                         }
                         std::cmp::Ordering::Equal => {
                             // choose named one
                             if sup_fv.is_named_unbound() {
                                 sup_fv.update_constraint(new_constraint, false);
-                                sub_fv.link(maybe_sup);
+                                maybe_sub.link(maybe_sup);
                             } else {
                                 sub_fv.update_constraint(new_constraint, false);
-                                sup_fv.link(maybe_sub);
+                                maybe_sup.link(maybe_sub);
                             }
                         }
                     }
@@ -874,7 +874,7 @@ impl Context {
                         }
                     }
                     if sup.contains_union(&new_sub) {
-                        sup_fv.link(&new_sub); // Bool <: ?T <: Bool or Y ==> ?T == Bool
+                        maybe_sup.link(&new_sub); // Bool <: ?T <: Bool or Y ==> ?T == Bool
                     } else {
                         let constr = Constraint::new_sandwiched(new_sub, mem::take(&mut sup));
                         sup_fv.update_constraint(constr, true);
@@ -943,7 +943,7 @@ impl Context {
                         && !new_sup.is_unbound_var()
                         && !sub.is_unbound_var()
                     {
-                        sub_fv.link(&sub);
+                        maybe_sub.link(&sub);
                     } else {
                         let constr = Constraint::new_sandwiched(sub, new_sup);
                         sub_fv.update_constraint(constr, true);
@@ -1057,8 +1057,10 @@ impl Context {
                         todo!("{maybe_sub} <: {maybe_sup}")
                     }
                 }
-                // covariant
+                if !sub_subr.return_t.is_generalized() {
-                self.sub_unify(&sub_subr.return_t, &sup_subr.return_t, loc, param_name)?;
+                    // covariant
+                    self.sub_unify(&sub_subr.return_t, &sup_subr.return_t, loc, param_name)?;
+                }
                 Ok(())
             }
             (Subr(sub_subr), Quantified(sup_subr)) => {
@@ -1090,8 +1092,10 @@ impl Context {
                         todo!("{maybe_sub} <: {maybe_sup}")
                     }
                 }
-                // covariant
+                if !sup_subr.return_t.is_generalized() {
-                self.sub_unify(&sub_subr.return_t, &sup_subr.return_t, loc, param_name)?;
+                    // covariant
+                    self.sub_unify(&sub_subr.return_t, &sup_subr.return_t, loc, param_name)?;
+                }
                 Ok(())
             }
             (

crates/erg_compiler/ty/free.rs

@@ -461,6 +461,14 @@ impl<T> FreeKind<T> {
         }
     }
 
+    /// SAFETY: carefully ensure that `to` is not a freevar equal to `self`
+    ///
+    /// e.g.
+    /// ```erg
+    /// x = ?T
+    /// x.replace(Type::Free(?T))
+    /// x == (((...)))
+    /// ```
     pub fn replace(&mut self, to: T) {
         match self {
             // REVIEW: What if `t` is also an unbound variable?
@@ -800,10 +808,16 @@ impl<T> Free<T> {
             }
         }
     }
+
+    pub fn addr_eq(&self, other: &Self) -> bool {
+        self.as_ptr() == other.as_ptr()
+    }
 }
 
 impl<T: Clone> Free<T> {
-    pub fn link(&self, to: &T) {
+    /// SAFETY: use `Type/TyParam::link` instead of this.
+    /// This method may cause circular references.
+    pub(super) fn link(&self, to: &T) {
         // prevent linking to self
         if self.is_linked() && addr_eq!(*self.crack(), *to) {
             return;
@@ -814,7 +828,7 @@ impl<T: Clone> Free<T> {
     /// NOTE: Do not use this except to rewrite circular references.
     /// No reference to any type variable may be left behind when rewriting.
     /// However, `get_subsup` is safe because it does not return references.
-    pub fn forced_link(&self, to: &T) {
+    pub(super) fn forced_link(&self, to: &T) {
         // prevent linking to self
         if self.is_linked() && addr_eq!(*self.crack(), *to) {
             return;

crates/erg_compiler/ty/mod.rs

@@ -2306,7 +2306,7 @@ impl Type {
             Type::FreeVar(fv) if fv.is_unbound() => {
                 let (sub, _sup) = fv.get_subsup().unwrap();
                 sub.coerce();
-                fv.link(&sub);
+                self.link(&sub);
             }
             Type::And(l, r) | Type::Or(l, r) => {
                 l.coerce();
@@ -2955,6 +2955,35 @@ impl Type {
             self.clone()
         }
     }
+
+    fn addr_eq(&self, other: &Type) -> bool {
+        match (self, other) {
+            (Self::FreeVar(slf), Self::FreeVar(otr)) => slf.addr_eq(otr),
+            _ => self == other,
+        }
+    }
+
+    pub(crate) fn link(&self, to: &Type) {
+        if self.addr_eq(to) {
+            return;
+        }
+        match self {
+            Self::FreeVar(fv) => fv.link(to),
+            Self::Refinement(refine) => refine.t.link(to),
+            _ => panic!("{self} is not a free variable"),
+        }
+    }
+
+    pub(crate) fn forced_link(&self, to: &Type) {
+        if self.addr_eq(to) {
+            return;
+        }
+        match self {
+            Self::FreeVar(fv) => fv.forced_link(to),
+            Self::Refinement(refine) => refine.t.forced_link(to),
+            _ => panic!("{self} is not a free variable"),
+        }
+    }
 }
 
 pub struct ReplaceTable<'t> {

crates/erg_compiler/ty/typaram.rs

@@ -4,11 +4,10 @@ use std::ops::{Add, Div, Mul, Neg, Range, RangeInclusive, Sub};
 use std::sync::Arc;
 
 use erg_common::dict::Dict;
-use erg_common::set;
 use erg_common::set::Set;
 use erg_common::traits::{LimitedDisplay, StructuralEq};
 use erg_common::Str;
-use erg_common::{dict, log};
+use erg_common::{dict, log, set};
 
 use erg_parser::ast::ConstLambda;
 
@@ -1116,6 +1115,23 @@ impl TyParam {
             other => other,
         }
     }
+
+    fn addr_eq(&self, other: &TyParam) -> bool {
+        match (self, other) {
+            (Self::FreeVar(slf), Self::FreeVar(otr)) => slf.addr_eq(otr),
+            _ => self == other,
+        }
+    }
+
+    pub(crate) fn link(&self, to: &TyParam) {
+        if self.addr_eq(to) {
+            return;
+        }
+        match self {
+            Self::FreeVar(fv) => fv.link(to),
+            _ => panic!("{self} is not a free variable"),
+        }
+    }
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]