[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-26 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith closed this revision. dexonsmith added a comment. r369943. 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/cg

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-26 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66705/new/ https://reviews.llvm.org/D66705 ___ cfe-commits mailing list cfe-commi

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Lang Hames via Phabricator via cfe-commits
lhames added a comment. In D66705#1644166 , @dexonsmith wrote: > Updated patch after running `check-clang` and learning more about `Expected`. > I've added a parallel API using `Optional` for clients that > don't want to do anything with the error. > >

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 217026. dexonsmith edited the summary of this revision. dexonsmith added a comment. Fix the change to CompilerInstance.cpp. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66705/new/ https://reviews.llvm.org/D66705 Files: clang/include/clang/Bas

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 217025. dexonsmith marked an inline comment as done. dexonsmith edited the summary of this revision. dexonsmith added a comment. Also update `FileEntryRef` to be copy- and move-assignable so that `Optional` can be assigned to. CHANGES SINCE LAST ACTION

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
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; arph

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith updated this revision to Diff 217024. dexonsmith edited the summary of this revision. dexonsmith added a subscriber: lhames. dexonsmith added a comment. Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Updated patch after running `check-clang` and learning more abo

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman added inline comments. Comment at: clang/lib/Lex/HeaderSearch.cpp:1079 if (!File) { // Check ".../Frameworks/HIToolbox.framework/PrivateHeaders/HIToolbox.h" HeadersFilename = FrameworkName; It should be fine to update the return type, but I

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. @arphaman, is there a reason you think `ErrorOr` is more appropriate long-term here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66705/new/ https://reviews.llvm.org/D66705 ___ cfe-commits mailing list cfe-comm

[PATCH] D66705: FileManager: Use llvm::Expected in new getFileRef API

2019-08-24 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith created this revision. dexonsmith added a reviewer: arphaman. Herald added a subscriber: ributzka. dexonsmith added a comment. @arphaman, is there a reason you think `ErrorOr` is more appropriate long-term here? `FileManager::getFileRef` is a modern API which we expect to convert to