Fix uniqueness error of variables bound internal to a module (#3632)

* add test

* fix

* add comments

Author: riaqn

testsuite/tests/typing-unique/unique_mod_class.ml

@@ -162,3 +162,15 @@ let foo (local_ x : string ref) =
 [%%expect{|
 val foo : local_ string ref -> (unit -> < m : string >) = <fun>
 |}]
+
+let () =
+    let module M =
+        struct
+        let () =
+          let x = "hello" in
+          let _ = unique_id x in
+          ()
+        end
+    in ()
+[%%expect{|
+|}]

typing/uniqueness_analysis.ml

@@ -2023,14 +2023,24 @@ let comp_pattern_match pat value =
   | Some pat' -> pattern_match pat' value
   | None -> Ienv.Extension.empty, UF.unused
 
-let value_of_ident ienv unique_use occ path =
+(** Given some [ienv], find the [Value.t] corresponding to an identifier.
+
+There are two cases that it might be missing, both of which are related to
+"module and class boundary" (see below):
+- We are checking inside a module, and the identifier is refering to a value
+defined outside of the module. In such case, we force this identifier to
+[aliased].
+- Another case is used by [open_variables]. See comments there.
+*)
+let value_of_ident ienv ?(force_missing = true) unique_use occ path =
   match path with
   | Path.Pident id -> (
     match Ienv.find_opt id ienv with
     (* TODO: for better error message, we should record in ienv why some
        variables are not in it. *)
     | None ->
-      force_aliased_boundary ~reason:Out_of_mod_class unique_use occ;
+      if force_missing
+      then force_aliased_boundary ~reason:Out_of_mod_class unique_use occ;
       None
     | Some paths ->
       let value = Value.existing paths unique_use occ in
@@ -2042,8 +2052,8 @@ let value_of_ident ienv unique_use occ path =
     None
   | Path.Papply _ | Path.Pextra_ty _ -> assert false
 
-(* TODO: replace the dirty hack.
-   The following functions are dirty hacks and used for modules and classes.
+(* Module and class boundary
+
    Currently we treat the boundary between modules/classes and their surrounding
    environment coarsely. To be specific, all references in the modules/classes
    pointing to the environment are treated as many and aliased. This translates
@@ -2061,8 +2071,15 @@ let open_variables ienv f =
         (fun self e ->
           (match e.exp_desc with
           | Texp_ident (path, _, _, _, unique_use) -> (
+            (* We test if a variable is open by looking it up in the current
+               [ienv]: the [ienv] does not contain the internally-bound
+               variables in the module. In other words, open variables will be
+               found, and closed variables will be missing. For the latter, we
+               of course should not force them to [aliased]. *)
             let occ = Occurrence.mk e.exp_loc in
-            match value_of_ident ienv unique_use occ path with
+            match
+              value_of_ident ienv ~force_missing:false unique_use occ path
+            with
             | None -> ()
             | Some value -> ll := value :: !ll)
           | _ -> ());