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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to