plotfi created this revision.
plotfi added reviewers: jkorous, compnerd, gribozavr.
Herald added subscribers: cfe-commits, dexonsmith.
Herald added a project: clang.
Also I went ahead and replaced all the instances of "auto DW =
DirectoryWatcher::create" with
"llvm::Expected<std::unique_ptr<DirectoryWatcher>> DW =
DirectoryWatcher::create(" to make it more clear that DirectoryWatcher::create
is returning an llvm::Expected (along wit a comment to make it extra clear).
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65829
Files:
clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
Index: clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
===================================================================
--- clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ clang/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -277,14 +277,17 @@
{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);
+ // llvm::Expected will throw an error if DW is an Error.
+ if (!DW)
+ return;
checkEventualResultWithTimeout(TestConsumer);
}
@@ -309,14 +312,17 @@
{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);
+ // llvm::Expected will throw an error if DW is an Error.
+ if (!DW)
+ return;
checkEventualResultWithTimeout(TestConsumer);
}
@@ -330,14 +336,17 @@
{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);
+ // llvm::Expected will throw an error if DW is an Error.
+ if (!DW)
+ return;
fixture.addFile("a");
fixture.addFile("b");
@@ -356,14 +365,17 @@
{{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);
+ // llvm::Expected will throw an error if DW is an Error.
+ if (!DW)
+ return;
// modify the file
{
@@ -387,14 +399,17 @@
{{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);
+ // llvm::Expected will throw an error if DW is an Error.
+ if (!DW)
+ return;
fixture.deleteFile("a");
@@ -409,14 +424,17 @@
{{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);
+ // llvm::Expected will throw an error if DW is an Error.
+ if (!DW)
+ return;
remove_directories(fixture.TestWatchedDir);
@@ -430,15 +448,18 @@
{}, {{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);
+ // llvm::Expected will throw an error if DW is an Error.
+ if (!DW)
+ return;
} // DW is destructed here.
checkEventualResultWithTimeout(TestConsumer);
-}
\ No newline at end of file
+}
Index: clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
===================================================================
--- clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
+++ clang/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: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
===================================================================
--- clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
+++ clang/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)
Index: clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
===================================================================
--- clang/include/clang/DirectoryWatcher/DirectoryWatcher.h
+++ clang/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,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
