Exception contexts#63
Conversation
73925cd to
55ed5db
Compare
Co-authored-by: Ruifeng Xie <ruifengx@outlook.com>
Co-authored-by: Edsko de Vries <edsko@well-typed.com>
55ed5db to
6040c53
Compare
|
Glad to see this resolved! I saw your blog post (before this PR actually) and really enjoyed the read. You are absolutely right that my PR did not correctly handle the case where the exception handler throws (in that the Regarding Regarding I will close my original PR, given that the fix already lands in the repo. Thanks again for working on binding generation. P.S. I had several attempts on a Haskell binding generation tool. It worked for some toy examples, but it never reached the maturity and generality I expect. I guess all that efforts were not futile in the end anyway, because at least I learned about As a side note, now that the |
Ah, that's how you noticed it was leaking! Makes sense now :) And yes, that's definitely a reasonable approach, the exception infra is definitely complicated. |
Supersedes #60 .
@ruifengx In this PR I have taken the various ideas from your PR (#60), and combined them with some work I did another project, as well as applied all the various insights that I learned while trying to make sense of the situation regarding exception annotations (which led to my blog post Exception Annotations: Lay of the Land).
The technical details of the PR looks quite a bit different from yours, but its essence is still there. There is one important different in approach though. In your version of
handleUnliftUsing, you were explicitly usingcatchNoPropagateandrethrowIO. Initially I did not understand why you opted for this, but then I realized that without that,simpleFold,foldWithHandlerandfoldTrywould all add unnecessaryWhileHandlingannotations because they all rethrow the exception (unconditionally insimpleFoldand when the cast fails infoldWithHandlerandfoldTry). And I agree: unnecessaryWhileHandlingannotations result in a lot of unnecessary noise, making backtraces more difficult to read than needed (for example, I submitted a patch against async a few days ago for the same reason).However, if the exception handler itself throws a (different) exception, then not adding the
WhileHandlingexception means that we lose the original exception, and its annotations: that's bad. So we want some kind of combination ofhandleandonException; I tackled this problem by changing the type of the handler result: the handler can now distinguish between (1) returning a normal result, (2) throwing a new exception (which will get aWhileHandlingannotaiton), or (3) indicating that the original exception should be rethrown (without aWhileHandlingannotation).The other changes are more minor:
Rethrownewtype wrapper toExactException; this is a pattern I recently starting adopting ingrapesy, and I find it quite useful; it emphasises "this exact exception, and not some variant of it with annotations altered". The nice thing is that this can be defined almost without CPP and it will work across all GHC versions.SomeExceptionanywhere anymore now; it's alwaysExactException, withthrowExactas a helper to avoid mistakes. I consider throwingSomeExceptiona red flag these days; in your version, you basically fixedthrowIOinstead, which also works, but I find this approach a bit more explicit.tryI think the version frombaseis just fine; althoughcatchadds aWhileHandlingannotations to exceptions thrown by the exception handler,trynever throws, so this does not matter.ExactException).Thanks for the fix for using
freeHaskellFunPtr! (How did you even notice that?). I modified it slightly to usebracketinstead.Thanks again for your contribution :) We've added you to the list of contributors in https://github.com/well-typed/hs-bindgen/ . Let me know if anything in this PR looks wrong to you!