Skip to content
This repository has been archived by the owner on Jun 15, 2023. It is now read-only.

Numeric poly var support #340

Closed
bobzhang opened this issue Mar 24, 2021 · 12 comments
Closed

Numeric poly var support #340

bobzhang opened this issue Mar 24, 2021 · 12 comments
Labels

Comments

@bobzhang
Copy link
Member

Note we are planning to provide numeric poly var support so that

#"1" is going to be compiled as 1 instead of "1"

Ideally, we should allow user to write #1 directly since it is compiled to 1 instead of "1"
Some more context are provided here: rescript-lang/rescript-compiler#5026

For the formal rules it would be:

# 0
# [1-9][0-9]*

Also an extra overflow check is needed

@gaku-sei
Copy link

Sorry @bobzhang if #1 is accepted in the future and compiles to 1 can we keep #"1" to compile to "1" (even though I understand it's an escape hatch it's a neat one ^^)?
There are several reasons for that, but the main one is backward compatibility (actually we do use #"number" here and there already 😁).

I wonder, why will numeric variants be compiled to numbers ?

@bobzhang
Copy link
Member Author

@gaku-sei it is to provide good interop with JS API which expects enums or return enums. There is currently no easy way to do this without a conversion; and it is a pain point for production users.
What's your use case for making #"1" to "1"?

@IwanKaramazow
Copy link
Contributor

@bobzhang is there a reason we restrict this to non-float numbers? (I'll document this for posteriority)

@bobzhang
Copy link
Member Author

@IwanKaramazow the main use case is for passing and receiving enums.

This is is the quote from some users:

i think positive integers covers 99% of our use cases

Float can be a bit tricky, since 0.33 may not be representable in IEEE 754.
FYI: https://github.com/rescript-lang/rescript-compiler/blob/master/jscomp/ext/ext_string.ml#L510 this is what we used to determine if it is the hash number.

Things could be done in the front-end:

  • support #1
  • gives an error for #"1" since this can be confusing
  • detect int overflow

In the future, we could support things like #0x01 (the hex format) which should be in no conflict with what we have right now. Note this addition is also a good example to demonstrate the value of rescript syntax.

@IwanKaramazow
Copy link
Contributor

Thanks for the background, will implement.

@gaku-sei
Copy link

There is currently no easy way to do this without a conversion; and it is a pain point for production users.

Makes actually, absolutely makes sense.

What's your use case for making #"1" to "1"?

We use to interface with tailwind and concat the poly variant with some other string. In that case the new syntax happens to work, but I was concerned that some cases wouldn't work (hence the idea to differentiate #"1" and #1 as it seems they cover 2 different use cases 😕

@bobzhang
Copy link
Member Author

@gaku-sei I think this still works since you mentioned concatenation.

1+"px" === "1" + "px"

@chenglou
Copy link
Member

chenglou commented Mar 26, 2021

Yeah kinda agree that maybe #"1" should be supported. Otherwise a bit inconsistent

@bobzhang
Copy link
Member Author

We could support #"1" syntactically, but it will be encoded as 1, so it may bring confusion instead of consistency.
Note for structural types, the encoding has to be simple and consistent.

@chenglou
Copy link
Member

Oh, if #"1" is encoded as 1 then nevermind. Would rather have a good error message or formatted format to #1. But I was saying that it was confusing to not support #"1" as "1"

@cannorin
Copy link

cannorin commented Mar 11, 2022

i think positive integers covers 99% of our use cases

I'm trying to create a tool to generate ReScript bindings from TypeScript type definitions (.d.ts), and I've been encountering so many negative integers, even in popular packages:

It seems people often use 1 | -1 to indicate left/right or positive/negative, 1 | 0 | -1 to represent larger/equal/smaller, and -1, -2, ... for special/erroneous cases, which should be better represented as type rather than making it just int.

Also, it kind of contradicts with the fact @deriving(jsConverter) for non-poly variant supports negative values.

@deriving(jsConverter) can be used as a workaround, but it introduces an unnecessary variant type each time something like 1 | 0 | -1 is used.

@stale
Copy link

stale bot commented May 28, 2023

The rescript-lang/syntax repo is obsolete and will be archived soon. If this issue is still relevant, please reopen in the compiler repo (https://github.com/rescript-lang/rescript-compiler) or comment here to ask for it to be moved. Thank you for your contributions.

@stale stale bot added the stale label May 28, 2023
@stale stale bot closed this as completed Jun 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants