rust: support rich errors #769
Conversation
Codecov Report
@@ Coverage Diff @@
## master #769 +/- ##
==========================================
+ Coverage 99.19% 99.23% +0.04%
==========================================
Files 77 77
Lines 11893 11929 +36
==========================================
+ Hits 11797 11838 +41
+ Misses 96 91 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
|
axic
force-pushed
the
rust-rich-error
branch
3 times, most recently
from
90feba2
to
21ef74d
Compare
gumb0
force-pushed
the
rich-error-reporting
branch
6 times, most recently
from
f04fd67
to
d02dde6
Compare
axic
force-pushed
the
rust-rich-error
branch
4 times, most recently
from
8bc7c36
to
b1a4b88
Compare
| ) | ||
| }; | ||
| if ret { | ||
| debug_assert!(err.code() == 0); |
There was a problem hiding this comment.
These assertions show the current error reporting system is not prefect, as there are two ways to tell failure, error code or the return value.
There was a problem hiding this comment.
For validate theoretically we could return error code instead of bool (plus optional error message), but for parse and instantiate it seems inevitable: we have to return a pointer to module / instance in any case, and when we also return an error code this pointer is going to be NULL.
There was a problem hiding this comment.
Do we want to add assertions into the capi for this? Or at least extend the tests? And document it?
These debug_asserts here act like the C assert, they are not part of a release build. Happy to keep them though.
There was a problem hiding this comment.
Do we want to add assertions into the capi for this? Or at least extend the tests? And document it?
Tests already have checks for both return value and error for both failure and success cases.
Assertions in C API - not sure where and which ones (it seems kind of obvious there that error is set when false or nullptr is returned)
Document it - something likeReturned error code is FIZZY_SUCCESS iff return value is true? Sounds somewhat obvious, too...
There was a problem hiding this comment.
I guess documentation is fine just saying that error code must be FIZZY_SUCCESS if a pointer is returned and must NOT be FIZZY_SUCCESS otherwise? Basically we want to give indication to the user that they do not need to check both.
Depends on #772.