From: Puyan Lotfi <puyan@puyan.org>
Date: Tue, 6 Aug 2019 23:25:34 +0000 (+0000)
Subject: [clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in create
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=c3583ea32ed5e0fb1aed4a983af121cd71f21ec2;p=clang

[clang][DirectoryWatcher][NFC] Swapping asserts for llvm fatal_error in create

I also have 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 Expected.

I've also allowed for logAllUnhandledErrors to consume errors in the case were
DirectoryWatcher::create produces them.

Differential Revision: https://reviews.llvm.org/D65829




git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@368108 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/DirectoryWatcher/DirectoryWatcher.h b/include/clang/DirectoryWatcher/DirectoryWatcher.h
index d2a1662163..4475807dfc 100644
--- a/include/clang/DirectoryWatcher/DirectoryWatcher.h
+++ b/include/clang/DirectoryWatcher/DirectoryWatcher.h
@@ -99,10 +99,10 @@ public:
         : 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,
diff --git a/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp b/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
index 8a5e76c521..865c2b33bf 100644
--- a/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
+++ b/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp
@@ -325,7 +325,9 @@ llvm::Expected<std::unique_ptr<DirectoryWatcher>> clang::DirectoryWatcher::creat
     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)
diff --git a/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp b/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
index 473bda7cd9..0a7a0eaef0 100644
--- a/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
+++ b/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
@@ -150,7 +150,8 @@ FSEventStreamRef createFSEventStream(
     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 @@ llvm::Expected<std::unique_ptr<DirectoryWatcher>> clang::DirectoryWatcher::creat
   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");
 
diff --git a/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp b/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
index 14ef346032..485f0eeab0 100644
--- a/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
+++ b/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp
@@ -277,14 +277,20 @@ TEST(DirectoryWatcherTest, InitialScanSync) {
        {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 @@ TEST(DirectoryWatcherTest, InitialScanAsync) {
        {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 @@ TEST(DirectoryWatcherTest, AddFiles) {
        {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 @@ TEST(DirectoryWatcherTest, ModifyFile) {
       {{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 @@ TEST(DirectoryWatcherTest, DeleteFile) {
       {{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 @@ TEST(DirectoryWatcherTest, DeleteWatchedDir) {
       {{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 @@ TEST(DirectoryWatcherTest, InvalidatedWatcher) {
       {}, {{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
+}