This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368108: [clang][DirectoryWatcher][NFC] Swapping asserts for
llvm fatal_error in create (authored by zer0, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D65829?vs=213742&id=213746#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65829/new/
https://reviews.llvm.org/D65829
Files:
cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h
cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
Index: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===================================================================
--- cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -277,14 +277,20 @@
{EventKind::Modified, "c"}}
};
- auto DW = DirectoryWatcher::create(
- fixture.TestWatchedDir,
- [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
- bool IsInitial) {
- TestConsumer.consume(Events, IsInitial);
- },
- /*waitForInitialSync=*/true);
- if (!DW) return;
+ llvm::Expected<std::unique_ptr<DirectoryWatcher>> DW =
+ DirectoryWatcher::create(
+ fixture.TestWatchedDir,
+ [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
+ bool IsInitial) {
+ TestConsumer.consume(Events, IsInitial);
+ },
+ /*waitForInitialSync=*/true);
+ if (!DW) {
+ logAllUnhandledErrors(
+ DW.takeError(), llvm::errs(),
+ "DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+ exit(EXIT_FAILURE);
+ }
checkEventualResultWithTimeout(TestConsumer);
}
@@ -309,14 +315,20 @@
{EventKind::Modified, "c"}}
};
- auto DW = DirectoryWatcher::create(
- fixture.TestWatchedDir,
- [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
- bool IsInitial) {
- TestConsumer.consume(Events, IsInitial);
- },
- /*waitForInitialSync=*/false);
- if (!DW) return;
+ llvm::Expected<std::unique_ptr<DirectoryWatcher>> DW =
+ DirectoryWatcher::create(
+ fixture.TestWatchedDir,
+ [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
+ bool IsInitial) {
+ TestConsumer.consume(Events, IsInitial);
+ },
+ /*waitForInitialSync=*/false);
+ if (!DW) {
+ logAllUnhandledErrors(
+ DW.takeError(), llvm::errs(),
+ "DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+ exit(EXIT_FAILURE);
+ }
checkEventualResultWithTimeout(TestConsumer);
}
@@ -330,14 +342,20 @@
{EventKind::Modified, "b"},
{EventKind::Modified, "c"}}};
- auto DW = DirectoryWatcher::create(
- fixture.TestWatchedDir,
- [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
- bool IsInitial) {
- TestConsumer.consume(Events, IsInitial);
- },
- /*waitForInitialSync=*/true);
- if (!DW) return;
+ llvm::Expected<std::unique_ptr<DirectoryWatcher>> DW =
+ DirectoryWatcher::create(
+ fixture.TestWatchedDir,
+ [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
+ bool IsInitial) {
+ TestConsumer.consume(Events, IsInitial);
+ },
+ /*waitForInitialSync=*/true);
+ if (!DW) {
+ logAllUnhandledErrors(
+ DW.takeError(), llvm::errs(),
+ "DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+ exit(EXIT_FAILURE);
+ }
fixture.addFile("a");
fixture.addFile("b");
@@ -356,14 +374,20 @@
{{EventKind::Modified, "a"}},
{{EventKind::Modified, "a"}}};
- auto DW = DirectoryWatcher::create(
- fixture.TestWatchedDir,
- [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
- bool IsInitial) {
- TestConsumer.consume(Events, IsInitial);
- },
- /*waitForInitialSync=*/true);
- if (!DW) return;
+ llvm::Expected<std::unique_ptr<DirectoryWatcher>> DW =
+ DirectoryWatcher::create(
+ fixture.TestWatchedDir,
+ [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
+ bool IsInitial) {
+ TestConsumer.consume(Events, IsInitial);
+ },
+ /*waitForInitialSync=*/true);
+ if (!DW) {
+ logAllUnhandledErrors(
+ DW.takeError(), llvm::errs(),
+ "DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+ exit(EXIT_FAILURE);
+ }
// modify the file
{
@@ -387,14 +411,20 @@
{{EventKind::Removed, "a"}},
{{EventKind::Modified, "a"}, {EventKind::Removed, "a"}}};
- auto DW = DirectoryWatcher::create(
- fixture.TestWatchedDir,
- [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
- bool IsInitial) {
- TestConsumer.consume(Events, IsInitial);
- },
- /*waitForInitialSync=*/true);
- if (!DW) return;
+ llvm::Expected<std::unique_ptr<DirectoryWatcher>> DW =
+ DirectoryWatcher::create(
+ fixture.TestWatchedDir,
+ [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
+ bool IsInitial) {
+ TestConsumer.consume(Events, IsInitial);
+ },
+ /*waitForInitialSync=*/true);
+ if (!DW) {
+ logAllUnhandledErrors(
+ DW.takeError(), llvm::errs(),
+ "DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+ exit(EXIT_FAILURE);
+ }
fixture.deleteFile("a");
@@ -409,14 +439,20 @@
{{EventKind::WatchedDirRemoved, ""},
{EventKind::WatcherGotInvalidated, ""}}};
- auto DW = DirectoryWatcher::create(
- fixture.TestWatchedDir,
- [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
- bool IsInitial) {
- TestConsumer.consume(Events, IsInitial);
- },
- /*waitForInitialSync=*/true);
- if (!DW) return;
+ llvm::Expected<std::unique_ptr<DirectoryWatcher>> DW =
+ DirectoryWatcher::create(
+ fixture.TestWatchedDir,
+ [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
+ bool IsInitial) {
+ TestConsumer.consume(Events, IsInitial);
+ },
+ /*waitForInitialSync=*/true);
+ if (!DW) {
+ logAllUnhandledErrors(
+ DW.takeError(), llvm::errs(),
+ "DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+ exit(EXIT_FAILURE);
+ }
remove_directories(fixture.TestWatchedDir);
@@ -430,15 +466,21 @@
{}, {{EventKind::WatcherGotInvalidated, ""}}};
{
- auto DW = DirectoryWatcher::create(
- fixture.TestWatchedDir,
- [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
- bool IsInitial) {
- TestConsumer.consume(Events, IsInitial);
- },
- /*waitForInitialSync=*/true);
- if (!DW) return;
+ llvm::Expected<std::unique_ptr<DirectoryWatcher>> DW =
+ DirectoryWatcher::create(
+ fixture.TestWatchedDir,
+ [&TestConsumer](llvm::ArrayRef<DirectoryWatcher::Event> Events,
+ bool IsInitial) {
+ TestConsumer.consume(Events, IsInitial);
+ },
+ /*waitForInitialSync=*/true);
+ if (!DW) {
+ logAllUnhandledErrors(
+ DW.takeError(), llvm::errs(),
+ "DirectoryWatcherTest Failure on DirectoryWatcher::create(): ");
+ exit(EXIT_FAILURE);
+ }
} // DW is destructed here.
checkEventualResultWithTimeout(TestConsumer);
-}
\ No newline at end of file
+}
Index: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h
===================================================================
--- cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h
+++ cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h
@@ -99,10 +99,10 @@
: Kind(Kind), Filename(Filename) {}
};
- /// Asserts if \param Path doesn't exist or isn't a directory.
+ /// llvm fatal_error if \param Path doesn't exist or isn't a directory.
/// Returns llvm::Expected Error if OS kernel API told us we can't start
/// watching. In such case it's unclear whether just retrying has any chance
- /// to succeeed.
+ /// to succeed.
static llvm::Expected<std::unique_ptr<DirectoryWatcher>>
create(llvm::StringRef Path,
std::function<void(llvm::ArrayRef<DirectoryWatcher::Event> Events,
Index: cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
===================================================================
--- cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
+++ cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
@@ -150,7 +150,8 @@
StringRef Path,
std::function<void(llvm::ArrayRef<DirectoryWatcher::Event>, bool)> Receiver,
dispatch_queue_t Queue) {
- assert(!Path.empty() && "Path.empty()");
+ if (Path.empty())
+ return nullptr;
CFMutableArrayRef PathsToWatch = [&]() {
CFMutableArrayRef PathsToWatch =
@@ -208,7 +209,10 @@
dispatch_queue_t Queue =
dispatch_queue_create("DirectoryWatcher", DISPATCH_QUEUE_SERIAL);
- assert(!Path.empty() && "Path.empty()");
+ if (Path.empty())
+ llvm::report_fatal_error(
+ "DirectoryWatcher::create can not accept an empty Path.");
+
auto EventStream = createFSEventStream(Path, Receiver, Queue);
assert(EventStream && "EventStream expected to be non-null");
Index: cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
===================================================================
--- cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
+++ cfe/trunk/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
@@ -325,7 +325,9 @@
StringRef Path,
std::function<void(llvm::ArrayRef<DirectoryWatcher::Event>, bool)> Receiver,
bool WaitForInitialSync) {
- assert(!Path.empty() && "Path.empty()");
+ if (Path.empty())
+ llvm::report_fatal_error(
+ "DirectoryWatcher::create can not accept an empty Path.");
const int InotifyFD = inotify_init1(IN_CLOEXEC);
if (InotifyFD == -1)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
