From 08afc2b4d5a4598ee9cfd1f4302b24d2c0e80107 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 7 Mar 2024 10:22:59 +0100 Subject: [PATCH 1/5] POC print variant runtime repr As in `type t = @as(undefined) A` Triggered in error message: ```res Type declarations do not match: type t = @as(undefined) A is not included in type t = @as(null) A ``` --- jscomp/ml/oprint.ml | 17 +++++++++------- jscomp/ml/outcometree.ml | 5 +++-- jscomp/ml/printtyp.ml | 18 ++++++++++++++-- jscomp/syntax/src/res_outcome_printer.ml | 26 ++++++++++++++++-------- 4 files changed, 47 insertions(+), 19 deletions(-) diff --git a/jscomp/ml/oprint.ml b/jscomp/ml/oprint.ml index a4ee54dd18..4b4c1bd6c9 100644 --- a/jscomp/ml/oprint.ml +++ b/jscomp/ml/oprint.ml @@ -499,13 +499,13 @@ and print_out_signature ppf = match items with Osig_typext(ext, Oext_next) :: items -> gather_extensions - ((ext.oext_name, ext.oext_args, ext.oext_ret_type) :: acc) + ((ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr) :: acc) items | _ -> (List.rev acc, items) in let exts, items = gather_extensions - [(ext.oext_name, ext.oext_args, ext.oext_ret_type)] + [(ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr)] items in let te = @@ -531,7 +531,7 @@ and print_out_sig_item ppf = name !out_class_type clt | Osig_typext (ext, Oext_exception) -> fprintf ppf "@[<2>exception %a@]" - print_out_constr (ext.oext_name, ext.oext_args, ext.oext_ret_type) + print_out_constr (ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr) | Osig_typext (ext, _es) -> print_out_extension_constructor ppf ext | Osig_modtype (name, Omty_abstract) -> @@ -639,7 +639,10 @@ and print_out_type_decl kwd ppf td = print_immediate print_unboxed -and print_out_constr ppf (name, tyl,ret_type_opt) = +and print_out_constr ppf (name, tyl, ret_type_opt, repr) = + let () = match repr with + | None -> () + | Some s -> pp_print_string ppf s in let name = match name with | "::" -> "(::)" (* #7200 *) @@ -686,7 +689,7 @@ and print_out_extension_constructor ppf ext = fprintf ppf "@[type %t +=%s@;<1 2>%a@]" print_extended_type (if ext.oext_private = Asttypes.Private then " private" else "") - print_out_constr (ext.oext_name, ext.oext_args, ext.oext_ret_type) + print_out_constr (ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr) and print_out_type_extension ppf te = let print_extended_type ppf = @@ -736,13 +739,13 @@ let rec print_items ppf = match items with (Osig_typext(ext, Oext_next), None) :: items -> gather_extensions - ((ext.oext_name, ext.oext_args, ext.oext_ret_type) :: acc) + ((ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr) :: acc) items | _ -> (List.rev acc, items) in let exts, items = gather_extensions - [(ext.oext_name, ext.oext_args, ext.oext_ret_type)] + [(ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr)] items in let te = diff --git a/jscomp/ml/outcometree.ml b/jscomp/ml/outcometree.ml index 10bd6535d4..2bad441a29 100644 --- a/jscomp/ml/outcometree.ml +++ b/jscomp/ml/outcometree.ml @@ -63,7 +63,7 @@ type out_type = | Otyp_object of (string * out_type) list * bool option | Otyp_record of (string * bool * bool * out_type) list | Otyp_stuff of string - | Otyp_sum of (string * out_type list * out_type option) list + | Otyp_sum of (string * out_type list * out_type option * string option) list | Otyp_tuple of out_type list | Otyp_var of bool * string | Otyp_variant of @@ -118,11 +118,12 @@ and out_extension_constructor = oext_type_params: string list; oext_args: out_type list; oext_ret_type: out_type option; + oext_repr: string option; oext_private: Asttypes.private_flag } and out_type_extension = { otyext_name: string; otyext_params: string list; - otyext_constructors: (string * out_type list * out_type option) list; + otyext_constructors: (string * out_type list * out_type option * string option) list; otyext_private: Asttypes.private_flag } and out_val_decl = { oval_name: string; diff --git a/jscomp/ml/printtyp.ml b/jscomp/ml/printtyp.ml index 38fe28a7e4..ecfe06b638 100644 --- a/jscomp/ml/printtyp.ml +++ b/jscomp/ml/printtyp.ml @@ -917,16 +917,29 @@ and tree_of_constructor_arguments = function and tree_of_constructor cd = let name = Ident.name cd.cd_id in + let nullary = Ast_untagged_variants.is_nullary_variant cd.cd_args in + let repr = + if not nullary then None + else match Ast_untagged_variants.process_tag_type cd.cd_attributes with + | Some Null -> Some "@as(null)" + | Some Undefined -> Some "@as(undefined)" + | Some (String s) -> Some (Printf.sprintf "@as(%S)" s) + | Some (Int i) -> Some (Printf.sprintf "@as(%d)" i) + | Some (Float f) -> Some (Printf.sprintf "@as(%s)" f) + | Some (Bool b) -> Some (Printf.sprintf "@as(%b)" b) + | Some (BigInt s) -> Some (Printf.sprintf "@as(%sn)" s) + | Some (Untagged _) (* should never happen *) + | None -> None in let arg () = tree_of_constructor_arguments cd.cd_args in match cd.cd_res with - | None -> (name, arg (), None) + | None -> (name, arg (), None, repr) | Some res -> let nm = !names in names := []; let ret = tree_of_typexp false res in let args = arg () in names := nm; - (name, args, Some ret) + (name, args, Some ret, repr) and tree_of_label l = let opt = l.ld_attributes |> List.exists (fun ({txt}, _) -> txt = "ns.optional" || txt = "res.optional") in @@ -982,6 +995,7 @@ let tree_of_extension_constructor id ext es = oext_type_params = ty_params; oext_args = args; oext_ret_type = ret; + oext_repr = None; oext_private = ext.ext_private } in let es = diff --git a/jscomp/syntax/src/res_outcome_printer.ml b/jscomp/syntax/src/res_outcome_printer.ml index 08f260c368..9f77e32bc5 100644 --- a/jscomp/syntax/src/res_outcome_printer.ml +++ b/jscomp/syntax/src/res_outcome_printer.ml @@ -429,8 +429,13 @@ and print_out_constructors_doc constructors = constructors); ])) -and print_out_constructor_doc (name, args, gadt) = - let gadt_doc = +and print_out_constructor_doc (name, args, gadt, repr) = + let reprDoc = + match repr with + | None -> Doc.nil + | Some s -> Doc.text (s ^ " ") + in + let gadtDoc = match gadt with | Some out_type -> Doc.concat [Doc.text ": "; print_out_type_doc out_type] | None -> Doc.nil @@ -469,7 +474,7 @@ and print_out_constructor_doc (name, args, gadt) = Doc.rparen; ]) in - Doc.group (Doc.concat [Doc.text name; args_doc; gadt_doc]) + Doc.group (Doc.concat [reprDoc; Doc.text name; args_doc; gadtDoc]) and print_record_decl_row_doc (name, mut, opt, arg) = Doc.group @@ -758,13 +763,14 @@ and print_out_signature_doc (signature : Outcometree.out_sig_item list) = match items with | Outcometree.Osig_typext (ext, Oext_next) :: items -> gather_extensions - ((ext.oext_name, ext.oext_args, ext.oext_ret_type) :: acc) + ((ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr) + :: acc) items | _ -> (List.rev acc, items) in let exts, items = gather_extensions - [(ext.oext_name, ext.oext_args, ext.oext_ret_type)] + [(ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr)] items in let te = @@ -822,7 +828,10 @@ and print_out_extension_constructor_doc (if out_ext.oext_private = Asttypes.Private then Doc.text "private " else Doc.nil); print_out_constructor_doc - (out_ext.oext_name, out_ext.oext_args, out_ext.oext_ret_type); + ( out_ext.oext_name, + out_ext.oext_args, + out_ext.oext_ret_type, + out_ext.oext_repr ); ]) and print_out_type_extension_doc @@ -1035,13 +1044,14 @@ let print_out_phrase_signature signature = match items with | (Outcometree.Osig_typext (ext, Oext_next), None) :: items -> gather_extensions - ((ext.oext_name, ext.oext_args, ext.oext_ret_type) :: acc) + ((ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr) + :: acc) items | _ -> (List.rev acc, items) in let exts, signature = gather_extensions - [(ext.oext_name, ext.oext_args, ext.oext_ret_type)] + [(ext.oext_name, ext.oext_args, ext.oext_ret_type, ext.oext_repr)] signature in let te = From 1ad348238194bddaf3f7e7dd514fe3a8cc66f057 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 13 Jun 2024 11:08:18 +0200 Subject: [PATCH 2/5] Print @unboxed for variants and check inclusion correctly. - Print `@unboxed` in the variant type declaration in the outcome printer. - Fix issue where attributes such as `@unboxed` were printed twice. - Fix issue where inconsistency in `@unboxed` in variant declarations between implementation and interface was not chedked. --- jscomp/ml/includecore.ml | 6 ++++-- jscomp/ml/printtyp.ml | 4 +++- jscomp/syntax/src/res_outcome_printer.ml | 1 - 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/jscomp/ml/includecore.ml b/jscomp/ml/includecore.ml index af4515be1b..d68d0546eb 100644 --- a/jscomp/ml/includecore.ml +++ b/jscomp/ml/includecore.ml @@ -328,8 +328,10 @@ let type_declarations ?(equality = false) ~loc env name decl1 id decl2 = in if err <> [] then err else let err = - match (decl2.type_kind, decl1.type_unboxed.unboxed, - decl2.type_unboxed.unboxed) with + let untagged1 = Ast_untagged_variants.process_untagged decl1.type_attributes in + let untagged2 = Ast_untagged_variants.process_untagged decl2.type_attributes in + match (decl2.type_kind, decl1.type_unboxed.unboxed || untagged1, + decl2.type_unboxed.unboxed || untagged2) with | Type_abstract, _, _ -> [] | _, true, false -> [Unboxed_representation false] | _, false, true -> [Unboxed_representation true] diff --git a/jscomp/ml/printtyp.ml b/jscomp/ml/printtyp.ml index ecfe06b638..7f89f03766 100644 --- a/jscomp/ml/printtyp.ml +++ b/jscomp/ml/printtyp.ml @@ -881,6 +881,7 @@ let rec tree_of_type_decl id decl = in let (name, args) = type_defined decl in let constraints = tree_of_constraints params in + let untagged = ref false in let ty, priv = match decl.type_kind with | Type_abstract -> @@ -890,6 +891,7 @@ let rec tree_of_type_decl id decl = tree_of_typexp false ty, decl.type_private end | Type_variant cstrs -> + untagged := Ast_untagged_variants.process_untagged decl.type_attributes; tree_of_manifest (Otyp_sum (List.map tree_of_constructor cstrs)), decl.type_private | Type_record(lbls, _rep) -> @@ -907,7 +909,7 @@ let rec tree_of_type_decl id decl = otype_type = ty; otype_private = priv; otype_immediate = immediate; - otype_unboxed = decl.type_unboxed.unboxed; + otype_unboxed = decl.type_unboxed.unboxed || !untagged; otype_cstrs = constraints ; } diff --git a/jscomp/syntax/src/res_outcome_printer.ml b/jscomp/syntax/src/res_outcome_printer.ml index 9f77e32bc5..c734e3d3b2 100644 --- a/jscomp/syntax/src/res_outcome_printer.ml +++ b/jscomp/syntax/src/res_outcome_printer.ml @@ -683,7 +683,6 @@ let rec print_out_sig_item_doc ?(print_name_as_is = false) Doc.group (Doc.concat [ - attrs; kw; (if print_name_as_is then Doc.text out_type_decl.otype_name else From e60731bc3bb3f2eaaeea17939f71f3cbba6c7bd9 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 13 Jun 2024 11:14:26 +0200 Subject: [PATCH 3/5] snake case --- jscomp/syntax/src/res_outcome_printer.ml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/jscomp/syntax/src/res_outcome_printer.ml b/jscomp/syntax/src/res_outcome_printer.ml index c734e3d3b2..f087359ff8 100644 --- a/jscomp/syntax/src/res_outcome_printer.ml +++ b/jscomp/syntax/src/res_outcome_printer.ml @@ -430,12 +430,12 @@ and print_out_constructors_doc constructors = ])) and print_out_constructor_doc (name, args, gadt, repr) = - let reprDoc = + let repr_doc = match repr with | None -> Doc.nil | Some s -> Doc.text (s ^ " ") in - let gadtDoc = + let gadt_doc = match gadt with | Some out_type -> Doc.concat [Doc.text ": "; print_out_type_doc out_type] | None -> Doc.nil @@ -474,7 +474,7 @@ and print_out_constructor_doc (name, args, gadt, repr) = Doc.rparen; ]) in - Doc.group (Doc.concat [reprDoc; Doc.text name; args_doc; gadtDoc]) + Doc.group (Doc.concat [repr_doc; Doc.text name; args_doc; gadt_doc]) and print_record_decl_row_doc (name, mut, opt, arg) = Doc.group From aae73e71d3722b371b18d598c46875752e92c542 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Thu, 13 Jun 2024 16:45:14 +0200 Subject: [PATCH 4/5] Changelog and test. --- CHANGELOG.md | 1 + .../expected/UntaggedImplIntf.res.expected | 29 +++++++++++++++++++ .../fixtures/UntaggedImplIntf.res | 5 ++++ 3 files changed, 35 insertions(+) create mode 100644 jscomp/build_tests/super_errors/expected/UntaggedImplIntf.res.expected create mode 100644 jscomp/build_tests/super_errors/fixtures/UntaggedImplIntf.res diff --git a/CHANGELOG.md b/CHANGELOG.md index 67de4a6fb5..f6fb91b5fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ - Reactivate unused attribute check for `@int`. https://github.com/rescript-lang/rescript-compiler/pull/6802 - Fix issue where optional labels were not taken into account when disambiguating record value construction. https://github.com/rescript-lang/rescript-compiler/pull/6798 - Fix issue in gentype when type `Jsx.element` surfaces to the user. https://github.com/rescript-lang/rescript-compiler/pull/6808 +- Fix inclusion check (impl vs interface) for untagged variants, and fix the outcome printer to show tags. https://github.com/rescript-lang/rescript-compiler/pull/6669 #### :house: Internal diff --git a/jscomp/build_tests/super_errors/expected/UntaggedImplIntf.res.expected b/jscomp/build_tests/super_errors/expected/UntaggedImplIntf.res.expected new file mode 100644 index 0000000000..ffdcb9713f --- /dev/null +++ b/jscomp/build_tests/super_errors/expected/UntaggedImplIntf.res.expected @@ -0,0 +1,29 @@ + + We've found a bug for you! + /.../fixtures/UntaggedImplIntf.res:3:5-5:1 + + 1 │ module M: { + 2 │ @unboxed type t = | @as(null) A + 3 │ } = { + 4 │  type t = | @as(null) A + 5 │ } + + Signature mismatch: + Modules do not match: + { + type t = @as(null) A +} + is not included in + { + @unboxed type t = @as(null) A +} + Type declarations do not match: + type t = @as(null) A + is not included in + @unboxed type t = @as(null) A + /.../fixtures/UntaggedImplIntf.res:2:12-33: + Expected declaration + /.../fixtures/UntaggedImplIntf.res:4:3-24: + Actual declaration + Their internal representations differ: + the second declaration uses unboxed representation. \ No newline at end of file diff --git a/jscomp/build_tests/super_errors/fixtures/UntaggedImplIntf.res b/jscomp/build_tests/super_errors/fixtures/UntaggedImplIntf.res new file mode 100644 index 0000000000..6d269fedf6 --- /dev/null +++ b/jscomp/build_tests/super_errors/fixtures/UntaggedImplIntf.res @@ -0,0 +1,5 @@ +module M: { + @unboxed type t = | @as(null) A +} = { + type t = | @as(null) A +} \ No newline at end of file From ddd7a2b8d42abfa174e504bad92c6402e17791ec Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Fri, 14 Jun 2024 13:21:57 +0200 Subject: [PATCH 5/5] Bump rescript-core for playground bundling. --- .../playground-bundling/package-lock.json | 28 +++++++++---------- packages/playground-bundling/package.json | 2 +- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/playground-bundling/package-lock.json b/packages/playground-bundling/package-lock.json index 21da087299..505773afca 100644 --- a/packages/playground-bundling/package-lock.json +++ b/packages/playground-bundling/package-lock.json @@ -9,16 +9,16 @@ "version": "1.0.0", "license": "ISC", "dependencies": { - "@rescript/core": "^1.1.0", + "@rescript/core": "^1.5.0", "@rescript/react": "^0.12.1" } }, "node_modules/@rescript/core": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/@rescript/core/-/core-1.1.0.tgz", - "integrity": "sha512-pz/CL8+9hBUTeMpUouvZohNsa5rqIwurlXoa1CZWN0ZKuWjMVjaoQ3V+0NB72J/QBbs6/8W82VABKBaDn3fGCA==", + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/@rescript/core/-/core-1.5.0.tgz", + "integrity": "sha512-XTKtpMfqCF5qmYWrdTJjhy2lDIWwwlOFgI5Vo5eB/kzRWXvM9R29ZpMoVXbn0fURl+uF3GvFp2/CbZlF3wt1Zg==", "peerDependencies": { - "rescript": ">=11.0.0 || ^11.1.0-rc.2" + "rescript": "^11.1.0-rc.7" } }, "node_modules/@rescript/react": { @@ -74,9 +74,9 @@ } }, "node_modules/rescript": { - "version": "11.0.1", - "resolved": "https://registry.npmjs.org/rescript/-/rescript-11.0.1.tgz", - "integrity": "sha512-7T4PRp/d0+CBNnY6PYKffFqo9tGZlvnZpboF/n+8SKS+JZ6VvXJO7W538VPZXf3EYx1COGAWWvkF9e/HgSAqHg==", + "version": "11.1.1", + "resolved": "https://registry.npmjs.org/rescript/-/rescript-11.1.1.tgz", + "integrity": "sha512-FMELeoiR1n3LzBdBt+k7U4l0vsz5Xh0HBSHf+0NhyhzZkMRLkEKEDNrcqZc6RIux9bxmxoO+zEa9qFM01VOXAw==", "hasInstallScript": true, "peer": true, "bin": { @@ -100,9 +100,9 @@ }, "dependencies": { "@rescript/core": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/@rescript/core/-/core-1.1.0.tgz", - "integrity": "sha512-pz/CL8+9hBUTeMpUouvZohNsa5rqIwurlXoa1CZWN0ZKuWjMVjaoQ3V+0NB72J/QBbs6/8W82VABKBaDn3fGCA==", + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/@rescript/core/-/core-1.5.0.tgz", + "integrity": "sha512-XTKtpMfqCF5qmYWrdTJjhy2lDIWwwlOFgI5Vo5eB/kzRWXvM9R29ZpMoVXbn0fURl+uF3GvFp2/CbZlF3wt1Zg==", "requires": {} }, "@rescript/react": { @@ -146,9 +146,9 @@ } }, "rescript": { - "version": "11.0.1", - "resolved": "https://registry.npmjs.org/rescript/-/rescript-11.0.1.tgz", - "integrity": "sha512-7T4PRp/d0+CBNnY6PYKffFqo9tGZlvnZpboF/n+8SKS+JZ6VvXJO7W538VPZXf3EYx1COGAWWvkF9e/HgSAqHg==", + "version": "11.1.1", + "resolved": "https://registry.npmjs.org/rescript/-/rescript-11.1.1.tgz", + "integrity": "sha512-FMELeoiR1n3LzBdBt+k7U4l0vsz5Xh0HBSHf+0NhyhzZkMRLkEKEDNrcqZc6RIux9bxmxoO+zEa9qFM01VOXAw==", "peer": true }, "scheduler": { diff --git a/packages/playground-bundling/package.json b/packages/playground-bundling/package.json index 879dfa91f7..0456bf38cc 100644 --- a/packages/playground-bundling/package.json +++ b/packages/playground-bundling/package.json @@ -11,7 +11,7 @@ "author": "", "license": "ISC", "dependencies": { - "@rescript/core": "^1.1.0", + "@rescript/core": "^1.5.0", "@rescript/react": "^0.12.1" } }