Skip to content

Commit

Permalink
fix: undo leak bug & sub-unification bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
mtshiba committed Aug 22, 2023
1 parent 97afccb commit 3724a74
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 117 deletions.
12 changes: 12 additions & 0 deletions crates/erg_common/triple.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,22 @@
use std::fmt;

#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
pub enum Triple<T, E> {
Ok(T),
Err(E),
None,
}

impl<T: fmt::Display, E: fmt::Display> fmt::Display for Triple<T, E> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
Triple::Ok(ok) => write!(f, "Ok({ok})"),
Triple::Err(err) => write!(f, "Err({err})"),
Triple::None => write!(f, "None"),
}
}
}

impl<T, E> Triple<T, E> {
pub fn none_then(self, err: E) -> Result<T, E> {
match self {
Expand Down
115 changes: 59 additions & 56 deletions crates/erg_compiler/context/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use erg_common::error::Location;
#[allow(unused)]
use erg_common::log;
use erg_common::set::Set;
use erg_common::shared::Shared;
use erg_common::traits::{Locational, Stream};
use erg_common::{dict, fmt_vec, fn_name, option_enum_unwrap, set, Triple};
use erg_common::{ArcArray, Str};
Expand All @@ -22,7 +23,7 @@ use crate::ty::constructors::{
array_t, dict_t, mono, named_free_var, poly, proj, proj_call, ref_, ref_mut, refinement, set_t,
subr_t, tp_enum, tuple_t, v_enum,
};
use crate::ty::free::{Constraint, FreeTyVar, HasLevel};
use crate::ty::free::{Constraint, HasLevel};
use crate::ty::typaram::{OpKind, TyParam};
use crate::ty::value::{GenTypeObj, TypeObj, ValueObj};
use crate::ty::{ConstSubr, HasType, Predicate, SubrKind, Type, UserConstSubr, ValueArgs};
Expand Down Expand Up @@ -92,27 +93,52 @@ fn op_to_name(op: OpKind) -> &'static str {
}
}

#[derive(Debug)]
pub struct Substituter<'c> {
ctx: &'c Context,
qt: Type,
#[allow(unused)]
st: Type,
child: Option<Box<Substituter<'c>>>,
#[derive(Debug, Default)]
pub struct UndoableLinkedList {
tys: Shared<Set<Type>>,
tps: Shared<Set<TyParam>>,
}

impl Drop for Substituter<'_> {
impl Drop for UndoableLinkedList {
fn drop(&mut self) {
Self::undo_substitute_typarams(&self.qt);
for t in self.tys.borrow().iter() {
t.undo();
}
for tp in self.tps.borrow().iter() {
tp.undo();
}
}
}

impl UndoableLinkedList {
pub fn new() -> Self {
Self {
tys: Shared::new(set! {}),
tps: Shared::new(set! {}),
}
}

pub fn push_t(&self, t: &Type) {
self.tys.borrow_mut().insert(t.clone());
}

pub fn push_tp(&self, tp: &TyParam) {
self.tps.borrow_mut().insert(tp.clone());
}
}

#[derive(Debug)]
pub struct Substituter<'c> {
ctx: &'c Context,
undoable_linked: UndoableLinkedList,
child: Option<Box<Substituter<'c>>>,
}

impl<'c> Substituter<'c> {
fn new(ctx: &'c Context, qt: Type, st: Type) -> Self {
fn new(ctx: &'c Context) -> Self {
Self {
ctx,
qt,
st,
undoable_linked: UndoableLinkedList::new(),
child: None,
}
}
Expand Down Expand Up @@ -142,7 +168,7 @@ impl<'c> Substituter<'c> {
log!(err "[{}] [{}]", erg_common::fmt_vec(&qtps), erg_common::fmt_vec(&stps));
return Ok(None); // TODO: e.g. Sub(Int) / Eq and Sub(?T)
}
let mut self_ = Self::new(ctx, qt.clone(), st.clone());
let mut self_ = Self::new(ctx);
let mut errs = EvalErrors::empty();
for (qtp, stp) in qtps.into_iter().zip(stps.into_iter()) {
if let Err(err) = self_.substitute_typaram(qtp, stp) {
Expand Down Expand Up @@ -171,7 +197,7 @@ impl<'c> Substituter<'c> {
log!(err "[{}] [{}]", erg_common::fmt_vec(&qtps), erg_common::fmt_vec(&stps));
return Ok(None); // TODO: e.g. Sub(Int) / Eq and Sub(?T)
}
let mut self_ = Self::new(ctx, qt.clone(), st.clone());
let mut self_ = Self::new(ctx);
let mut errs = EvalErrors::empty();
for (qtp, stp) in qtps.into_iter().zip(stps.into_iter()) {
if let Err(err) = self_.overwrite_typaram(qtp, stp) {
Expand All @@ -188,7 +214,7 @@ impl<'c> Substituter<'c> {
fn substitute_typaram(&mut self, qtp: TyParam, stp: TyParam) -> EvalResult<()> {
match qtp {
TyParam::FreeVar(ref fv) if fv.is_generalized() => {
qtp.undoable_link(&stp);
qtp.undoable_link(&stp, &self.undoable_linked);
/*if let Err(errs) = self.sub_unify_tp(&stp, &qtp, None, &(), false) {
log!(err "{errs}");
}*/
Expand Down Expand Up @@ -226,13 +252,13 @@ impl<'c> Substituter<'c> {
)
})?;
if !qt.is_undoable_linked_var() && qt.is_generalized() && qt.is_free_var() {
qt.undoable_link(&st);
qt.undoable_link(&st, &self.undoable_linked);
} else if qt.is_undoable_linked_var() && qt != st {
// e.g. Array(T, N) <: Add(Array(T, M))
// Array((Int), (3)) <: Add(Array((Int), (4))): OK
// Array((Int), (3)) <: Add(Array((Str), (4))): NG
if let Some(union) = self.ctx.unify(&qt, &st) {
qt.undoable_link(&union);
qt.undoable_link(&union, &self.undoable_linked);
} else {
return Err(EvalError::unification_error(
self.ctx.cfg.input.clone(),
Expand All @@ -257,7 +283,10 @@ impl<'c> Substituter<'c> {
} else {
qt
};
if let Err(errs) = self.ctx.undoable_sub_unify(&st, &qt, &(), None) {
if let Err(errs) = self
.ctx
.undoable_sub_unify(&st, &qt, &(), &self.undoable_linked, None)
{
log!(err "{errs}");
}
Ok(())
Expand All @@ -266,7 +295,7 @@ impl<'c> Substituter<'c> {
fn overwrite_typaram(&mut self, qtp: TyParam, stp: TyParam) -> EvalResult<()> {
match qtp {
TyParam::FreeVar(ref fv) if fv.is_undoable_linked() => {
qtp.undoable_link(&stp);
qtp.undoable_link(&stp, &self.undoable_linked);
/*if let Err(errs) = self.sub_unify_tp(&stp, &qtp, None, &(), false) {
log!(err "{errs}");
}*/
Expand All @@ -276,7 +305,7 @@ impl<'c> Substituter<'c> {
// Whether this could be a problem is under consideration.
// e.g. `T` of Array(T, N) <: Add(T, M)
TyParam::FreeVar(ref fv) if fv.is_generalized() => {
qtp.undoable_link(&stp);
qtp.undoable_link(&stp, &self.undoable_linked);
/*if let Err(errs) = self.sub_unify_tp(&stp, &qtp, None, &(), false) {
log!(err "{errs}");
}*/
Expand Down Expand Up @@ -314,7 +343,7 @@ impl<'c> Substituter<'c> {
)
})?;
if qt.is_undoable_linked_var() {
qt.undoable_link(&st);
qt.undoable_link(&st, &self.undoable_linked);
}
if !st.is_unbound_var() || !st.is_generalized() {
self.child = Self::overwrite_typarams(self.ctx, &qt, &st)?.map(Box::new);
Expand All @@ -325,42 +354,15 @@ impl<'c> Substituter<'c> {
} else {
qt
};
if let Err(errs) = self.ctx.undoable_sub_unify(&st, &qt, &(), None) {
if let Err(errs) = self
.ctx
.undoable_sub_unify(&st, &qt, &(), &self.undoable_linked, None)
{
log!(err "{errs}");
}
Ok(())
}

fn undo_substitute_typarams(substituted_q: &Type) {
for tp in substituted_q.typarams().into_iter() {
Self::undo_substitute_typaram(tp);
}
}

fn undo_substitute_typaram(substituted_q_tp: TyParam) {
match substituted_q_tp {
TyParam::FreeVar(fv) if fv.is_undoable_linked() => fv.undo(),
TyParam::Type(t) if t.is_free_var() => {
let Ok(subst) = <&FreeTyVar>::try_from(t.as_ref()) else { unreachable!() };
if subst.is_undoable_linked() {
subst.undo();
}
}
/*TyParam::Type(t) => {
Self::undo_substitute_typarams(&t);
}
TyParam::Value(ValueObj::Type(t)) => {
Self::undo_substitute_typarams(t.typ());
}
TyParam::App { args, .. } => {
for arg in args.into_iter() {
Self::undo_substitute_typaram(arg);
}
}*/
_ => {}
}
}

/// ```erg
/// substitute_self(Iterable('Self), Int)
/// -> Iterable(Int)
Expand All @@ -372,8 +374,9 @@ impl<'c> Substituter<'c> {
&& t.get_super()
.is_some_and(|sup| ctx.supertype_of(&sup, subtype))
{
t.undoable_link(subtype);
return Some(Self::new(ctx, qt.clone(), subtype.clone()));
let mut _self = Self::new(ctx);
t.undoable_link(subtype, &_self.undoable_linked);
return Some(_self);
}
}
None
Expand Down Expand Up @@ -2173,7 +2176,7 @@ impl Context {
let proj = proj_call(coerced, attr_name, args);
self.eval_t_params(proj, level, t_loc)
.map(|t| {
lhs.coerce();
lhs.destructive_coerce();
t
})
.map_err(|(_, errs)| errs)
Expand Down
20 changes: 14 additions & 6 deletions crates/erg_compiler/context/generalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use crate::{feature_error, hir};
use Type::*;
use Variance::*;

use super::eval::Substituter;
use super::eval::{Substituter, UndoableLinkedList};

pub struct Generalizer {
level: usize,
Expand Down Expand Up @@ -557,21 +557,29 @@ impl<'c, 'q, 'l, L: Locational> Dereferencer<'c, 'q, 'l, L> {
// we need to force linking to avoid infinite loop
// e.g. fv == ?T(<: Int, :> Add(?T))
// fv == ?T(:> ?T.Output, <: Add(Int))
let list = UndoableLinkedList::new();
let fv_t = Type::FreeVar(fv.clone());
match (sub_t.contains_type(&fv_t), super_t.contains_type(&fv_t)) {
let dummy = match (sub_t.contains_type(&fv_t), super_t.contains_type(&fv_t)) {
// REVIEW: to prevent infinite recursion, but this may cause a nonsense error
(true, true) => {
fv.dummy_link();
true
}
(true, false) => {
fv_t.undoable_link(&super_t);
fv_t.undoable_link(&super_t, &list);
false
}
(false, true | false) => {
fv_t.undoable_link(&sub_t);
fv_t.undoable_link(&sub_t, &list);
false
}
}
};
let res = self.validate_subsup(sub_t, super_t);
fv.undo();
if dummy {
fv.undo();
} else {
drop(list);
}
match res {
Ok(ty) => {
// TODO: T(:> Nat <: Int) -> T(:> Nat, <: Int) ==> Int -> Nat
Expand Down
9 changes: 6 additions & 3 deletions crates/erg_compiler/context/initialize/const_func.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ use std::mem;
use erg_common::dict::Dict;
#[allow(unused_imports)]
use erg_common::log;
use erg_common::{dict, Str};
use erg_common::{enum_unwrap, set};
use erg_common::{dict, enum_unwrap, set};

use crate::context::Context;
use crate::feature_error;
Expand Down Expand Up @@ -482,12 +481,16 @@ pub(crate) fn __named_tuple_getitem__(
}

/// `NamedTuple({ .x = Int; .y = Str }).union() == Int or Str`
/// `GenericNamedTuple.union() == Obj`
pub(crate) fn named_tuple_union(mut args: ValueArgs, ctx: &Context) -> EvalValueResult<TyParam> {
let slf = args
.remove_left_or_key("Self")
.ok_or_else(|| not_passed("Self"))?;
let fields = match ctx.convert_value_into_type(slf) {
Ok(Type::NamedTuple(fields)) => fields,
Ok(Type::Mono(n)) if &n == "GenericNamedTuple" => {
return Ok(ValueObj::builtin_type(Type::Obj).into());
}
Ok(other) => {
return Err(type_mismatch("NamedTuple", other, "Self"));
}
Expand All @@ -509,7 +512,7 @@ pub(crate) fn as_dict(mut args: ValueArgs, ctx: &Context) -> EvalValueResult<TyP
.ok_or_else(|| not_passed("Self"))?;
let fields = match ctx.convert_value_into_type(slf) {
Ok(Type::Record(fields)) => fields,
Ok(Type::Mono(Str::Static("Record"))) => {
Ok(Type::Mono(n)) if &n == "Record" => {
let dict = dict! { Type::Obj => Type::Obj };
return Ok(ValueObj::builtin_type(Type::from(dict)).into());
}
Expand Down
6 changes: 3 additions & 3 deletions crates/erg_compiler/context/inquire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ impl Context {
for ctx in ctxs {
match ctx.rec_get_var_info(ident, AccessKind::BoundAttr, input, namespace) {
Triple::Ok(vi) => {
obj.ref_t().coerce();
obj.ref_t().destructive_coerce();
return Triple::Ok(vi);
}
Triple::Err(e) => {
Expand Down Expand Up @@ -1133,7 +1133,7 @@ impl Context {
.map_err(|mut errs| errs.remove(0))?;
if &coerced != obj.ref_t() {
let hash = get_hash(obj.ref_t());
obj.ref_t().coerce();
obj.ref_t().destructive_coerce();
if get_hash(obj.ref_t()) != hash {
return self
.search_method_info(obj, attr_name, pos_args, kw_args, input, namespace);
Expand Down Expand Up @@ -1392,7 +1392,7 @@ impl Context {
}
if sub != Never {
let hash = get_hash(instance);
instance.coerce();
instance.destructive_coerce();
if instance.is_quantified_subr() {
let instance = self.instantiate(instance.clone(), obj)?;
self.substitute_call(obj, attr_name, &instance, pos_args, kw_args)?;
Expand Down
Loading

0 comments on commit 3724a74

Please sign in to comment.