Stream: git-cranelift

Topic: cranelift / Issue #544 VerifierStepResult is confusing

GitHub (Feb 28 2020 at 23:25):

alexcrichton transferred Issue #544:

The verifier functions have an API that could probably be simplified. Functions which return VerifierStepResult must by convention also take an out-param VerifierErrors that will contain non-fatal errors.

Moreover, the T in VerifierStepResult<T> seems to always be set to (), so it's unused.

It seems the out param is redundant with the error that's present in the Result hidden VerifierStepResult. I think slightly modifying the interface of VerifierStepResult would avoid this out param:

• let the Ok type be VerifierErrors, in case we only have non-fatal errors (they could be called "warnings").
• let the Err type stay the same (VerifierErrors too), and contain non-fatal and fatal errors, if there was at least one fatal error.

Then we wouldn't need the errors outparam anymore, which looks cleaner and "more Rusty". It might mean that a few users of the fatal! etc macros would need to have their own errors variable, but that seems OK.

Thoughts?

Last updated: Nov 22 2024 at 16:03 UTC