Skip to content

chore(compiler): Prefer option over exception Not_found where possible in compiler helpers.#2349

Open
spotandjake wants to merge 1 commit intograin-lang:mainfrom
spotandjake:spotandjake/compiler-exn-to-opts
Open

chore(compiler): Prefer option over exception Not_found where possible in compiler helpers.#2349
spotandjake wants to merge 1 commit intograin-lang:mainfrom
spotandjake:spotandjake/compiler-exn-to-opts

Conversation

@spotandjake
Copy link
Member

I've noticed that a lot of the crashes in the lsp and compiler tend to relate back to Not_found exceptions which bubble up from the Env and other helper functions.

This pr does some refactoring to begin moving us over to using options rather than exceptions which helps to keep error handling more localized and make missing id's more clear.

I haven't fully refactored everything as this pr is already getting rather large and I wanted to get opinions on this change before going any further.

This probably makes sense to merge after we merge wasm-gc to make the rebasing of that pr easier. If we don't like this change i'm happy to close this pr as well.

Longer term I would like to have all of our helper functions return either options or result types and raise only to be used for actual user facing compiler errors.

I've noticed that a lot of the crashes in the lsp and compiler tend to relate back to `Not_found` exceptions which bubble up from the `Env` and other helper functions.

This pr does some refactoring to begin moving us over to using options rather than exceptions which helps to keep error handling more localized and make missing id's more clear.

I haven't fully refactored everything as this pr is already getting rather large and I wanted to get opinions on this change before going any further.

This probably makes sense to merge after we merge wasm-gc to make the rebasing of that pr easier. If we don't like this change i'm happy to close this pr as well.

Longer term I would like to have all of our helper functions return either options or result types and `raise` only to be used for actual user facing compiler errors.
Copy link
Member

@ospencer ospencer left a comment

Choose a reason for hiding this comment

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

Overall, this looks really good. My main piece of feedback is that for constructs that we define, we drop the _opt ending. So long as we consistently return options, that ending is redundant. (Plus, the _opt to me implies that there's a version that will raise instead, and we shouldn't have those.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants