Add callstacks to generators that can error. #538
Conversation
Previously, `val <- Gen.element []` would give an error like
Hedgehog.Gen.element: used with empty Foldable
CallStack (from HasCallStack):
error, called at src/Hedgehog/Internal/Gen.hs:1197:5 in hedgehog-1.4-FEcjASqhnyiHkb8BJanjYM:Hedgehog.Internal.Gen
Which isn't very helpful. Now the callstack entry points to my own code,
instead.
|
Welp, apparently this fails on 8.0.2 :/ I imagine I can fix that somehow (we use |
There was a problem hiding this comment.
Thank you for the PR!
The addition of HasCallStack and withFrozenCallStack looks good wrt improving error diagnostics. However, since this change modifies the function signatures, it could potentially break backward compatibility for users with explicit type annotations. So we’ll need to consider a major version bump.
Regarding GHC 8.0.2, I agree that it’s reasonable to drop support, allowing us to focus on more recent releases.
|
Yeah, agree it'll need a major version bump. I can put either or both of the version bump and dropping 8.0.2 support in this PR if you want, or they can be separate. |
|
Could be done in the same PR, just separate Git commit. And then we may update the PR title. |
9c85082 doesn't compile on it, and 8.2.1 was released in 2017.
ChickenProp
force-pushed
the
gen-callstacks
branch
from
220896e
to
ec5f834
Compare
|
Okay, done both of those. |
There was a problem hiding this comment.
@ChickenProp, thank you! The changes look good, so I’m happy to approve.
I’ll leave some room for other contributors to review, and if we don’t hear back, we can merge later this week. Hope that's okay! Thanks again for your work on this.
Previously,
val <- Gen.element []would give an error likeWhich isn't very helpful. Now the callstack entry points to my own code, instead.
I guess this will need a version bump since it changes types?
I think I got all the generators that are expected to throw errors. Some call
errorbut describe it as internal, and I didn't touch those.