gribozavr added inline comments.
================
Comment at: cfe/trunk/include/clang/DirectoryWatcher/DirectoryWatcher.h:102
- /// Returns nullptr if \param Path doesn't exist or isn't a directory.
- /// Returns nullptr 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.
- static std::unique_ptr<DirectoryWatcher>
+ /// Asserts 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
----------------
I don't see where this logic is implemented.
Also, LLVM is often built with assertions disabled, so "asserts" can't be a
part of the contract.
================
Comment at: cfe/trunk/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp:153
dispatch_queue_t Queue) {
- if (Path.empty())
- return nullptr;
+ assert(!Path.empty() && "Path.empty()");
----------------
No need to repeat the condition in the `&& "..."` part. assert does that by
default. Use an extra string to explain the assertion to the reader.
================
Comment at: cfe/trunk/unittests/DirectoryWatcher/DirectoryWatcherTest.cpp:287
/*waitForInitialSync=*/true);
+ if (!DW) return;
----------------
Why? This is a test -- ignoring errors would only hide them from developers.
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
