[PATCH] D65704: DirectoryWatcher::create: Adding better error handling.

Puyan Lotfi via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 6 12:52:03 PDT 2019


plotfi marked an inline comment as done.
plotfi added inline comments.


================
Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:287
       /*waitForInitialSync=*/true);
+  if (!DW) return;
 
----------------
gribozavr wrote:
> plotfi wrote:
> > gribozavr wrote:
> > > Why? This is a test -- ignoring errors would only hide them from developers.
> > Please see how llvm::Expected works. This does not ignore or hide anything. 
> I think I know how llvm::Expected works.
> 
> The problem that this check and return hides is that if DirectoryWatcher::create returns an error, the rest of the test is silently slipped. The test should be using something like `cantFail` instead.
If DirectoryWatcher returns an error and you hit the early return, without taking the error the Expected destructor will print a callstack and halt the program. I can check this again, but I am quite sure that without invoking something like takeError you will hit the crash in the case of an early return. 


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65704/new/

https://reviews.llvm.org/D65704





More information about the cfe-commits mailing list