Skip to content

Workaround for @asin labels in uncurried externals. #6527

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.

Sign up for GitHub

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
Dec 14, 2023
Merged

Workaround for @asin labels in uncurried externals. #6527

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
58b3f67
Workaround for `@as`in labels in uncurried externals.
cristianoc Dec 13, 2023
1fabd25
Example with 2 default args.
cristianoc Dec 14, 2023
ca5cdc9
Add example outside externals, which should not fail.
cristianoc Dec 14, 2023
db3dfd3
Restrict workaround to only fire inside an external.
cristianoc Dec 14, 2023
bd5b2cd
Merge branch 'master' into as_uncurried_externals
cristianoc Dec 14, 2023
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
15 changes: 15 additions & 0 deletions jscomp/syntax/src/res_core.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4276,6 +4276,21 @@ and parseEs6ArrowType ~attrs p =
p.uncurried_config |> Res_uncurried.fromDotted ~dotted
in
let loc = mkLoc startPos endPos in
let arity =
(* Workaround for ~lbl: @as(json`false`) _, which changes the arity *)
match argLbl with
| Labelled _s ->
let typ_is_any =
match typ.ptyp_desc with
| Ptyp_any -> true
| _ -> false
in
let has_as =
Ext_list.exists typ.ptyp_attributes (fun (x, _) -> x.txt = "as")
in
if typ_is_any && has_as then arity - 1 else arity
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there can be multiple @as() _, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is inside a fold for each argument.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is approximate: does not check that we're inside an external.
The same construction inside a normal type annotation which is not an external should have no effect (in that the rest of the compiler ignores it).

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to deprecate this whole construction in future.
Kind of hesitant to go for this workaround, unless it's really needed to unbreak something.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good stuff!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah we should get rid of this eventually. But for v11 we should have it imo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK I've guarded it to only apply inside externals, so it won't accidentally break in the corner case where the annotations are used outside externals by mistake.

| _ -> arity
in
let tArg = Ast_helper.Typ.arrow ~loc ~attrs argLbl typ t in
if uncurried && (paramNum = 1 || p.uncurried_config = Legacy) then
(paramNum - 1, Ast_uncurried.uncurriedType ~loc ~arity tArg, 1)
Expand Down
19 changes: 19 additions & 0 deletions jscomp/test/AsInUncurriedExternals.js
View file
Open in desktop

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

12 changes: 12 additions & 0 deletions jscomp/test/AsInUncurriedExternals.res
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
@@uncurried

@obj
external makeOptions: (
~objectMode: @as(json`false`) _,
~name: string,
unit,
) => int = ""

let mo = makeOptions

let options = mo(~name="foo", ())
3 changes: 2 additions & 1 deletion jscomp/test/build.ninja
View file
Open in desktop

Large diffs are not rendered by default.