gribozavr added inline comments.
================
Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:287
/*waitForInitialSync=*/true);
+ if (!DW) return;
----------------
plotfi wrote:
> 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.
That's good -- but I think a `cantFail` call would be more readable.
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65704/new/
https://reviews.llvm.org/D65704
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
