Conversation
|
@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. |
nhuurre
left a comment
There was a problem hiding this comment.
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?
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 |
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
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)