Skip to content
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.

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

chore(stdlib)!: Replace parseInt error strings with structured error enum #1755

Merged
merged 10 commits into from Mar 4, 2024

Conversation

spotandjake
Copy link
Member

@spotandjake spotandjake commented Mar 17, 2023

This makes parseInt use an exception in the err case.

On a note though until #1053 is fixed I dont know if it makes sense to make this change as you lose the ability to pattern match on the individual cases without importing the exceptions directly from the runtime which we dont want users doing.

That could still make it in before 0.6 so this pr is up for that and also just in case we want to move forward on this anyways.

This has been blocked by: #1758 or #1759 either one would work to get this moving.

Closes: #1057

@spotandjake
Copy link
Member Author

This is now blocked on #1886

@phated phated removed the blocked label Jan 29, 2024
@phated
Copy link
Member

phated commented Jan 29, 2024

@spotandjake is this unblocked now that the exception re-export PR has been merged?

@spotandjake
Copy link
Member Author

@spotandjake is this unblocked now that the exception re-export PR has been merged?

No this is still blocked. We had decided on a conversation in discord, to use an enum instead of an exception.

#1886 is what is blocking this at the moment.

@phated
Copy link
Member

phated commented Jan 31, 2024

@spotandjake I don't remember that conversation exactly. We don't really want many enums in pervasives because they can cause ambiguous type inference with user code.

@spotandjake
Copy link
Member Author

@spotandjake I don't remember that conversation exactly. We don't really want many enums in pervasives because they can cause ambiguous type inference with user code.

Switched back to an exception, looks like that did fix the problem I was running into. One note graindoc doesnt seem to be generating documentation for provided exceptions.

@phated phated added blocked and removed blocked labels Feb 29, 2024
@spotandjake spotandjake force-pushed the spotandjake-parseInt-exception branch from 4f9a822 to ff26f8d Compare March 3, 2024 01:27
compiler/test/stdlib/number.test.gr Show resolved Hide resolved
stdlib/number.gr Outdated Show resolved Hide resolved
stdlib/number.gr Outdated Show resolved Hide resolved
@spotandjake
Copy link
Member Author

When trying to graindoc this I get the error Failure("NYI: Otyp_manifest pretty-printer"), so the graindoc needs to be updated.

Copy link
Member Author

@spotandjake spotandjake left a comment

Choose a reason for hiding this comment

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

This has been switched to an enum now.

compiler/src/typed/oprint.re Outdated Show resolved Hide resolved
stdlib/number.md Outdated Show resolved Hide resolved
@phated phated changed the title chore(stdlib)!: Make parseInt use an exception in the err case chore(stdlib)!: Replace parseInt error strings with structured error enum Mar 4, 2024
@spotandjake spotandjake force-pushed the spotandjake-parseInt-exception branch from 100e879 to 1090422 Compare March 4, 2024 02:51
@phated phated enabled auto-merge March 4, 2024 03:22
@phated phated added this pull request to the merge queue Mar 4, 2024
Merged via the queue into grain-lang:main with commit ea26d18 Mar 4, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Use exceptions in the Err case of Number.parseInt
3 participants