From c2452ff751be04c308d6cbafc4b0fd48af7cf3bc Mon Sep 17 00:00:00 2001
From: Ayaz Hafiz <ayaz@rwx.com>
Date: Tue, 6 Sep 2022 17:42:38 -0500
Subject: [PATCH] Lose rigidity of annotated optional fields before
 generalization

We have this idea of "rigid optional" fields to annotate record fields
that must necessarily be optional. That avoids the admission of programs
we cannot faithfully compile, like

```
f : {a: Str, b ? U64}
f = {a: "b", b: 1}
```

We want to lose the rigidity restriction when a generalized symbol is
used as at a specialized site; for example it should be possible to call
`f : {x ? Str} -> {}` with both `{}` and `{x : Str}`, neither of which
have a rigidly optional field unless they were to be annotated.

Prior to this commit we would loosen the rigidity restriction upon
specialization of a generalized type at a use site. However, what we
really want to do is apply the loosening during calculation of
generalization. The reason is that otherwise, we must make types that
would be ground (like `{x ? Str} -> {}`) generalized just for the sake
of the optional field annotation. But since the rigidity constraint is
irrelevant after an annotated body has been checked, we can loosen the
rigidity restriction then, which conveniently happens to coincide with
the generalization calculation.

Closes #3955
---
 crates/compiler/solve/src/solve.rs        | 13 ++++++++++---
 crates/compiler/solve/tests/solve_expr.rs | 14 ++++++++++++++
 crates/reporting/tests/test_reporting.rs  | 19 -------------------
 3 files changed, 24 insertions(+), 22 deletions(-)

diff --git a/crates/compiler/solve/src/solve.rs b/crates/compiler/solve/src/solve.rs
index 9eadb602e8..185edb69b8 100644
--- a/crates/compiler/solve/src/solve.rs
+++ b/crates/compiler/solve/src/solve.rs
@@ -3124,9 +3124,16 @@ fn adjust_rank_content(
                 Record(fields, ext_var) => {
                     let mut rank = adjust_rank(subs, young_mark, visit_mark, group_rank, *ext_var);
 
-                    for index in fields.iter_variables() {
-                        let var = subs[index];
+                    for (_, var_index, field_index) in fields.iter_all() {
+                        let var = subs[var_index];
                         rank = rank.max(adjust_rank(subs, young_mark, visit_mark, group_rank, var));
+
+                        // When generalizing annotations with rigid optionals, we want to promote
+                        // them to non-rigid, so that usages at specialized sites don't have to
+                        // exactly include the optional field.
+                        if let RecordField::RigidOptional(()) = subs[field_index] {
+                            subs[field_index] = RecordField::Optional(());
+                        }
                     }
 
                     rank
@@ -3499,7 +3506,7 @@ fn deep_copy_var_help(
                                 let slice = SubsSlice::extend_new(
                                     &mut subs.record_fields,
                                     field_types.into_iter().map(|f| match f {
-                                        RecordField::RigidOptional(()) => RecordField::Optional(()),
+                                        RecordField::RigidOptional(()) => internal_error!("RigidOptionals should be generalized to non-rigid by this point"),
 
                                         RecordField::Demanded(_)
                                         | RecordField::Required(_)
diff --git a/crates/compiler/solve/tests/solve_expr.rs b/crates/compiler/solve/tests/solve_expr.rs
index c1c11e06fa..e2ee7f040b 100644
--- a/crates/compiler/solve/tests/solve_expr.rs
+++ b/crates/compiler/solve/tests/solve_expr.rs
@@ -7761,4 +7761,18 @@ mod solve_expr {
         "###
         );
     }
+
+    #[test]
+    fn unify_optional_record_fields_in_two_closed_records() {
+        infer_eq_without_problem(
+            indoc!(
+                r#"
+                f : { x ? Str, y ? Str } -> {}
+                
+                f {x : ""}
+                "#
+            ),
+            "{}",
+        );
+    }
 }
diff --git a/crates/reporting/tests/test_reporting.rs b/crates/reporting/tests/test_reporting.rs
index 999decb35c..c83dc2cbdd 100644
--- a/crates/reporting/tests/test_reporting.rs
+++ b/crates/reporting/tests/test_reporting.rs
@@ -6602,25 +6602,6 @@ All branches in an `if` must have the same type!
 
         { inputs ? List Str }
 
-    Tip: To extract the `.inputs` field it must be non-optional, but the
-    type says this field is optional. Learn more about optional fields at
-    TODO.
-
-    ── TYPE MISMATCH ───────────────────────────────────────── /code/proj/Main.roc ─
-
-    This 1st argument to `job` has an unexpected type:
-
-    9│      job { inputs: ["build", "test"] }
-                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-
-    The argument is a record of type:
-
-        { inputs : List Str }
-
-    But `job` needs its 1st argument to be:
-
-        { inputs ? List Str }
-
     Tip: To extract the `.inputs` field it must be non-optional, but the
     type says this field is optional. Learn more about optional fields at
     TODO.