[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API
Duncan P. N. Exon Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Sat Aug 24 11:53:29 PDT 2019
dexonsmith marked 2 inline comments as done.
dexonsmith added inline comments.
================
Comment at: clang/lib/Lex/HeaderSearch.cpp:1079
if (!File) {
// Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h"
HeadersFilename = FrameworkName;
----------------
arphaman wrote:
> It should be fine to update the return type, but I believe that `Expected` will cause an assertion as the error is unhandled in cases like this one. Can you verify that all errors are consumed somehow?
Yup, when the tests finished running I saw that I'd misunderstood how `Expected::operator bool` works.
For the users that don't care about the errors, consuming them is harmful boilerplate. I've switched to a split API that either returns `Expected` or `Optional`. This way clients that do care get the assertions if they mishandle it, and the clients that don't annotate that at the call site. WDYT?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66705/new/
https://reviews.llvm.org/D66705
More information about the cfe-commits
mailing list