<div dir="ltr"><div>Leaving 'Status' aside for now (the rename makes perfect sense), I'm basing my ErrorAnd / WithError naming suggestion on this comment:</div><div><br></div><div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Is there any chance of introducing something like make_status<T>() into llvm/Error.h, that constructs the llvm::Error in such a way that it still interoperates nicely with everything else?</blockquote></div><div><br></div><div>which contains a fundamental tension: Error's purpose is to be un-ignorable, which could be considered "not nice", and is definitely at odds with the idea of the user implicitly ignoring it if they want to (though it can be explicitly ignored by calling consumeError).</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">But if it does need to be handled (and as such is called an error), then I'm not sure if it makes sense to say there's also a value.  So ErrorOr, or Expected seems to convey that meaning in the only way possible.  If you don't get the thing you're expected to get, you need to handle the error.</blockquote><div><br></div><div>This is an aside from the LLDB conversation, but worth noting: While Error instances must be dealt with, that doesn't mean Error is only useful for hard errors. Being good for diagnostics was part of its original design brief. The ErrorAnd concept comes into play any time you have a data structure that can be partially malformed but still useful. Consider libObject, for example: It should be able to parse partially malformed object files (ones that have been truncated, or contain bad symbol indexes, or malformed symbols, etc.). However you want to make sure that the client explicitly acknowledges any errors in the file before proceeding (that way they can't claim later that you didn't warn them. ;). ErrorAnd<ObjectFile> is a perfect fit for this. It would take an Error (which may be success, a singleton Error, or a compound Error) and an ObjectFile, and force you to consume the Errors before accessing the file:</div><div><br></div><div>In pseudo-c++:</div><div><br></div><div><font face="monospace, monospace">// Parse my object file.</font></div><div><font face="monospace, monospace">auto ErrorAnd<ObjectFile> ObjAndErr = parseObjectFile(...);</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">// I claim that I'm willing to handle truncated objects, or objects</font></div><div><font face="monospace, monospace">// containing bad symbol </font><span style="font-family:monospace,monospace">indexes. If the object file contains errors</span></div><div><font face="monospace, monospace">// other than this I will bail out.</font></div><div><font face="monospace, monospace">if (auto RemainingErrors = handleErrors(ObjAndErr.takeError(),</font></div><div><font face="monospace, monospace">                                        [](const BadSymbolIndex &BSI) { ... },</font></div><div><font face="monospace, monospace">                                        [](const TruncatedObject &TO) { ... }))</font></div><div><font face="monospace, monospace">  return RemainingErrors;</font></div><div><font face="monospace, monospace"><br></font></div><div><font face="monospace, monospace">// Ok, *now* I can access my object:</font></div><div><font face="monospace, monospace">auto Obj = ObjAndError.takeValue();</font></div><div><br></div><div>Again, I'm not recommending this for any specific LLDB interfaces (I don't know them well enough yet), but I believe it has its place as an idiom.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Right.  We know that at some point (at the least where they escape to the SB API's) we'll have to have a container for the content of the error which carries none of the programmatic imperatives we want to impose at lower layers.</blockquote><div><br></div><div>The programmatic imperative is the key difference here.</div><div><br></div><div>Actually, Zachary - it's just occurred to me that that's what you may have been asking for: A type that's structured like Error, but without the hard requirement that it be checked? If so you're right - that might be an interesting thing to add to llvm/Error.h.</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">Instead, you could do this nicely by making a Status object that can accumulate (and by doing so check) the errors from its primitive operations.  You'd also have to be able to mark the Status as Succeeding.  Then the composite operation could turn the Status back into an Error, or pass it out as a Status, depending on its contract.  So I think it is worthwhile keep two separate entities.</blockquote><div><br></div><div>For a function that returns an Error, and whose callees return Error, I think you could just stay in Error mode the whole way through. Error supports compound values, and there are idioms for accumulating them with arbitrary checking applied at the accumulation point. Having an easy conversion utility between Error and Status is definitely a must though.</div><div><br></div><div>Cheers,</div><div>Lang.</div><div><br></div><div> <br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 10, 2017 at 7:36 PM, Jim Ingham <span dir="ltr"><<a href="mailto:jingham@apple.com" target="_blank">jingham@apple.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=""><br>
> On May 10, 2017, at 6:28 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
><br>
> Yes, this is just the rename.<br>
><br>
> Regarding the naming, if you call it ErrorAnd, or WithError, or anything that includes the word error, you are implying that something actually went wrong.  I don't think that's the intended use case, or at least not what I have in mind (and from previous conversations on the list, I don't think what Jim had in mind either).<br>
><br>
> If we're going to say that something does not need to be handled, I don't know if we should be calling it an error at all.  By definition, we should assert that errors must be handled, so the converse is that if it doesn't need to be handled, it's not an error.<br>
><br>
> But if it does need to be handled (and as such is called an error), then I'm not sure if it makes sense to say there's also a value.  So ErrorOr, or Expected seems to convey that meaning in the only way possible.  If you don't get the thing you're expected to get, you need to handle the error.<br>
><br>
> But it seemed like what we were talking was more of a way to provide diagnostic information about a long process that you could return alongside a result.  And if you don't get one, you don't necessarily care.  So it's like one step down in the expectation chain from Expected.  Possible<T> maybe?<br>
><br>
> I would expect an interface similar to Optional<T>, but with a way to get error *like* diagnostic information or messages that the user could ignore if they wanted to.<br>
<br>
</span>Right.  We know that at some point (at the least where they escape to the SB API's) we'll have to have a container for the content of the error which carries none of the programmatic imperatives we want to impose at lower layers.  So the plan as I understood it was to first declare that all Errors are actually what we give out to that layer.  So we will rename them to Status, since that makes more sense at the outer level. 'Course until we get to SB API 2.0 we can't ACTUALLY rename the type we give out to the SB layer, but since we had to have two names and that's what we want to convey, we might as well change it as far out as we can...  Then we'll go back and start from the leaves and chase up as far as is reasonable converting to force checked errors.  My guess is the point at which we'll find propagating checked errors starts to get annoying will be before the SB API layer anyway, though we'll see about that.<br>
<br>
But I think the "Status" object is more generally useful.<br>
<br>
For instance, there are places where we try a bunch of things to perform a task for the user: like all the places we might look in debug info to find types when parsing expressions, or all the paths on the system we look at when trying to find SDK's or other system data, etc.  In these sorts of composite operations, many of the sub-tasks can fail - and from the sub-tasks perspective the failure should be reported as an error.  But only if all of them fail is the composite operation actually an error.  In this sort of operation, you have to be careful not to lose any of the error messages along the way.  For instance, when you look in a bunch of places for something, you should start from the most plausible place and proceed through more unlikely fallbacks.  It's a common mistake in this situation to end up reporting as the error, the failure of the last operation.  That is almost never helpful, it was the failure in the first couple (should really have succeeded) attempts that was actually meaningful.<br>
<br>
If we start handling errors in the "I'm returning a thing or an error" type interface you are considering, then accumulating many failures into one of these errors seems wrong.<br>
<br>
Instead, you could do this nicely by making a Status object that can accumulate (and by doing so check) the errors from its primitive operations.  You'd also have to be able to mark the Status as Succeeding.  Then the composite operation could turn the Status back into an Error, or pass it out as a Status, depending on its contract.  So I think it is worthwhile keep two separate entities.<br>
<span class="HOEnZb"><font color="#888888"><br>
Jim<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
<br>
<br>
<br>
><br>
> On Wed, May 10, 2017 at 6:09 PM Lang Hames <<a href="mailto:lhames@gmail.com">lhames@gmail.com</a>> wrote:<br>
> Cool. This is just the rename portion, right?<br>
><br>
> Sorry I didn't respond to your last message too.<br>
><br>
> I suppose, but I'm not sure ErrorAnd captures the intended meaning very well.  In any case, that's not super important at this stage since this isn't on the immediate horizon.<br>
><br>
> Do you just mean that ErrorAnd isn't an especially nice name? I wasn't entirely sure what make_status<T>(...) was supposed to do so I assumed it was to create a pair of an Error and a T. If that's the case, make_with_error<T>(T, Error) (and WithError<T>) might be better names?<br>
><br>
> Cheers,<br>
> Lang.<br>
><br>
><br>
> On Tue, May 9, 2017 at 8:58 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
> I'm probably going to be looking at submitting this this week, more likely sooner rather than later.  If I can get it all working hopefully even tomorrow.<br>
><br>
> On Mon, May 1, 2017 at 5:49 PM Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
> I suppose, but I'm not sure ErrorAnd captures the intended meaning very well.  In any case, that's not super important at this stage since this isn't on the immediate horizon.<br>
><br>
> On Mon, May 1, 2017 at 5:43 PM Lang Hames <<a href="mailto:lhames@gmail.com">lhames@gmail.com</a>> wrote:<br>
> Hi Zachary,<br>
><br>
> ... Then instead of Expected<T> you could have WithDiagnostics<T> that enforces the proper semantics.<br>
><br>
> You mean something like an ErrorAnd<T>? Chris Bieneman floated that idea for some libObject code but we haven't got around to implementing it. If it were generically useful we could do something like that.<br>
><br>
> Cheers,<br>
> Lang.<br>
><br>
><br>
> On Mon, May 1, 2017 at 5:36 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
> Is there any chance of introducing something like make_status<T>() into llvm/Error.h, that constructs the llvm::Error in such a way that it still interoperates nicely with everything else?  Then instead of Expected<T> you could have WithDiagnostics<T> that enforces the proper semantics.<br>
><br>
> On Mon, May 1, 2017 at 5:33 PM Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
> On Mon, May 1, 2017 at 5:27 PM Jim Ingham <<a href="mailto:jingham@apple.com">jingham@apple.com</a>> wrote:<br>
><br>
> > On May 1, 2017, at 4:52 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
> ><br>
> > Yea, grouping the error and the result together is one of the most compelling features of it.  It's called Expected<T>, so where we would currently write something like:<br>
> ><br>
> > int getNumberOfSymbols(Error &Err) {}<br>
> ><br>
> > or<br>
> ><br>
> > Error getNumberOfSymbols(int &Count) {}<br>
> ><br>
> > You would now write:<br>
> ><br>
> > Expected<int> getNumberOfSymbols() {<br>
> >    if (foo) return 1;<br>
> >    else return make_error<DWARFError>("No symbols!");<br>
> > }<br>
> ><br>
> > and on the caller side you write:<br>
> ><br>
> > Error processAllSymbols() {<br>
> >   if (auto Syms = getNumberOfSymbols()) {<br>
> >     outs() << "There are " << *Syms << " symbols!";<br>
> >   } else {<br>
> >     return Syms.takeError();<br>
> >     // alternatively, you could write:<br>
> >     // consumeError(Syms.takeError())<wbr>;<br>
> >     // return Error::success();<br>
> >   }<br>
> > }<br>
> ><br>
><br>
> Interesting.<br>
><br>
> This pattern doesn't quite work for fetching symbols - maybe that really is more suitable as a Status than an Error.  After all, number of symbols == 0 is not necessarily an error, there just might not have been any symbols (e.g. a fully stripped main); and I'm going to work on whatever symbols I get back, since there's nothing I can do about the ones that didn't make it.  I just want to propagate the error so the user knows that there was a problem.<br>
><br>
> Jim<br>
><br>
> Sure, that was just a made up example.  You could imagine that being some private function deep in the implementation details of a symbol parser, where you've got some header that's supposed to be N bytes, and getNumberOfSymbols() seeks to offset 42 and reads a 4 byte value and returns it, but the function sees that there's only 40 bytes in the header, so it's not that there's no symbols, it's that something is seriously messed up.<br>
><br>
> In that case you could return an error such as this.<br>
><br>
> Of course, the person who called this function can either propagate it, deal with it some other way and mask it, or whatever.  Mostly I was just trying to show what the syntax looked like for grouping return values with errors.<br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div>