compnerd updated this revision to Diff 296092.
compnerd added a comment.
Herald added a subscriber: mgorny.
Address feedback
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88666/new/
https://reviews.llvm.org/D88666
Files:
clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
clang/unittests/DirectoryWatcher/CMakeLists.txt
Index: clang/unittests/DirectoryWatcher/CMakeLists.txt
===================================================================
--- clang/unittests/DirectoryWatcher/CMakeLists.txt
+++ clang/unittests/DirectoryWatcher/CMakeLists.txt
@@ -1,4 +1,4 @@
-if(APPLE OR CMAKE_SYSTEM_NAME MATCHES "Linux")
+if(APPLE OR CMAKE_SYSTEM_NAME MATCHES "Linux" OR CMAKE_SYSTEM_NAME STREQUAL Windows)
set(LLVM_LINK_COMPONENTS
Support
Index: clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
===================================================================
--- clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
+++ clang/lib/DirectoryWatcher/windows/DirectoryWatcher-windows.cpp
@@ -6,18 +6,12 @@
//
//===----------------------------------------------------------------------===//
-// TODO: This is not yet an implementation, but it will make it so Windows
-// builds don't fail.
-
#include "DirectoryScanner.h"
#include "clang/DirectoryWatcher/DirectoryWatcher.h"
-
#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/ScopeExit.h"
-#include "llvm/Support/AlignOf.h"
-#include "llvm/Support/Errno.h"
-#include "llvm/Support/Mutex.h"
+#include "llvm/Support/ConvertUTF.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/Windows/WindowsSupport.h"
#include <atomic>
#include <condition_variable>
#include <mutex>
@@ -26,25 +20,240 @@
#include <thread>
#include <vector>
+
namespace {
+using DirectoryWatcherCallback =
+ std::function<void(llvm::ArrayRef<clang::DirectoryWatcher::Event>, bool)>;
+
using namespace llvm;
using namespace clang;
class DirectoryWatcherWindows : public clang::DirectoryWatcher {
+ OVERLAPPED Overlapped;
+
+ alignas(DWORD)
+ CHAR Notifications[4 * (sizeof(FILE_NOTIFY_INFORMATION) + MAX_PATH * sizeof(WCHAR))];
+
+ std::thread WatcherThread;
+ std::thread HandlerThread;
+ std::function<void(ArrayRef<DirectoryWatcher::Event>, bool)> Callback;
+ SmallString<MAX_PATH> Path;
+
+ class EventQueue {
+ std::mutex M;
+ std::queue<DirectoryWatcher::Event> Q;
+ std::condition_variable CV;
+
+ public:
+ void emplace(DirectoryWatcher::Event::EventKind Kind, StringRef Path) {
+ {
+ std::unique_lock<std::mutex> L(M);
+ Q.emplace(Kind, Path);
+ }
+ CV.notify_one();
+ }
+
+ DirectoryWatcher::Event pop_front() {
+ std::unique_lock<std::mutex> L(M);
+ while (true) {
+ if (!Q.empty()) {
+ DirectoryWatcher::Event E = Q.front();
+ Q.pop();
+ return E;
+ }
+ CV.wait(L, [this]() { return !Q.empty(); });
+ }
+ }
+ } Q;
+
public:
- ~DirectoryWatcherWindows() override { }
- void InitialScan() { }
- void EventReceivingLoop() { }
- void StopWork() { }
+ DirectoryWatcherWindows(HANDLE DirectoryHandle, bool WaitForInitialSync,
+ DirectoryWatcherCallback Receiver);
+
+ ~DirectoryWatcherWindows() override;
+
+ void InitialScan();
};
+
+DirectoryWatcherWindows::DirectoryWatcherWindows(
+ HANDLE DirectoryHandle, bool WaitForInitialSync,
+ DirectoryWatcherCallback Receiver)
+ : Callback(Receiver) {
+ // Pre-compute the real location as we will be handing over the directory
+ // handle to the watcher and performing synchronous operations.
+ {
+ DWORD Length = GetFinalPathNameByHandleW(DirectoryHandle, NULL, 0, 0);
+
+ std::vector<WCHAR> Buffer;
+ Buffer.reserve(Length);
+
+ Length = GetFinalPathNameByHandleW(DirectoryHandle, Buffer.data(),
+ Buffer.capacity(), 0);
+ Buffer.resize(Length);
+
+ llvm::sys::windows::UTF16ToUTF8(Buffer.data(), Buffer.size(), Path);
+ }
+
+ memset(&Overlapped, 0, sizeof(Overlapped));
+ Overlapped.hEvent =
+ CreateEventW(NULL, /*bManualReset=*/TRUE, /*bInitialState=*/FALSE, NULL);
+ assert(Overlapped.hEvent && "unable to create event");
+
+ WatcherThread = std::thread([this, DirectoryHandle]() {
+ while (true) {
+ // We do not guarantee subdirectories, but macOS already provides
+ // subdirectories, might as well as ...
+ BOOL WatchSubtree = TRUE;
+ DWORD NotifyFilter = FILE_NOTIFY_CHANGE_FILE_NAME
+ | FILE_NOTIFY_CHANGE_DIR_NAME
+ | FILE_NOTIFY_CHANGE_SIZE
+ | FILE_NOTIFY_CHANGE_LAST_ACCESS
+ | FILE_NOTIFY_CHANGE_LAST_WRITE
+ | FILE_NOTIFY_CHANGE_CREATION;
+
+ DWORD BytesTransferred;
+ if (!ReadDirectoryChangesW(DirectoryHandle, Notifications,
+ sizeof(Notifications), WatchSubtree,
+ NotifyFilter, &BytesTransferred, &Overlapped,
+ NULL)) {
+ Q.emplace(DirectoryWatcher::Event::EventKind::WatcherGotInvalidated,
+ "");
+ break;
+ }
+
+ if (!GetOverlappedResult(DirectoryHandle, &Overlapped, &BytesTransferred,
+ TRUE)) {
+ Q.emplace(DirectoryWatcher::Event::EventKind::WatcherGotInvalidated,
+ "");
+ break;
+ }
+
+ // There was a buffer underrun on the kernel side. We may have lost
+ // events, please re-synchronize.
+ if (BytesTransferred == 0) {
+ Q.emplace(DirectoryWatcher::Event::EventKind::WatcherGotInvalidated,
+ "");
+ break;
+ }
+
+ for (FILE_NOTIFY_INFORMATION *I = (FILE_NOTIFY_INFORMATION *)Notifications;
+ I; I = I->NextEntryOffset
+ ? (FILE_NOTIFY_INFORMATION *)((CHAR *)I + I->NextEntryOffset)
+ : NULL) {
+
+ DirectoryWatcher::Event::EventKind Kind =
+ DirectoryWatcher::Event::EventKind::WatcherGotInvalidated;
+ switch (I->Action) {
+ case FILE_ACTION_MODIFIED:
+ Kind = DirectoryWatcher::Event::EventKind::Modified;
+ break;
+ case FILE_ACTION_ADDED:
+ Kind = DirectoryWatcher::Event::EventKind::Modified;
+ break;
+ case FILE_ACTION_REMOVED:
+ Kind = DirectoryWatcher::Event::EventKind::Removed;
+ break;
+ case FILE_ACTION_RENAMED_OLD_NAME:
+ Kind = DirectoryWatcher::Event::EventKind::Removed;
+ break;
+ case FILE_ACTION_RENAMED_NEW_NAME:
+ Kind = DirectoryWatcher::Event::EventKind::Modified;
+ break;
+ }
+
+ SmallString<MAX_PATH> filename;
+ sys::windows::UTF16ToUTF8(I->FileName, I->FileNameLength / 2,
+ filename);
+ Q.emplace(Kind, filename);
+ }
+ }
+
+ (void)CloseHandle(DirectoryHandle);
+ });
+
+ if (WaitForInitialSync)
+ InitialScan();
+
+ HandlerThread = std::thread([this, WaitForInitialSync]() {
+ // If we did not wait for the initial sync, then we should perform the
+ // scan when we enter the thread.
+ if (!WaitForInitialSync)
+ this->InitialScan();
+
+ while (true) {
+ DirectoryWatcher::Event E = Q.pop_front();
+ Callback(E, /*IsInitial=*/false);
+ if (E.Kind == DirectoryWatcher::Event::EventKind::WatcherGotInvalidated)
+ break;
+ }
+ });
+}
+
+DirectoryWatcherWindows::~DirectoryWatcherWindows() {
+ // Signal the Watcher to exit.
+ SetEvent(Overlapped.hEvent);
+ HandlerThread.join();
+ WatcherThread.join();
+ CloseHandle(Overlapped.hEvent);
+}
+
+void DirectoryWatcherWindows::InitialScan() {
+ Callback(getAsFileEvents(scanDirectory(Path.data())), /*IsInitial=*/true);
+}
+
+auto error(DWORD ErrorCode) {
+ DWORD Flags = FORMAT_MESSAGE_ALLOCATE_BUFFER
+ | FORMAT_MESSAGE_FROM_SYSTEM
+ | FORMAT_MESSAGE_IGNORE_INSERTS;
+
+ LPSTR Buffer;
+ if (!FormatMessageA(Flags, NULL, ErrorCode,
+ MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPSTR)&Buffer,
+ 0, NULL)) {
+ return make_error<llvm::StringError>("error " + utostr(ErrorCode),
+ inconvertibleErrorCode());
+ }
+ std::string Message{Buffer};
+ LocalFree(Buffer);
+ return make_error<llvm::StringError>(Message, inconvertibleErrorCode());
+}
+
} // namespace
llvm::Expected<std::unique_ptr<DirectoryWatcher>>
-clang::DirectoryWatcher::create(
- StringRef Path,
- std::function<void(llvm::ArrayRef<DirectoryWatcher::Event>, bool)> Receiver,
- bool WaitForInitialSync) {
- return llvm::Expected<std::unique_ptr<DirectoryWatcher>>(
- llvm::errorCodeToError(std::make_error_code(std::errc::not_supported)));
+clang::DirectoryWatcher::create(StringRef Path,
+ DirectoryWatcherCallback Receiver,
+ bool WaitForInitialSync) {
+ if (Path.empty())
+ llvm::report_fatal_error(
+ "DirectoryWatcher::create can not accept an empty Path.");
+
+ if (!sys::fs::is_directory(Path))
+ llvm::report_fatal_error(
+ "DirectoryWatcher::create can not accept a filepath.");
+
+ SmallVector<wchar_t, MAX_PATH> WidePath;
+ if (sys::windows::UTF8ToUTF16(Path, WidePath))
+ return llvm::make_error<llvm::StringError>(
+ "unable to convert path to UTF-16", llvm::inconvertibleErrorCode());
+
+ DWORD DesiredAccess = FILE_LIST_DIRECTORY;
+ DWORD ShareMode = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE;
+ DWORD CreationDisposition = OPEN_EXISTING;
+ DWORD FlagsAndAttributes = FILE_FLAG_BACKUP_SEMANTICS | FILE_FLAG_OVERLAPPED;
+
+ HANDLE DirectoryHandle =
+ CreateFileW(WidePath.data(), DesiredAccess, ShareMode,
+ /*lpSecurityAttributes=*/NULL, CreationDisposition,
+ FlagsAndAttributes, NULL);
+ if (DirectoryHandle == INVALID_HANDLE_VALUE)
+ return error(GetLastError());
+
+ // NOTE: We use the watcher instance as a RAII object to discard the handles
+ // for the directory and the IOCP in case of an error. Hence, this is early
+ // allocated, with the state being written directly to the watcher.
+ return std::make_unique<DirectoryWatcherWindows>(DirectoryHandle,
+ WaitForInitialSync,
+ Receiver);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
