<div dir="ltr">Hi James,<div><br></div><div>The complexity involved in runtime checking is minimal. In terms of source complexity the checking code runs only ~20 lines total (it's orthogonal to the RTTI system and utilities, which take up the bulk of the code). The runtime overhead would be minimal in debug builds, and non-existent in release if we turn off checking there.</div><div><br></div><div>Runtime checking is significantly more powerful too. Take an anti-pattern that I've seen a few times:</div><div><br></div><div><font face="monospace, monospace">for (auto &Elem : Collection) {</font></div><div><font face="monospace, monospace"> if (std::error_code Err = foo(Elem))</font></div><div><font face="monospace, monospace"> if (Err == recoverable_error_code) {</font></div><div><font face="monospace, monospace"> // Skip items with 'recoverable' failures. </font></div><div><font face="monospace, monospace"> continue;</font></div><div><font face="monospace, monospace"> }</font></div><div><font face="monospace, monospace"> // Do stuff with elem.</font></div><div><font face="monospace, monospace">}</font></div><div><br></div><div>This is the kind of code I want to stop: The kind where we pay just enough lip service to the error to feel like we've "handled" it, so we can get on with what we really want to do. This code will fail at runtime with unpredictable results if Err is anything other than 'success' or 'recoverable_error_code', but it does inspect the return type, so an attribute won't generate any warning.</div><div><br></div><div><br></div><div><br></div><div>The advantage of catchTypedErrors over an if statement is that it lets you defer errors, which we wanted to be able to do in our archive walking code:</div><div><br></div><div><font face="monospace, monospace">TypedError processArchive(Archive &A) {</font></div><div><font face="monospace, monospace"> TypedError Errs;</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> for (auto &Obj : A) {</font></div><div><font face="monospace, monospace"> if (auto Err = processObject(Obj))</font></div><div><font face="monospace, monospace"> if (Err.isA<ObjectError>()) {</font></div><div><font face="monospace, monospace"> // processObject failed because our object was bad. We want to report</font></div><div><font face="monospace, monospace"> // this to the user, but we also want to walk the rest of the archive</font></div><div><font face="monospace, monospace"> // to collect further diagnostics, or take other meaningful actions.</font></div><div><font face="monospace, monospace"> // For now, just append 'Err' to the list of deferred errors.</font></div><div><font face="monospace, monospace"> Errs = join_error(std::move(Errs), std::move(Err));</font></div><div><font face="monospace, monospace"> continue;</font></div><div><font face="monospace, monospace"> } else</font></div><div><font face="monospace, monospace"> return join_error(std::move(Err), std::move(Errs));</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace"> // Do more work.</font></div><div><font face="monospace, monospace"> }</font></div><div><br></div><div><font face="monospace, monospace"> return Errs;</font></div><div><font face="monospace, monospace">}</font></div><div><br></div><div>and now in main, you can have:</div><div><br></div><div><font face="monospace, monospace">catchTypedErrors(processArchive(A),</font></div><div><font face="monospace, monospace"> handleTypedError<ObjectError>([&](std::unique_ptr<ObjectError> OE) {</font></div><div><font face="monospace, monospace"> ...</font></div><div><font face="monospace, monospace"> })</font></div><div><font face="monospace, monospace">);</font></div><div><br></div><div>And this one handler will be able to deal with all your deferred object errors.</div><div><br></div><div>For clients who know up-front that they'll never have to deal with compound errors, the if-statement would be fine, but I think it's better not to assume that.</div><div><br></div><div>I want to stress that I appreciate the distaste boilerplate, but as I mentioned in the proposal actually catching errors is the uncommon case, so it's probably ok if it's a little bit ugly.</div><div><br></div><div>Cheers,</div><div>Lang.</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 3, 2016 at 7:40 AM, James Y Knight <span dir="ltr"><<a href="mailto:jyknight@google.com" target="_blank">jyknight@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><span class="">On Tue, Feb 2, 2016 at 9:23 PM, Lang Hames <span dir="ltr"><<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="auto"><div>I see the attribute as complimentary. The runtime check provides a stronger guarantee: the error cannot be dropped on any path, rather than just "the result is used". The attribute can help you catch obvious violations of this at compile time.</div></div></blockquote><div> </div></span><div>I agree the runtime check <i>can</i> catch something additional, it just doesn't feel to me like the extra complexity has been justified.</div><div><br></div><div>Or, at least, I'd have imagined a much simpler and straightforward interface would be fully sufficient. E.g., instead of the all the catch/handle stuff, if you want to handle a particular class specially, how about just using an if?<div><div><br></div><div><div>TypedError err = somethingThatCanFail();</div><div>if (err) {</div><div> if (err.isClassOf(...)) {<br></div><div> whatever;</div><div> else</div><div> return err;<br>}</div></div></div></div></div>
</blockquote></div><br></div>