Skip to content

Conversation

chenglou
Copy link
Member

@chenglou chenglou commented Feb 10, 2021

Depends on #4940

Before 😅:
Screen Shot 2021-02-10 at 3 40 06 AM

After:
image

Fixes rescript-lang#4939

The previous message was also wrong (?). The types were flipped; if the function expected 1 and got 2, it'd say "this function has arity2 but was expected arity1".
_,
(Pdot (Pdot ((Pident {name = "Js"}), "Fn", _), arityB, _))
)]
) ->
Copy link
Member Author

Choose a reason for hiding this comment

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

This pattern is a bit unclear to me. Here's the original: https://github.com/rescript-lang/ocaml/blob/bc2625fe99e2e01700d1ec0fea8dcc0672c94564/typing/typecore.ml#L5011

tp is a tuple of Path.t, and in my test here, both items are the same. tpl is a list of such tuple, and in my test here, a list with a single item of a tuple.

From what I understand, the second item of the tuple is the expanded path... right? So I should be matching on that and ignoring the first item?

Copy link
Member

Choose a reason for hiding this comment

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

For the record:

fun[@bs] a0 a1 -> a0 + a1  

is expanded into

{Js.Fn.I2 = fun a0 a1 -> a0 + a1}

it does not come with an annotation like below:

({Js.Fn.I2 = fun a0 a1 -> a0 + a1} : _ Js.Fn.arity2)

With such annotation, the error message is better.
However, this is intentional, since adding such annotation will make this

data -> mapU (fun [@bs] x -> x.label) 

not work

Copy link
Member

Choose a reason for hiding this comment

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

To answer your question, tp is its type and expanded type (e.g, type aliases)
The changes are mostly correct, I will merge it and do some tweak later

@bobzhang
Copy link
Member

the two changes are great, thanks!

@bobzhang bobzhang merged commit 8f3c563 into rescript-lang:master Feb 18, 2021
@chenglou chenglou deleted the more-arity branch February 18, 2021 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants