Skip to content

Print variant runtime repr in error messages, and fix inclusion check. #6669

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jun 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions jscomp/build_tests/super_errors/fixtures/UntaggedImplIntf.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module M: {
@unboxed type t = | @as(null) A
} = {
type t = | @as(null) A
}
6 changes: 4 additions & 2 deletions jscomp/ml/includecore.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
17 changes: 10 additions & 7 deletions jscomp/ml/oprint.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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) ->
Expand Down Expand Up @@ -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 *)
Expand Down Expand Up @@ -686,7 +689,7 @@ and print_out_extension_constructor ppf ext =
fprintf ppf "@[<hv 2>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 =
Expand Down Expand Up @@ -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 =
Expand Down
5 changes: 3 additions & 2 deletions jscomp/ml/outcometree.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
22 changes: 19 additions & 3 deletions jscomp/ml/printtyp.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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 ->
Expand All @@ -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) ->
Expand All @@ -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 ;
}

Expand All @@ -917,16 +919,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
Expand Down Expand Up @@ -982,6 +997,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 =
Expand Down
25 changes: 17 additions & 8 deletions jscomp/syntax/src/res_outcome_printer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,12 @@ and print_out_constructors_doc constructors =
constructors);
]))

and print_out_constructor_doc (name, args, gadt) =
and print_out_constructor_doc (name, args, gadt, repr) =
let repr_doc =
match repr with
| None -> Doc.nil
| Some s -> Doc.text (s ^ " ")
in
let gadt_doc =
match gadt with
| Some out_type -> Doc.concat [Doc.text ": "; print_out_type_doc out_type]
Expand Down Expand Up @@ -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 [repr_doc; Doc.text name; args_doc; gadt_doc])

and print_record_decl_row_doc (name, mut, opt, arg) =
Doc.group
Expand Down Expand Up @@ -678,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
Expand Down Expand Up @@ -758,13 +762,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 =
Expand Down Expand Up @@ -822,7 +827,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
Expand Down Expand Up @@ -1035,13 +1043,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 =
Expand Down
28 changes: 14 additions & 14 deletions packages/playground-bundling/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/playground-bundling/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"author": "",
"license": "ISC",
"dependencies": {
"@rescript/core": "^1.1.0",
"@rescript/core": "^1.5.0",
"@rescript/react": "^0.12.1"
}
}