Report internal errors more clearly #5661
Conversation
- `CompileExn` and `internalBug` have been moved to a new module and had duplicate implementations removed, - `internalBug` has a much richer message that should be more helpful to users, - `internalBug` now accepts a list of issue numbers relevant to that message, and - exception-related imports are now more explicit. Also, there are a couple open questions tagged with __TODO__.
Previously there were three ways of handling optional `Width`
1. distinct `x` and `xUnbroken` functions,
2 `Maybe Width`, and
3 `Width 0` implies “unbroken”.
Since it’s difficult to use strictly positive numeric types[^1], this
now consistently treats `width <= 0`[^2] as “unbroken”.
[^1]: If it weren’t, I would have preferred something like `Maybe
Positive`.
[^2]: It would be nice to define `Width` using something like `Word`
instead of `Int`, which would allow `width == 0`, but that has
cascading impacts across functions like `length` and `replicate`,
which makes it more intrusive.
This also results in a couple inconsequential semantic changes to
“unbroken” rendering:
- “unbroken” is truly unbroken, not limited to `maxBound :: Int`
- right-aligning `Pretty` output (which isn’t currently used anyway)
would do nothing in the “unbroken” case, rather than padding the
output to `maxBound :: Int` columns.
sellout
force-pushed
the
improve-internal-errors
branch
from
84a2d1b to
9b252cc
Compare
|
Hey this looks great. Two thoughts:
Sorry I ended up making this a bad example by closing 3625 as a duplicate 😅 but yes it should take a list like you have already done.
|
Ah, yeah – I should have included this under “loose ends”. I was thinking we could fetch the title via the GitHub API, so the
Yeah, I was following the instructions in interpreter-tests.md. I figured I shouldn’t open a contribution until this PR is stabilized (e.g., you suggestions will require an update to my runtime-tests branch). |
sellout
force-pushed
the
improve-internal-errors
branch
from
8384155 to
26428cd
Compare
- rephase - include GitHub issue titles when possible
`Unison.Runtime.Exception.die` is overridden in `Machine.Types`, and most call sites end up inheriting and using that instead. This just folds the minor alteration of that version into the one in `Exception`.
sellout
force-pushed
the
improve-internal-errors
branch
from
8754c04 to
37d85c8
Compare
yeah that could work; we just have to remember to apply short github titles in that case
My bad. Ok so we'll stabilize the PR, then open the contribution for |
Overview
Internal errors are rather opaque. In #4463, we see
this adds more context, so now we have
Test coverage
There is a new transcript to replicate #4463, which does exercise this, but it will stop exercising it once that issue is fixed. But perhaps more reproductions will be added in the meantime …
I recommend reviewing commit-by-commit.