Closed WardBrian closed 3 months ago
nhuurre
commented
3 months ago Great, this looks good.
My only concern is that there's no way to test it!
How do feel about, for example, adding command line option --internal-compiler-error-test (disabled in release builds) that would make statements like
fatal_error("InternalCompilerError", "parsing");
fatal_error("InternalCompilerError", "typecheck");
fatal_error("InternalCompilerError", "lowering");trigger ICE in various compilation phases?
WardBrian
commented
3 months ago I have a branch with examples of ICEs triggered by known-bad edge cases in the pretty-printer.
We could merge that was a 'test' of this functionality, but of course it could eventually go away if we fixed those bugs. And because they all require the #include statement, couldn't use them to test the JS parser
WardBrian
commented
3 months ago I added some code unit tests. End to end tests are tricky because of the idea of the command line option being "(disabled in release builds) " -- OCaml intentionally avoids having easy support for conditional compilation
codecov[bot]
commented
3 months ago Attention: Patch coverage is 66.66667% with 3 lines in your changes are missing coverage. Please review.
Project coverage is 89.87%. Comparing base (
efbefa0) to head (7014b3b).
This closes #1408. There is no real difference in the end result for the binary executable, but this is a huge improvement for stanc3js since internal errors will be cleanly converted to the error string format rather than percolating as javascript exceptions.
Submission Checklist
Release notes
The compiler should display a cleaner error if it encounters an internal error.
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)