<div dir="ltr">Hi Rafael,<div><br></div><div>Just in case it's useful: You can accumulate Error instances using the join_errors function. It's intended for situations where you want to accumulate multiple diagnostics before bailing out. That said, I think it would be reasonable to just bail early here as you suggested.</div><div><br></div><div>Convertibility to error_code is important: Unless you can be certain that all clients of the interface have transitioned 100% to Error, you have to assume that a client might call errorToErrorCode, and if you haven't implemented the conversion that call will crash.</div><div><br></div><div>I've generally aimed for error_code <-> Error mappings that are reasonably information preserving, but we could introduce a new error_category and enum in the Error header: IndescribableError, and allow people who are introducing new Error types to map to that. It might even serve as a handy forcing function to get people to move over.</div><div><br></div><div>- Lang.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, May 24, 2016 at 5:09 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">With this patch the linker would be using all 3 error handling systems<br>
in llvm, which is way too much.<br>
<br>
What I think needs to happen is for it to have an Error class that is<br>
not converted to error_code. We also have to decide if we are keeping<br>
a list of error or just failing early. I don't think it is worth it<br>
keeping multiple errors, so I would suggest refactoring the code to<br>
return early and only store the error when it hits an interface that<br>
returns void.<br>
<br>
We should also stop logging error to the streamer and instead have the<br>
caller uses log on the Error.<br>
<br>
The attached patch implements part of that. What do you think?<br>
<br>
Cheers,<br>
Rafael<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
On 24 May 2016 at 18:53, Rafael Espíndola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> wrote:<br>
> Taking a look.<br>
><br>
> On 24 May 2016 at 18:30, Peter Collingbourne <<a href="mailto:peter@pcc.me.uk">peter@pcc.me.uk</a>> wrote:<br>
>> pcc added a comment.<br>
>><br>
>> Thanks Lang. This new patch does as you suggested: it retains the std::error_category, but converts the error_code to an Error at the last moment. I have also updated the call sites to handle a non-null Error in the same way as they handled a true return value before.<br>
>><br>
>><br>
>> <a href="http://reviews.llvm.org/D20550" rel="noreferrer" target="_blank">http://reviews.llvm.org/D20550</a><br>
>><br>
>><br>
>><br>
</div></div></blockquote></div><br></div>