diff --git a/changes/untagged-no-hardcoded-classes.internal.md b/changes/untagged-no-hardcoded-classes.internal.md new file mode 100644 index 0000000000..73ab3199df --- /dev/null +++ b/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. diff --git a/compiler/ml/UntaggedVariants.md b/compiler/ml/UntaggedVariants.md index f624f69b2b..ddda2f35e3 100644 --- a/compiler/ml/UntaggedVariants.md +++ b/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` 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` is unknown); - `tests/tests/src/untagged_runtime_annotations_test.res`. + `UntaggedUnknown.res` (`list` 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). diff --git a/compiler/ml/ast_untagged_variants.ml b/compiler/ml/ast_untagged_variants.ml index 5c25d8fad1..f87d428d3f 100644 --- a/compiler/ml/ast_untagged_variants.ml +++ b/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) -> diff --git a/compiler/ml/typeopt.ml b/compiler/ml/typeopt.ml index b0bec6b290..d20ab70932 100644 --- a/compiler/ml/typeopt.ml +++ b/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 diff --git a/compiler/ml/variant_layout.ml b/compiler/ml/variant_layout.ml index bfa6976b9a..75652afd39 100644 --- a/compiler/ml/variant_layout.ml +++ b/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 diff --git a/tests/build_tests/super_errors/expected/untagged_unannotated_stdlib_name.res.expected b/tests/build_tests/super_errors/expected/untagged_unannotated_stdlib_name.res.expected new file mode 100644 index 0000000000..e9ca2b50ef --- /dev/null +++ b/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. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/untagged_unannotated_stdlib_name.res b/tests/build_tests/super_errors/fixtures/untagged_unannotated_stdlib_name.res new file mode 100644 index 0000000000..fc12725eff --- /dev/null +++ b/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) diff --git a/tests/tests/src/untagged_builtin_alias_test.mjs b/tests/tests/src/untagged_builtin_alias_test.mjs new file mode 100644 index 0000000000..fbaf95ceb5 --- /dev/null +++ b/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 */ diff --git a/tests/tests/src/untagged_builtin_alias_test.res b/tests/tests/src/untagged_builtin_alias_test.res new file mode 100644 index 0000000000..d5de5e80ee --- /dev/null +++ b/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 +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)))) + }) +})