<div dir="ltr">Hi Rafael,<div><br></div><div>> <span style="font-size:13px">But they are always created, even if it as error the caller wants to</span></div><span style="font-size:13px">> ignore. For example, you will always create a "file foo.o in bar.a is</span><br style="font-size:13px"><span style="font-size:13px">> not a bitcode" message (or copy sufficient information for that to be</span><br style="font-size:13px"><span style="font-size:13px">> created). With a dignostic handler no copying is needed, since the</span><br style="font-size:13px"><span style="font-size:13px">> call happens in the context where the error is found. It is easy to</span><br style="font-size:13px"><span style="font-size:13px">> see us in a position where a lot of context is copied because some</span><br style="font-size:13px"><span style="font-size:13px">> client somewhere might want it.</span><div><br></div><div>I'm not saying that this system would replace diagnostic handlers in general, or that it should be shoehorned into places where it doesn't fit. This system is a replacement for std::error_code, because std::error_code is deficient as an error return. It happens that once you have a generic error return value you can use it for a lot of simple diagnostic cases, rather than threading a diagnostic handler everywhere. If you want to mix and match my error scheme with a diagnostic handler you still can.</div><div><br></div><div>As to the performance concern - you're trying to optimizing the error case. As noted, my scheme is slower than std::error_code on the error_path, but not prohibitively so. I've thought about this, and cannot imagine a reasonable use of this system that would lead to unacceptable overhead on the error path.</div><div><br></div><div><span style="font-size:13px">> So I am worried we are coding for the hypothetical and adding </span><span style="font-size:13px">complexity. </span><br></div><div><br></div><div>Don't worry - I'm too busy with real problems to tackle hypothetical ones. Here's another example of a problem that my proposal solves:</div><div><br></div><div>I recently added support for remote JITing to ORC. There are in-tree library facilities for JITing code into a process on the other end of an abstract "RPCChannel". What happens if something goes wrong at one end? We want to be able to communicate an error back across the RPCChannel (assuming it's still intact) so the other side can recover or fail gracefully. That means we need to be able to serialize an error with enough information to describe what went wrong. There's no practical way to maintain a serialization routine for all possible std::error_codes that might come up, even if they were powerful enough to describe everything that could go wrong (which again, being static kinds, they're not). With my proposal however, a JITError base class can be defined as:</div><div><br></div><div>class JITError : public TypedErrorInfo<JITError> {</div><div>public:</div><div>  virtual void serialize(RPCChannel &C) const = 0;</div><div>};</div><div><br></div><div>Now you just describe serialization/deserialization for each error when you define it. :)</div><div><br></div><div>(Yes - this hand waves over deserialization. It's solvable. The gory details will add nothing to this discussion).</div><div><br></div><div>> <span style="font-size:13px">In particular, if we are going this way I think it is</span></div><span style="font-size:13px">> critical that your patch *removes* the existing diagnostic handlers</span><br style="font-size:13px"><span style="font-size:13px">> (while making sure test/Bitcode/invalid.ll still passes) so that we</span><br style="font-size:13px"><span style="font-size:13px">> don't end up with two overlapping solutions. We were still not even</span><br style="font-size:13px"><span style="font-size:13px">> done converting away from bool+std::string :-(</span><div><span style="font-size:13px"><br></span></div><div>I think I've covered this now, but once more for emphasis: I'm not going to start tearing all the diagnostic handlers out. I absolutely see the value of diagnostic handlers for producing intricate diagnostics. My proposal tackles the error return problem, and it turns out that once you have a customizable error return value you can use it to produce decent diagnostics, as long as the client doesn't need to be able to control the formatting of those diagnostics. This property will allow us to produce better diagnostics that we currently do in several tools, without the need to introduce diagnostic handlers there. We can still introduce them later if we want to.</div><div><br></div><div>Cheers,</div><div>Lang.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 10, 2016 at 6:47 AM, 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"><span class="">>> This highlights why I think it is important to keep diagnostics and<br>
>> errors separate. In the solution you have there is a need to allocate<br>
>> a std::string, even if that is never used.<br>
><br>
> Errors are only constructed on the error path. There is no construction on<br>
> the success path.<br>
<br>
</span>But they are always created, even if it as error the caller wants to<br>
ignore. For example, you will always create a "file foo.o in bar.a is<br>
not a bitcode" message (or copy sufficient information for that to be<br>
created). With a dignostic handler no copying is needed, since the<br>
call happens in the context where the error is found. It is easy to<br>
see us in a position where a lot of context is copied because some<br>
client somewhere might want it.<br>
<br>
So I am worried we are coding for the hypothetical and adding<br>
complexity. In particular, if we are going this way I think it is<br>
critical that your patch *removes* the existing diagnostic handlers<br>
(while making sure test/Bitcode/invalid.ll still passes) so that we<br>
don't end up with two overlapping solutions. We were still not even<br>
done converting away from bool+std::string :-(<br>
<br>
Cheers,<br>
Rafael<br>
</blockquote></div><br></div>