Add Prim.Int.ToString compiler-solved class#4267
Add Prim.Int.ToString compiler-solved class#4267JordanMartinez merged 6 commits intopurescript:masterfrom JordanMartinez:addIntToString
Conversation
purefunctor
left a comment
There was a problem hiding this comment.
This LGTM, but would there be any merit to a FromString type class as well?
Similarly, would generalizing ToString/FromString such that they can be used for Boolean/Ordering be a good idea? I figured not really, considering that unlike Int they don't have an arbitrary number of members.
I am not sure. For one, if it's needed, we can add it later without making a breaking change. For two, I'm not sure what situations may arise where it would be desirable to do that. Also, a |
|
Isn't |
Yes. |
|
I've spent some time on updating this PR. However, there's a number of factors to consider. I'm going to open an issue where this can be discussed further. |
|
Ha, there's already an issue. I'll further discuss there. |
|
Per the discussion in #4263, I believe this is ready for a final review. |
rhendric
left a comment
There was a problem hiding this comment.
Everything here looks solid, but I'd like to see the following tested (could be the same test):
- A case where the integer being
ToString'd is not a literal—useAddorMulto compute an integer that doesn't appear in the source, and thenToStringthat. - A case where the integer is big (larger than 2^64), given the initial motivation for this class.
|
On it! |
|
|
Description of the change
Fixes #4263. I looked through #4207 to guide me on how to implement this.
Checklist: