@unboxed fails if a constructor has more than one argument

[glennsl]

@unboxed
type t = Tuple(int, string)

fails with the error:

  This type cannot be unboxed because
  its constructor has more than one argument.

which is wrong, because it does produce the expected output if I add just a few more parens, making it an explicit tuple which should be equivalent.

@unboxed
type t = Tuple((int, string))

[glennsl]

Also, if I add another constructor to the first definition above:

@unboxed
type t = Tuple(int, string) | Float(float)

the error turns into

  This type cannot be unboxed because it has more than one constructor.

which is clearly incorrect since this does work:

@unboxed
type t = Float(float) | String(string)

[glennsl]

And if I add another constructor to the second definition above:

@unboxed
type t = Tuple((float, string)) | Float(float)

the error now becomes:

  This untagged variant definition is invalid: An unknown case must be the only case with payloads.

which I have no idea what's even supposed to mean.

[cristianoc]

```rescript
@unboxed
type t = Tuple(int, string)

fails with the error:
This type cannot be unboxed because
its constructor has more than one argument.

which is wrong, because it does produce the expected output if I add just a few more parens, making it an explicit tuple which should be equivalent.

@unboxed
type t = Tuple((int, string))

That's a different type that's why it works.
It's not so obvious, but variants payloads have arity in the language.
That's true in OCaml too though you never see it as you don't get to see the runtime representation of things.

[cristianoc]

Closing this optimistically.
Please reopen if there's more to discuss or some idea for better error messages/design etc.

[glennsl]

The second error message seems plain wrong. If the first case is intentionally unsupported, then it should emit the first error message in the second case as well. I don't know if the second error message should ever be emitted.

In the third case I don't understand the error message, or why it would error at all. If I understood that, I could perhaps suggest a better error message.

[cristianoc]

I guess this would also be confusing?

https://rescript-lang.org/try?version=v10.1.2&code=LYewJgrgNgpgBAWTgLjgbwFBzgFwJ4AO8OcAvHAIIAUAlgHY4A0c9OAlBgL5npa6HEe1Wg2as2HThiA

The analogous OCaml example will give an analogous error.
Except, the OCaml syntax uses * which makes the issues less mysterious.

[glennsl]

No, I think it's clear that those are different. What I find confusing about the reported issue is that it's clearly possible to represent a constructor with multiple arguments as a tuple, because it's possible to represent a constructor with a single argument that is explicitly a tuple. There doesn't seem to be a technical reason (on the surface at least) for not allowing it.

But even if this is as designed, the two other errors seem wrong.

[cristianoc]

A((int,int)) lets me store an object [1,2] without losing the identity.
A(int, int) forces you to take a copy.
So in principle one could silently transform it to a tuple, but I'm it seems strictly less flexible.
In any case, the current implementation only supports cases with 1 argument.
The error message tries to simply state that.
Perhaps "cannot be unboxed" is not the best phrasing, as the idea is to communicate a conscious restriction, rather than a theoretical impossibility of some other implementation choice.

[glennsl]

A((int,int)) lets me store an object [1,2] without losing the identity.
A(int, int) forces you to take a copy.
So in principle one could silently transform it to a tuple, but I'm it seems strictly less flexible.

From a user perspective, not doing so forces one to deal with unnecessary parentheses. Having the choice of whether or not to include those seems more flexible to me, not less.

As a motivating example, what I wanted to do here is to model a GeoJSON position, which can be either 2-dimensional or 3-dimensional. The natural rescript type for this is:

type t = LatLng(float, float) | LatLngAlt(float, float, float)

But this cannot be @unboxed, as already established. Neither can

type t = LatLng((float, float)) | LatLngAlt((float, float, float))

Because it causes one of the other confusing errors originally reported. It probably wouldn't be able to distinguish between them anyway, and also has these unnecessary and "unnatural" extra parenthesis. Constructors of different arity could on the other hand be distinguished by their arity.

In any case, the current implementation only supports cases with 1 argument.
The error message tries to simply state that.
Perhaps "cannot be unboxed" is not the best phrasing, as the idea is to communicate a conscious restriction, rather than a theoretical impossibility of some other implementation choice.

I think the error message is clear enough, although it could be improved by suggesting that it's made an explicit tuple (i.e. Perhaps you meant Tuple((int, string))``). It just seems like an unnecessary restriction (from a user POV). The other errors are much more problematic, and I think ought to be addressed regardless of whether this restriction is lifted.

[cristianoc]

Leaving open for better error messages / documentation.

1. for unboxing cases with more than 1 argument, that could be considered in future, when multiple objects of different shape can be accepted

2. "more than one constructor" should be improved. It should give the same error message as 1.

3. unknown should be explained

There are literal and block cases. Block cases belong to the following categories. Single payload of type

• Int
• String
• Float
• Array
• Function
• Object

And:

• Unknown

Where Unknown represents: none of the above. The tuple cases ends up in the unknown case as it's none of the above.

This won't fit all in one error message, but a combination of better error message and link to documentation on the web site, should clean things up.
So yes the documentation has fallen a bit behind on this feature at the moment.

[cristianoc]

See rescript-lang/rescript-lang.org#697

[cristianoc]

@glennsl see the PR:
#6290

How do the error messages look now?