<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class=""><br class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Sep 4, 2019, at 17:39, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="">dblaikie@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><br class="Apple-interchange-newline"><br style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;" class=""><div class="gmail_quote" style="caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: 12px; font-style: normal; font-variant-caps: normal; font-weight: normal; letter-spacing: normal; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;"><div dir="ltr" class="gmail_attr">On Mon, Aug 26, 2019 at 11:28 AM Duncan P. N. Exon Smith via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a>> wrote:<br class=""></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;">Author: dexonsmith<br class="">Date: Mon Aug 26 11:29:51 2019<br class="">New Revision: 369943<br class=""><br class="">URL:<span class="Apple-converted-space"> </span><a href="http://llvm.org/viewvc/llvm-project?rev=369943&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=369943&view=rev</a><br class="">Log:<br class="">FileManager: Use llvm::Expected in new getFileRef API<br class=""><br class="">`FileManager::getFileRef` is a modern API which we expect to convert to<br class="">over time. We should modernize the error handling as well, using<br class="">`llvm::Expected` instead of `llvm::ErrorOr`, to help clients that care<br class="">about errors to ensure nothing is missed.<br class=""><br class="">However, not all clients care. I've also added another path for those<br class="">that don't:<br class=""><br class="">- `FileEntryRef` is now copy- and move-assignable (using a pointer<br class=""> <span class="Apple-converted-space"> </span>instead of a reference).<br class="">- `FileManager::getOptionalFileRef` returns an `llvm::Optional` instead<br class=""> <span class="Apple-converted-space"> </span>of `llvm::Expected`.<br class="">- Added an `llvm::expectedToOptional` utility in case this is useful<br class=""> <span class="Apple-converted-space"> </span>elsewhere.<br class=""></blockquote><div class=""><br class="">I'd hesitate to add new general constructs that swallow errors like this - keeping them manually written might help avoid their use becoming too common.<br class=""><br class="">On that note/direction - are there enough callers of getFileRef that don't care about errors that it's really impractical for them to each explicitly swallow the errors?</div></div></div></blockquote><div><br class=""></div><div>`getFileRef` is intended to eventually supplant `getFile` which has many users. Most of them don't care about the error, they just want to know whether or not they have a file entry. If it makes sense to change them at some point that's great, but I think having them use `getOptionalFileRef` makes it easy to track down (and potentially change) the ones that are ignoring the specific error, without requiring a ton of boilerplate at each call site in the meantime. An un-posted version of the patch changed all the current call sites of getFileRef to handle/ignore the error explicitly and it looked like I was making the code worse.</div><div><br class=""></div><div>That said, as long as we have the getOptionalFileRef API, I don't feel strongly about the llvm::expectedToOptional utility. The points in favour are that it aligns well with llvm::errorToBool, it reduces boilerplate, and it seems both explicit and grep'able. Maybe that's not compelling enough though.</div></div></body></html>