Recognize untagged payloads only by annotations and builtin types

changes/untagged-no-hardcoded-classes.internal.md

@@ -0,0 +1 @@
+- Untagged variants recognize stdlib classes only through their `@instanceOf` annotations; the compiler's list of stdlib type names is gone.

compiler/ml/UntaggedVariants.md

@@ -90,29 +90,31 @@ into one kind:
 | a function type | function | `typeof x === "function"` |
 | a record (not `@unboxed`), an inline record, `dict<_>` | object | `typeof x === "object"` |
 | `array<_>`, a tuple | instance `Array` | `Array.isArray(x)` |
-| a type declared with `@instanceOf("C")`, or listed in rule 5 | instance `C` | `x instanceof C` |
+| a type declared with `@instanceOf("C")` (rule 5) | instance `C` | `x instanceof C` |
+| `promise<_>` | instance `Promise` | `x instanceof Promise` |
 | a type declared with `@typeOf("k")` | kind `k` | `typeof x === "k"` |
 | anything else: an abstract type, a type variable, a variant, `list<_>`, an `@unboxed` record | unknown | no check |
 
 The annotations of rule 5 are read first, on the type as written and on each
-type it abbreviates. Then the list of rule 5 is consulted on the type as
-written, and then the payload type is classified after expanding
-abbreviations (`type myDate = Date.t` is an instance case). `@typeOf("number")`
+type it abbreviates. Otherwise the payload type is classified after expanding
+abbreviations: `type ints = array<int>` is an array case, and `type name =
+string` a string case. A type is never recognized by its name. `@typeOf("number")`
 is the number kind, and `@typeOf("symbol")` is a kind of its own, with no
 builtin payload type. `@notUndefined` on an abstract type leaves it unknown.
 
 - Implementation: `variant_layout.ml:get_block_type_from_typ`,
-  `get_block_type`, `declared_block_type`;
-  `ast_untagged_variants.ml:type_is_builtin_object`.
+  `get_block_type`, `declared_block_type`.
 - Tests: `spec_test` rules 4 (`kinds`, `tuples`, `aliasDict`, `instances`,
   `unknown`); fixtures `UntaggedConstructorMoreThanOneArg.res`,
   `UntaggedConstructorAttributeMoreThanOneArg.res`, `UntaggedNonUnary1.res`,
   `UntaggedNonUnary2.res` ("Constructor A has more than one argument."),
-  `UntaggedUnknown.res` (`list<float>` is unknown);
-  `tests/tests/src/untagged_runtime_annotations_test.res`.
+  `UntaggedUnknown.res` (`list<float>` is unknown),
+  `untagged_unannotated_stdlib_name.res` (a type named like a stdlib class
+  is unknown); `tests/tests/src/untagged_runtime_annotations_test.res`,
+  `untagged_builtin_alias_test.res`.
 - Shaped by #6121, #6208, #6219, #6279, #6368, #6394, #6550, #6670.
 
-### 5. Declared and listed instance types
+### 5. Declared instance types
 
 `@instanceOf("C")` on a type declares that its values are instances of the
 global class at the path `C`, such as `"Date"` or `"Intl.Locale"`.
@@ -181,37 +183,22 @@ The stdlib annotates the types of the classes it binds:
 | `Int8Array.t` and the other typed arrays, public abbreviations of `TypedArray.t<_>` | `@instanceOf` the class of the same name |
 | `Symbol.t` | `@typeOf("symbol")` |
 
-Types without annotations are recognized from a fixed list of type names in
-`ast_untagged_variants.ml:type_to_instanceof_backed_obj`. Every listed stdlib
-type is annotated, so the list only covers other types printed with one of
-these paths. The predefined
-`promise<_>` and `array<_>` are matched by path; the others by the printed
-path of the type:
-
-| Type | Class |
-| --- | --- |
-| `promise<_>` | `Promise` |
-| `array<_>` | `Array` (checked with `Array.isArray`) |
-| `Date.t`, `RegExp.t`, `File.t`, `Blob.t`, `ArrayBuffer.t`, `DataView.t` | the class of the same name |
-| `Set.t`, `Map.t`, `WeakSet.t`, `WeakMap.t` | the class of the same name |
-| `Int8Array.t`, `Uint8Array.t`, `Uint8ClampedArray.t`, `Int16Array.t`, `Uint16Array.t`, `Int32Array.t`, `Uint32Array.t`, `Float32Array.t`, `Float64Array.t`, `BigInt64Array.t`, `BigUint64Array.t` | the class of the same name |
-
-The listed classes have no superclasses. The typed-array types are
-abbreviations of one type, `TypedArray.t<_>`, and are told apart only by
-their annotations, which are read on the name as written (Known bugs).
+The typed-array types are abbreviations of one type, `TypedArray.t<_>`, and
+are told apart only by their annotations, which are read on the name as
+written (Known bugs).
 
 - Implementation: `variant_layout.ml:runtime_annotations`,
   `declared_block_type`, `check_manifest_annotation`;
   `ast_untagged_variants.ml:process_runtime_annotation`, `unsupported_class`,
-  `check_runtime_annotation`,
-  `type_to_instanceof_backed_obj`;
+  `check_runtime_annotation`;
   `variant_runtime.ml:Instance`; `js_exp_make.ml:class_constructor`.
 - Tests: `spec_test` rules 4 and 5;
   `tests/tests/src/untagged_runtime_annotations_test.res`,
   `untagged_runtime_annotations_shadow_test.res`,
   `untagged_file_blob_test.res`; the fixtures above,
   `UntaggedAtMostOneInstance.res`, `UntaggedTupleAndArray.res`.
-- Shaped by #6296, #6383, #6396, #7788, #8009, #8950, #8948. Related: #7147.
+- Shaped by #6296, #6383, #6396, #7788, #8009, #8950, #8948, #8949.
+  Related: #7147.
 
 ### 6. Well-formedness
 
@@ -545,8 +532,8 @@ had to answer, and the answers of rule 5:
 7. **Implied `@notUndefined`.** A declared class or kind implies it, and
    rule 5 rejects annotations on abbreviations of types that can hold
    `undefined`.
-8. **Abbreviations.** Annotations are read along the chain of abbreviations;
-   the list of rule 5 is still read on the name as written.
+8. **Abbreviations.** Annotations are read along the chain of abbreviations,
+   and a type is never recognized by its name.
 9. **Globals.** A shadowed class is read through `globalThis`. A missing
    class throws when the check runs (rule 10).
 

compiler/ml/ast_untagged_variants.ml

@@ -360,47 +360,6 @@ let check_runtime_annotation ~loc ~is_abstract_or_abbreviation
              RuntimeAnnotationOnRepresentedType
                (runtime_attribute_name annotation) ))
 
-let type_is_builtin_object (t : Types.type_expr) =
-  match t.desc with
-  | Tconstr (Path.Pident ident, [_], _) when Ident.name ident = "dict" -> true
-  | Tconstr (path, _, _) ->
-    let name = Path.name path in
-    name = "Stdlib.Dict.t" || name = "Stdlib_Dict.t"
-  | _ -> false
-
-let type_to_instanceof_backed_obj (t : Types.type_expr) =
-  let cls name = Some (Instance.Class {name; ancestors = []}) in
-  match t.desc with
-  | Tconstr (path, _, _) when Path.same path Predef.path_promise ->
-    cls "Promise"
-  | Tconstr (path, _, _) when Path.same path Predef.path_array ->
-    Some Instance.Array
-  | Tconstr (path, _, _) -> (
-    match Path.name path with
-    | "Stdlib_ArrayBuffer.t" -> cls "ArrayBuffer"
-    | "Stdlib.BigInt64Array.t" -> cls "BigInt64Array"
-    | "Stdlib.BigUint64Array.t" -> cls "BigUint64Array"
-    | "Stdlib.DataView.t" -> cls "DataView"
-    | "Stdlib_Date.t" -> cls "Date"
-    | "Stdlib.Float32Array.t" -> cls "Float32Array"
-    | "Stdlib.Float64Array.t" -> cls "Float64Array"
-    | "Stdlib.Int16Array.t" -> cls "Int16Array"
-    | "Stdlib.Int32Array.t" -> cls "Int32Array"
-    | "Stdlib.Int8Array.t" -> cls "Int8Array"
-    | "Stdlib_RegExp.t" -> cls "RegExp"
-    | "Stdlib.Uint16Array.t" -> cls "Uint16Array"
-    | "Stdlib.Uint32Array.t" -> cls "Uint32Array"
-    | "Stdlib.Uint8Array.t" -> cls "Uint8Array"
-    | "Stdlib.Uint8ClampedArray.t" -> cls "Uint8ClampedArray"
-    | "Stdlib_File.t" -> cls "File"
-    | "Stdlib_Blob.t" -> cls "Blob"
-    | "Stdlib.Set.t" -> cls "Set"
-    | "Stdlib.Map.t" -> cls "Map"
-    | "Stdlib.WeakSet.t" -> cls "WeakSet"
-    | "Stdlib.WeakMap.t" -> cls "WeakMap"
-    | _ -> None)
-  | _ -> None
-
 let process_tag_name (attrs : Parsetree.attributes) =
   let st = ref None in
   Ext_list.iter attrs (fun ({txt; loc}, payload) ->

compiler/ml/typeopt.ml

@@ -96,15 +96,12 @@ let rec type_cannot_contain_undefined (typ : Types.type_expr) (env : Env.t) =
             && ((not payload_is_unboxed)
                ||
                match cd.cd_args with
-               | Cstr_tuple [t] ->
-                 Ast_untagged_variants.type_is_builtin_object t
-                 || type_cannot_contain_undefined t env
+               | Cstr_tuple [t] -> type_cannot_contain_undefined t env
                | Cstr_tuple [] -> true
                | Cstr_tuple (_ :: _ :: _) ->
                  true (* Not actually possible for an unboxed payload. *)
                | Cstr_record [{ld_type = t}] ->
-                 Ast_untagged_variants.type_is_builtin_object t
-                 || type_cannot_contain_undefined t env
+                 type_cannot_contain_undefined t env
                | Cstr_record ([] | _ :: _ :: _) -> true)
             && all_cases_cannot_contain_undefined (position + 1) rest
         in

compiler/ml/variant_layout.ml

@@ -128,34 +128,26 @@ let get_block_type_from_typ ~env (t : Types.type_expr) : block_type option =
   match declared_block_type ~env t with
   | Some _ as declared -> declared
   | None -> (
-    (* First check the original (unexpanded) type for typed arrays and other instance types *)
-    match type_to_instanceof_backed_obj t with
-    | Some instance_type -> Some (InstanceType instance_type)
-    | None -> (
-      (* If original type didn't match, expand and try standard checks *)
-      let expanded_t = Ctype.expand_head env t in
-      match expanded_t with
-      | {desc = Tconstr (path, _, _)} when Path.same path Predef.path_string ->
-        Some StringType
-      | {desc = Tconstr (path, _, _)} when Path.same path Predef.path_int ->
-        Some IntType
-      | {desc = Tconstr (path, _, _)} when Path.same path Predef.path_float ->
-        Some FloatType
-      | {desc = Tconstr (path, _, _)} when Path.same path Predef.path_bigint ->
-        Some BigintType
-      | {desc = Tconstr (path, _, _)} when Path.same path Predef.path_bool ->
-        Some BooleanType
-      | {desc = Tarrow _} -> Some FunctionType
-      | {desc = Tconstr _} as expanded_t when type_is_builtin_object expanded_t
-        ->
-        Some ObjectType
-      | {desc = Tconstr _} as expanded_t
-        when type_to_instanceof_backed_obj expanded_t |> Option.is_some -> (
-        match type_to_instanceof_backed_obj expanded_t with
-        | None -> None
-        | Some instance_type -> Some (InstanceType instance_type))
-      | {desc = Ttuple _} -> Some (InstanceType Array)
-      | _ -> None))
+    (* Builtin types have no declaration to annotate *)
+    match (Ctype.expand_head env t).desc with
+    | Tconstr (path, _, _) when Path.same path Predef.path_string ->
+      Some StringType
+    | Tconstr (path, _, _) when Path.same path Predef.path_int -> Some IntType
+    | Tconstr (path, _, _) when Path.same path Predef.path_float ->
+      Some FloatType
+    | Tconstr (path, _, _) when Path.same path Predef.path_bigint ->
+      Some BigintType
+    | Tconstr (path, _, _) when Path.same path Predef.path_bool ->
+      Some BooleanType
+    | Tconstr (path, _, _) when Path.same path Predef.path_dict ->
+      Some ObjectType
+    | Tconstr (path, _, _) when Path.same path Predef.path_array ->
+      Some (InstanceType Array)
+    | Tconstr (path, _, _) when Path.same path Predef.path_promise ->
+      Some (InstanceType (Class {name = "Promise"; ancestors = []}))
+    | Ttuple _ -> Some (InstanceType Array)
+    | Tarrow _ -> Some FunctionType
+    | _ -> None)
 
 let get_block_type ~env (cstr : Types.constructor_declaration) : block_type =
   match cstr.cd_args with

tests/build_tests/super_errors/expected/untagged_unannotated_stdlib_name.res.expected

@@ -0,0 +1,10 @@
+
+  We've found a bug for you!
+  /.../fixtures/untagged_unannotated_stdlib_name.res:8:10-28
+
+  6 │ 
+  7 │ @unboxed
+  8 │ type t = Date(Stdlib_Date.t) | Text(string)
+  9 │ 
+
+  This untagged variant definition is invalid: Case Date has a payload that is not of one of the recognized shapes (object, array, etc). Then it must be the only case with payloads.

tests/build_tests/super_errors/fixtures/untagged_unannotated_stdlib_name.res

@@ -0,0 +1,8 @@
+// Payloads are not recognized by their type's name: this unannotated type
+// only shares its path with a stdlib class
+module Stdlib_Date = {
+  type t
+}
+
+@unboxed
+type t = Date(Stdlib_Date.t) | Text(string)

tests/tests/src/untagged_builtin_alias_test.mjs

@@ -0,0 +1,31 @@
+// Generated by ReScript, PLEASE EDIT WITH CARE
+
+import * as Mocha from "mocha";
+import * as Test_utils from "./test_utils.mjs";
+
+function kind(value) {
+  if (Array.isArray(value)) {
+    return "pair";
+  }
+  switch (typeof value) {
+    case "object" :
+      return "entries";
+    case "string" :
+      return value;
+  }
+}
+
+Mocha.describe("Untagged_builtin_alias_test", () => {
+  Mocha.test("aliases of builtin types", () => {
+    Test_utils.eq("File \"untagged_builtin_alias_test.res\", line 21, characters 7-14", "entries", kind({
+      a: 1
+    }));
+    Test_utils.eq("File \"untagged_builtin_alias_test.res\", line 22, characters 7-14", "text", "text");
+    Test_utils.eq("File \"untagged_builtin_alias_test.res\", line 23, characters 7-14", "pair", "pair");
+  });
+});
+
+export {
+  kind,
+}
+/*  Not a pure module */

tests/tests/src/untagged_builtin_alias_test.res

@@ -0,0 +1,25 @@
+open Mocha
+open Test_utils
+
+// Aliases of builtin types are recognized through their expansion
+type entries = dict<int>
+type label = string
+type pair = (int, int)
+
+@unboxed
+type value = Entries(entries) | Label(label) | Pair(pair)
+
+let kind = value =>
+  switch value {
+  | Entries(_) => "entries"
+  | Label(label) => label
+  | Pair(_) => "pair"
+  }
+
+describe(__MODULE__, () => {
+  test("aliases of builtin types", () => {
+    eq(__LOC__, "entries", kind(Entries(dict{"a": 1})))
+    eq(__LOC__, "text", kind(Label("text")))
+    eq(__LOC__, "pair", kind(Pair((1, 2))))
+  })
+})