Make sure mentat error types preserve failure backtraces (wip)#798
Make sure mentat error types preserve failure backtraces (wip)#798thomcc wants to merge 1 commit intomozilla:masterfrom thomcc:failure-keep-backtraces
Conversation
This adds a lot of boilerplate which could be simplified by macros. I was planning on cleaning this up before pushing it but here it is!
grigoryk
left a comment
There was a problem hiding this comment.
Pardon my ignorance, but could you explain how the added split of Error/ErrorKind actually helps preserve backtraces? It's not clear to me what this achieves beyond adding another layer into the error cake - but I'm sure I'm missing something.
| } | ||
|
|
||
| #[derive(Debug)] | ||
| pub struct AlgebrizerError(Box<Context<AlgebrizerErrorKind>>); |
There was a problem hiding this comment.
It's not clear to me why you're boxing the Context. Can you explain why?
|
Okay, after reading a tad more and talking to Thom, I understand why this works. As should have been obvious, just deriving a Fail trait won't provide our error enums with a place to store backtrace. And so, something like this is basically what we'd want - a struct to host bunch of contextual information, a lightweight enum for actual error types, and some glue to tie it all together. It's not clear to me why we need to Box stuff. You mentioned reducing memcpy traffic in case of large ErrorKinds, but in our case they're mostly just simple enums with an i64 or sometimes a String in them. Could you explain your reasoning here? Otherwise, maybe a clean-up with some macros? That would either mean duplicating the macros in a bunch of places, or having them live in |
This adds a lot of boilerplate which could be simplified by macros.
I was planning on cleaning this up before pushing it but here it is!
Caveat: I haven't tested this in versions of rust other than whatever i happen to be using by default right now! I did try to make the changes compatible with 1.25.0 (e.g.
&/refnoise in matches) but that was just to minimize future work and I might not have been thorough here