Skip to content

Encapsulate local exception usage#1544

Merged
WardBrian merged 9 commits intomasterfrom
cleanup-local-exception-usage
Sep 10, 2025
Merged

Encapsulate local exception usage#1544
WardBrian merged 9 commits intomasterfrom
cleanup-local-exception-usage

Conversation

@WardBrian
Copy link
Copy Markdown
Member

There are a couple places where we use exceptions for control flow. This PR improves the encapsulation of those usages by making the exception type module-local and private, preventing other modules from throwing these, or even knowing that they are being used.

This required a little bit of module trickery for the parsing code, but I don't think it's too bad overall.

Submission Checklist

  • Run unit tests
  • Documentation
    • If a user-facing facing change was made, the documentation PR is here:
    • OR, no user-facing changes were made

Release notes

Replace this text with a short note on what will change if this pull request is merged. This will be included in the release notes.

Copyright and Licensing

By submitting this pull request, the copyright holder is agreeing to
license the submitted work under the BSD 3-clause license (https://opensource.org/licenses/BSD-3-Clause)

@WardBrian WardBrian requested a review from nhuurre September 8, 2025 18:16
@WardBrian
Copy link
Copy Markdown
Member Author

@nhuurre apologies for all the review requests, these are all things I worked on during the release window that I'm only opening as PRs now.

Copy link
Copy Markdown
Collaborator

@nhuurre nhuurre left a comment

Choose a reason for hiding this comment

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

Typechecker looks fine but I'm not a fan of this "module trickery" when it comes to the lexer. How much does encapsulation buy us?

@WardBrian
Copy link
Copy Markdown
Member Author

How much does encapsulation buy us?

Less than enough to be worth it, honestly.

I think my bigger issue with the existing code was less about encapsulation (it should be obvious on code review that someone is raising an exception somewhere they should not, even if it is cool that we can statically prevent this with enough effort) and more about code organization, so I've just tried something else which looks more like how we handle Semantic_error.ts

@WardBrian WardBrian requested a review from nhuurre September 9, 2025 20:24
@WardBrian WardBrian merged commit 2ccb822 into master Sep 10, 2025
1 check passed
@WardBrian WardBrian deleted the cleanup-local-exception-usage branch September 10, 2025 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants