Disallow multiple dereferences during typechecking (#214)

Author: LiterallyVoid

examples/typechecking/field_diagnostics.an

@@ -0,0 +1,54 @@
+tuple = (1, 1)
+
+_ = (&&tuple).first
+
+_ = tuple.!first
+_ = tuple.!missing
+
+imm_ref = &tuple
+_ = imm_ref.!first
+_ = imm_ref.!missing
+
+mut_tuple = mut (1, 1)
+
+_ = mut_tuple.&missing
+_ = mut_tuple.!missing
+
+mut_ref = !mut_tuple
+
+_ = mut_ref.&missing
+_ = mut_ref.!missing
+
+_ = (!&tuple).!first
+
+// flags: --check
+// expected stderr:
+// field_diagnostics.an:3:6	error: &shared &shared Int c, Int d has no field 'first' of type { first: b, .. }
+// _ = (&&tuple).first
+// 
+// field_diagnostics.an:5:5	error: Cannot mutably reference `tuple`. It was declared as immutable
+// _ = tuple.!first
+// 
+// field_diagnostics.an:6:5	error: Int a, Int b has no field 'missing' of type { missing: b, .. }
+// _ = tuple.!missing
+// 
+// field_diagnostics.an:9:5	error: Expected a mutable reference but found &shared Int b, Int c instead
+// _ = imm_ref.!first
+// 
+// field_diagnostics.an:10:5	error: &shared Int b, Int c has no field 'missing' of type { missing: b, .. }
+// _ = imm_ref.!missing
+// 
+// field_diagnostics.an:14:5	error: Int a, Int b has no field 'missing' of type { missing: b, .. }
+// _ = mut_tuple.&missing
+// 
+// field_diagnostics.an:15:5	error: Int a, Int b has no field 'missing' of type { missing: b, .. }
+// _ = mut_tuple.!missing
+// 
+// field_diagnostics.an:19:5	error: !shared Int b, Int c has no field 'missing' of type { missing: b, .. }
+// _ = mut_ref.&missing
+// 
+// field_diagnostics.an:20:5	error: !shared Int b, Int c has no field 'missing' of type { missing: b, .. }
+// _ = mut_ref.!missing
+// 
+// field_diagnostics.an:22:6	error: !shared &shared Int c, Int d has no field 'first' of type { first: b, .. }
+// _ = (!&tuple).!first

src/error/mod.rs

@@ -125,7 +125,7 @@ pub enum TypeErrorKind {
     MatchPatternTypeDiffers,
     MatchReturnTypeDiffers,
     DoesNotMatchAnnotatedType,
-    ExpectedStructReference,
+    ExpectedMutable,
 
     // This taking a String is the reason we can't have nice things (Copy)
     NoFieldOfType(/*field name*/ String),
@@ -309,8 +309,8 @@ impl Display for DiagnosticKind {
             DiagnosticKind::TypeError(TypeErrorKind::DoesNotMatchAnnotatedType, actual, expected) => {
                 write!(f, "Expression of type {actual} does not match its annotated type {expected}")
             },
-            DiagnosticKind::TypeError(TypeErrorKind::ExpectedStructReference, actual, _expected) => {
-                write!(f, "Expected a struct reference but found {actual} instead")
+            DiagnosticKind::TypeError(TypeErrorKind::ExpectedMutable, actual, _expected) => {
+                write!(f, "Expected a mutable reference but found {actual} instead")
             },
             DiagnosticKind::TypeError(TypeErrorKind::NoFieldOfType(field_name), actual, expected) => {
                 write!(f, "{actual} has no field '{field_name}' of type {expected}")

src/types/typechecker.rs

@@ -856,9 +856,8 @@ fn get_fields(
                 },
             }
         },
-        TypeApplication(constructor, args) => match follow_bindings_in_cache_and_map(constructor, bindings, cache) {
-            Ref { .. } => get_fields(&args[0], &[], bindings, cache),
-            other => get_fields(&other, args, bindings, cache),
+        TypeApplication(constructor, args) => {
+            get_fields(&follow_bindings_in_cache_and_map(constructor, bindings, cache), args, bindings, cache)
         },
         Struct(fields, rest) => match &cache.type_bindings[rest.0] {
             Bound(binding) => get_fields(&binding.clone(), args, bindings, cache),
@@ -2007,33 +2006,60 @@ impl<'a> Inferable<'a> for ast::Extern<'a> {
 
 impl<'a> Inferable<'a> for ast::MemberAccess<'a> {
     fn infer_impl(&mut self, cache: &mut ModuleCache<'a>) -> TypeResult {
-        let mut result = infer(self.lhs.as_mut(), cache);
+        let result = infer(self.lhs.as_mut(), cache);
 
         let level = LetBindingLevel(CURRENT_LEVEL.load(Ordering::SeqCst));
         let mut field_type = cache.next_type_variable(level);
 
-        // If this is a mutable reference to a field, ensure the collection is either
-        // a mutable reference to a collection, or is a local mutable variable.
-        if let Some(Mutability::Mutable) = self.offset {
-            let collection_variable = next_type_variable(cache);
-            let expected = ref_of(Mutability::Mutable, collection_variable.clone(), cache);
-
-            match try_unify(&result.typ, &expected, self.lhs.locate(), cache, TE::ExpectedStructReference) {
-                Ok(bindings) => bindings.perform(cache),
-                Err(_) => check_field_access_lhs_is_mutable(&self.lhs, false, cache),
-            }
-
-            result.typ = collection_variable;
-        }
-
         let mut fields = BTreeMap::new();
         fields.insert(self.field.clone(), field_type.clone());
 
         // The '..' or 'rest of the struct' stand-in variable
         let rho = cache.next_type_variable_id(level);
         let struct_type = Type::Struct(fields, rho);
 
-        unify(&result.typ, &struct_type, self.location, cache, TE::NoFieldOfType(self.field.clone()));
+        let mut bindings =
+            try_unify(&result.typ, &struct_type, self.location, cache, TE::NoFieldOfType(self.field.clone()));
+
+        if bindings.is_ok() && self.offset == Some(Mutability::Mutable) {
+            // If unification succeeded (`bindings.is_ok()`) and this is a
+            // mutable field access (`self.offset == Mutable`), `self.lhs`
+            // must be a mutable variable.
+            check_field_access_lhs_is_mutable(self.lhs.as_ref(), false, cache);
+        } else if let Err(error_without_deref) = bindings {
+            // If unification failed, try again with a single dereference.
+            //
+            // (This replicates logic from AST-to-HIR monomorphisation.)
+
+            // If this is a mutable field access (`a.!b`), it's necessary to
+            // unify the actual type (`result.typ`) with a mutable reference to
+            // the partial `struct_type`.
+            let struct_ref = ref_of(self.offset.unwrap_or(Mutability::Immutable), struct_type.clone(), cache);
+
+            bindings = try_unify(&result.typ, &struct_ref, self.lhs.locate(), cache, TE::ExpectedMutable);
+
+            // The complicated logic here is solely to improve diagnostics.
+            bindings = bindings.map_err(|error_with_mutable_deref| {
+                let struct_imm_ref = ref_of(Mutability::Immutable, struct_type.clone(), cache);
+
+                // This unification is only used for a single bit of information.
+                let unification_with_immutable_deref =
+                    try_unify(&result.typ, &struct_imm_ref, self.lhs.locate(), cache, TE::ExpectedMutable);
+
+                if unification_with_immutable_deref.is_ok() {
+                    // Show TE::ExpectedMutable from the second `try_unify`
+                    // when an immutable deref unifies but a mutable deref
+                    // doesn't.
+                    error_with_mutable_deref
+                } else {
+                    // Unification still fails against an immutable reference.
+                    // Show the diagnostic that doesn't mention references.
+                    error_without_deref
+                }
+            });
+        }
+
+        perform_bindings_or_push_error(bindings, cache);
 
         if let Some(mutability) = self.offset {
             field_type = ref_of(mutability, field_type, cache);