[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Harlan Haskins via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL367618: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure (authored by harlanhaskins, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits.

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Jan Korous via Phabricator via cfe-commits
jkorous accepted this revision. jkorous marked an inline comment as done. jkorous added a comment. This revision is now accepted and ready to land. LGTM Thanks for all the work here! Comment at: clang/lib/ARCMigrate/FileRemapper.cpp:156 + auto newE =

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Harlan Haskins via Phabricator via cfe-commits
harlanhaskins updated this revision to Diff 212860. harlanhaskins marked 8 inline comments as done. harlanhaskins added a comment. Updated in response to feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Harlan Haskins via Phabricator via cfe-commits
harlanhaskins marked 6 inline comments as done and an inline comment as not done. harlanhaskins added inline comments. Comment at: clang/lib/ARCMigrate/FileRemapper.cpp:156 + auto newE = FileMgr->getFile(tempPath); + if (newE) { +remap(origFE, *newE);

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment. Nice! This makes https://bugs.llvm.org/show_bug.cgi?id=42524#c3 easier. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/ https://reviews.llvm.org/D65534 ___ cfe-commits

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-08-01 Thread Raphael Isemann via Phabricator via cfe-commits
teemperor added a comment. The error handling in LLDB seems fine to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/ https://reviews.llvm.org/D65534 ___ cfe-commits mailing list

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-07-31 Thread Jan Korous via Phabricator via cfe-commits
jkorous added inline comments. Comment at: clang-tools-extra/clang-tidy/ClangTidy.cpp:240 +auto File = SourceMgr.getFileManager().getFile(FilePath); +if (!File) + return SourceLocation(); Previously we'd hit the assert in `translateFile()` called

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-07-31 Thread Harlan Haskins via Phabricator via cfe-commits
harlanhaskins updated this revision to Diff 212701. harlanhaskins added a comment. Store references instead of raw pointers in FileManger's cache Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/ https://reviews.llvm.org/D65534 Files:

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-07-31 Thread Harlan Haskins via Phabricator via cfe-commits
harlanhaskins added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:217 /// - /// This returns NULL if the file doesn't exist. + /// This returns a \c std::error_code if there was an error loading the file. /// JDevlieghere wrote: >

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-07-31 Thread Jonas Devlieghere via Phabricator via cfe-commits
JDevlieghere added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:217 /// - /// This returns NULL if the file doesn't exist. + /// This returns a \c std::error_code if there was an error loading the file. /// harlanhaskins wrote: >

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-07-31 Thread Harlan Haskins via Phabricator via cfe-commits
harlanhaskins marked 2 inline comments as done. harlanhaskins added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:143 /// - llvm::StringMap SeenDirEntries; + llvm::StringMap, llvm::BumpPtrAllocator> + SeenDirEntries; jkorous wrote: >

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-07-31 Thread Jan Korous via Phabricator via cfe-commits
jkorous added inline comments. Comment at: clang/include/clang/Basic/FileManager.h:143 /// - llvm::StringMap SeenDirEntries; + llvm::StringMap, llvm::BumpPtrAllocator> + SeenDirEntries; Maybe we could replace this with some type that has

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-07-31 Thread Davide Italiano via Phabricator via cfe-commits
davide added a comment. [and Raphael for the clang vendor bits] Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/ https://reviews.llvm.org/D65534 ___ cfe-commits mailing list

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-07-31 Thread Davide Italiano via Phabricator via cfe-commits
davide added a comment. Really on the lldb side, Jonas is the right person to review this patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65534/new/ https://reviews.llvm.org/D65534 ___

[PATCH] D65534: [clang] Change FileManager to use llvm::ErrorOr instead of null on failure

2019-07-31 Thread Harlan Haskins via Phabricator via cfe-commits
harlanhaskins created this revision. harlanhaskins added reviewers: arphaman, bruno. Herald added subscribers: lldb-commits, cfe-commits, jsji, kadircet, dexonsmith, jkorous, MaskRay, kbarton, nemanjai. Herald added a reviewer: martong. Herald added a reviewer: shafik. Herald added projects: