Skip to content

Commit

Permalink
fix: eliminate unsafe type-var updating
Browse files Browse the repository at this point in the history
  • Loading branch information
@mtshiba
mtshiba committed May 27, 2023
1 parent 4d98007 commit 7049faf
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 103 deletions.
2 changes: 2 additions & 0 deletions crates/erg_common/shared.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,13 @@ impl<T: ?Sized> Shared<T> {
}

#[inline]
#[track_caller]
pub fn borrow(&self) -> Ref<'_, T> {
RefCell::borrow(&self.0)
}

#[inline]
#[track_caller]
pub fn borrow_mut(&self) -> RefMut<'_, T> {
RefCell::borrow_mut(&self.0)
}
Expand Down
80 changes: 36 additions & 44 deletions crates/erg_compiler/context/compare.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -459,54 +459,46 @@ impl Context {
// => ?P.undoable_link(Int)
// => Mul Int :> Int
(FreeVar(lfv), rhs) => {
match &*lfv.borrow() {
FreeKind::Linked(t) | FreeKind::UndoableLinked { t, .. } => {
self.supertype_of(t, rhs)
}
FreeKind::Unbound { constraint: _, .. }
| FreeKind::NamedUnbound { constraint: _, .. } => {
if let Some((_sub, sup)) = lfv.get_subsup() {
lfv.forced_undoable_link(rhs);
let res = self.supertype_of(&sup, rhs);
lfv.undo();
res
} else if let Some(lfvt) = lfv.get_type() {
// e.g. lfv: ?L(: Int) is unreachable
// but
// ?L(: Array(Type, 3)) :> Array(Int, 3)
// => Array(Type, 3) :> Array(Typeof(Int), 3)
// => true
let rhs_meta = self.meta_type(rhs);
self.supertype_of(&lfvt, &rhs_meta)
} else {
// constraint is uninitialized
log!(err "constraint is uninitialized: {lfv}/{rhs}");
true
}
}
if let FreeKind::Linked(t) | FreeKind::UndoableLinked { t, .. } = &*lfv.borrow() {
return self.supertype_of(t, rhs);
}
if let Some((_sub, sup)) = lfv.get_subsup() {
lfv.undoable_link(rhs);
let res = self.supertype_of(&sup, rhs);
lfv.undo();
res
} else if let Some(lfvt) = lfv.get_type() {
// e.g. lfv: ?L(: Int) is unreachable
// but
// ?L(: Array(Type, 3)) :> Array(Int, 3)
// => Array(Type, 3) :> Array(Typeof(Int), 3)
// => true
let rhs_meta = self.meta_type(rhs);
self.supertype_of(&lfvt, &rhs_meta)
} else {
// constraint is uninitialized
log!(err "constraint is uninitialized: {lfv}/{rhs}");
true
}
}
(lhs, FreeVar(rfv)) => match &*rfv.borrow() {
FreeKind::Linked(t) | FreeKind::UndoableLinked { t, .. } => {
self.supertype_of(lhs, t)
(lhs, FreeVar(rfv)) => {
if let FreeKind::Linked(t) | FreeKind::UndoableLinked { t, .. } = &*rfv.borrow() {
return self.supertype_of(lhs, t);
}
FreeKind::Unbound { constraint: _, .. }
| FreeKind::NamedUnbound { constraint: _, .. } => {
if let Some((sub, _sup)) = rfv.get_subsup() {
rfv.forced_undoable_link(lhs);
let res = self.supertype_of(lhs, &sub);
rfv.undo();
res
} else if let Some(rfvt) = rfv.get_type() {
let lhs_meta = self.meta_type(lhs);
self.supertype_of(&lhs_meta, &rfvt)
} else {
// constraint is uninitialized
log!(err "constraint is uninitialized: {lhs}/{rfv}");
true
}
if let Some((sub, _sup)) = rfv.get_subsup() {
rfv.undoable_link(lhs);
let res = self.supertype_of(lhs, &sub);
rfv.undo();
res
} else if let Some(rfvt) = rfv.get_type() {
let lhs_meta = self.meta_type(lhs);
self.supertype_of(&lhs_meta, &rfvt)
} else {
// constraint is uninitialized
log!(err "constraint is uninitialized: {lhs}/{rfv}");
true
}
},
}
(Record(lhs), Record(rhs)) => {
for (l_k, l_t) in lhs.iter() {
if let Some((r_k, r_t)) = rhs.get_key_value(l_k) {
Expand Down
6 changes: 3 additions & 3 deletions crates/erg_compiler/context/generalize.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ impl Generalizer {
if sub == sup {
let t = self.generalize_t(sub, uninit);
let res = FreeVar(fv);
res.forced_link(&t);
res.link(&t);
res
} else if sup != Obj
&& !self.qnames.contains(&fv.unbound_name().unwrap())
Expand Down Expand Up @@ -532,10 +532,10 @@ impl<'c, 'q, 'l, L: Locational> Dereferencer<'c, 'q, 'l, L> {
fv.dummy_link();
}
(true, false) => {
fv.forced_undoable_link(&super_t);
fv.undoable_link(&super_t);
}
(false, true | false) => {
fv.forced_undoable_link(&sub_t);
fv.undoable_link(&sub_t);
}
}
let res = self.validate_subsup(sub_t, super_t);
Expand Down
56 changes: 12 additions & 44 deletions crates/erg_compiler/ty/free.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -601,9 +601,11 @@ impl<T: LimitedDisplay> LimitedDisplay for Free<T> {
}

impl<T> Free<T> {
#[track_caller]
pub fn borrow(&self) -> Ref<'_, FreeKind<T>> {
self.0.borrow()
}
#[track_caller]
pub fn borrow_mut(&self) -> RefMut<'_, FreeKind<T>> {
self.0.borrow_mut()
}
Expand All @@ -614,15 +616,6 @@ impl<T> Free<T> {
pub fn forced_as_ref(&self) -> &FreeKind<T> {
unsafe { self.as_ptr().as_ref() }.unwrap()
}
pub fn force_replace(&self, new: FreeKind<T>) {
// prevent linking to self
if addr_eq!(*self.borrow(), new) {
return;
}
unsafe {
*self.0.as_ptr() = new;
}
}
pub fn can_borrow(&self) -> bool {
self.0.can_borrow()
}
Expand Down Expand Up @@ -756,6 +749,7 @@ impl<T> Free<T> {
Self(Shared::new(FreeKind::Linked(t)))
}

#[track_caller]
pub fn replace(&self, to: FreeKind<T>) {
// prevent linking to self
if self.is_linked() && addr_eq!(*self.borrow(), to) {
Expand All @@ -766,6 +760,7 @@ impl<T> Free<T> {

/// returns linked type (panic if self is unbounded)
/// NOTE: check by `.is_linked` before call
#[track_caller]
pub fn crack(&self) -> Ref<'_, T> {
Ref::map(self.borrow(), |f| match f {
FreeKind::Linked(t) | FreeKind::UndoableLinked { t, .. } => t,
Expand All @@ -775,6 +770,7 @@ impl<T> Free<T> {
})
}

#[track_caller]
pub fn crack_constraint(&self) -> Ref<'_, Constraint> {
Ref::map(self.borrow(), |f| match f {
FreeKind::Linked(_) | FreeKind::UndoableLinked { .. } => panic!("the value is linked"),
Expand Down Expand Up @@ -817,6 +813,7 @@ impl<T> Free<T> {
impl<T: Clone> Free<T> {
/// SAFETY: use `Type/TyParam::link` instead of this.
/// This method may cause circular references.
#[track_caller]
pub(super) fn link(&self, to: &T) {
// prevent linking to self
if self.is_linked() && addr_eq!(*self.crack(), *to) {
Expand All @@ -825,19 +822,7 @@ impl<T: Clone> Free<T> {
self.borrow_mut().replace(to.clone());
}

/// 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(super) fn forced_link(&self, to: &T) {
// prevent linking to self
if self.is_linked() && addr_eq!(*self.crack(), *to) {
return;
}
unsafe {
self.as_ptr().as_mut().unwrap().replace(to.clone());
}
}

#[track_caller]
pub fn undoable_link(&self, to: &T) {
if self.is_linked() && addr_eq!(*self.crack(), *to) {
panic!("link to self");
Expand All @@ -850,29 +835,12 @@ impl<T: Clone> Free<T> {
*self.borrow_mut() = new;
}

/// 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_undoable_link(&self, to: &T) {
if self.is_linked() && addr_eq!(*self.crack(), *to) {
panic!("link to self");
}
let prev = self.clone_inner();
let new = FreeKind::UndoableLinked {
t: to.clone(),
previous: Box::new(prev),
};
self.force_replace(new);
}

pub fn undo(&self) {
match &*self.borrow() {
FreeKind::UndoableLinked { previous, .. } => {
let prev = *previous.clone();
self.force_replace(prev);
}
let prev = match &*self.borrow() {
FreeKind::UndoableLinked { previous, .. } => *previous.clone(),
_other => panic!("cannot undo"),
}
};
self.replace(prev);
}

pub fn unwrap_unbound(self) -> (Option<Str>, usize, Constraint) {
Expand Down Expand Up @@ -922,7 +890,7 @@ impl<T: Clone> Free<T> {

impl<T: Default + Clone> Free<T> {
pub fn dummy_link(&self) {
self.forced_undoable_link(&T::default());
self.undoable_link(&T::default());
}
}

Expand Down
13 changes: 1 addition & 12 deletions crates/erg_compiler/ty/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2326,7 +2326,7 @@ impl Type {

pub fn qvars(&self) -> Set<(Str, Constraint)> {
match self {
Self::FreeVar(fv) if fv.is_linked() => fv.crack().qvars(),
Self::FreeVar(fv) if fv.is_linked() => fv.unsafe_crack().qvars(),
Self::FreeVar(fv) if !fv.constraint_is_uninited() => {
let base = set! {(fv.unbound_name().unwrap(), fv.constraint().unwrap())};
if let Some((sub, sup)) = fv.get_subsup() {
Expand Down Expand Up @@ -2973,17 +2973,6 @@ impl Type {
_ => 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> {
Expand Down

0 comments on commit 7049faf

Please sign in to comment.