Skip to content

Commit

Permalink
Fix type inference when comparing references
Browse files Browse the repository at this point in the history
When comparing (for example) `ref A` with `ref B`, we want to assign `B`
to `A`, not `ref a`. Prior to this commit, `B` would be assigned `ref A`
as a whole.

This fixes #346.

Changelog: fixed
  • Loading branch information
yorickpeterse committed May 26, 2023
1 parent 5545768 commit d687283
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 7 deletions.
74 changes: 67 additions & 7 deletions types/src/check.rs
Expand Up @@ -226,6 +226,7 @@ impl<'a> TypeChecker<'a> {
// This is OK because in practise, Infer() only shows up on the left in
// a select few cases.
let rules = rules.dont_infer_as_rigid();
let original_right = right;
let right = self.resolve(right, &env.right, rules);

// If at this point we encounter a type placeholder, it means the
Expand Down Expand Up @@ -272,7 +273,12 @@ impl<'a> TypeChecker<'a> {
}
TypeRef::Placeholder(id) => self
.check_type_id_with_placeholder(
left, left_id, id, env, rules,
left,
left_id,
original_right,
id,
env,
rules,
),
TypeRef::Any | TypeRef::Error => true,
_ => false,
Expand All @@ -291,7 +297,12 @@ impl<'a> TypeChecker<'a> {
}
TypeRef::Placeholder(id) => self
.check_type_id_with_placeholder(
left, left_id, id, env, rules,
left,
left_id,
original_right,
id,
env,
rules,
),
TypeRef::Any => {
if let TypeId::ClassInstance(ins) = left_id {
Expand All @@ -318,7 +329,12 @@ impl<'a> TypeChecker<'a> {
}
TypeRef::Placeholder(id) => self
.check_type_id_with_placeholder(
left, left_id, id, env, rules,
left,
left_id,
original_right,
id,
env,
rules,
),
TypeRef::Any => {
if let TypeId::ClassInstance(ins) = left_id {
Expand All @@ -341,7 +357,12 @@ impl<'a> TypeChecker<'a> {
}
TypeRef::Placeholder(id) => self
.check_type_id_with_placeholder(
left, left_id, id, env, rules,
left,
left_id,
original_right,
id,
env,
rules,
),
TypeRef::Any | TypeRef::Error => true,
_ => false,
Expand Down Expand Up @@ -373,7 +394,12 @@ impl<'a> TypeChecker<'a> {
}

self.check_type_id_with_placeholder(
left, left_id, id, env, rules,
left,
left_id,
original_right,
id,
env,
rules,
)
}
TypeRef::Any | TypeRef::Error => true,
Expand All @@ -396,7 +422,12 @@ impl<'a> TypeChecker<'a> {
}
TypeRef::Placeholder(id) => self
.check_type_id_with_placeholder(
left, left_id, id, env, rules,
left,
left_id,
original_right,
id,
env,
rules,
),
TypeRef::Any | TypeRef::Error => true,
_ => false,
Expand Down Expand Up @@ -560,14 +591,23 @@ impl<'a> TypeChecker<'a> {
&mut self,
left: TypeRef,
left_id: TypeId,
original_right: TypeRef,
placeholder: TypePlaceholderId,
env: &mut Environment,
rules: Rules,
) -> bool {
// By assigning the placeholder first, recursive checks against the same
// placeholder don't keep recursing into this method, instead checking
// against the value on the left.
placeholder.assign(self.db, left);
//
// When comparing `ref A` with `ref B` or `mut A` with `mut B`, we want
// to assign `B` to `A`, not `ref A`/`mut A`.
if left.is_ref_or_mut(self.db) && original_right.is_ref_or_mut(self.db)
{
placeholder.assign(self.db, TypeRef::Owned(left_id));
} else {
placeholder.assign(self.db, left);
}

let req = if let Some(req) = placeholder.required(self.db) {
req
Expand Down Expand Up @@ -1400,6 +1440,26 @@ mod tests {
check_err(&db, mutable(instance(thing)), uni(instance(thing)));
}

#[test]
fn test_mut_with_mut_type_parameter() {
let mut db = Database::new();
let param = new_parameter(&mut db, "T");
let var = TypePlaceholder::alloc(&mut db, None);

let mut env = Environment::new(
TypeArguments::new(),
type_arguments(vec![(param, placeholder(var))]),
);
let res = TypeChecker::new(&db).run(
mutable(rigid(param)),
mutable(parameter(param)),
&mut env,
);

assert!(res);
assert_eq!(var.value(&db), Some(owned(rigid(param))));
}

#[test]
fn test_ref_uni() {
let mut db = Database::new();
Expand Down
10 changes: 10 additions & 0 deletions types/src/lib.rs
Expand Up @@ -3639,6 +3639,16 @@ impl TypeRef {
}
}

pub fn is_ref_or_mut(self, db: &Database) -> bool {
match self {
TypeRef::Mut(_) | TypeRef::Ref(_) => true,
TypeRef::Placeholder(id) => {
id.value(db).map_or(false, |v| v.is_ref_or_mut(db))
}
_ => false,
}
}

pub fn is_mutable(self, db: &Database) -> bool {
match self {
TypeRef::Owned(_)
Expand Down

0 comments on commit d687283

Please sign in to comment.