[PATCH] D58418: [clang][DirectoryWatcher] Upstream DirectoryWatcher

Michał Górny via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 22 23:13:14 PST 2019


mgorny added inline comments.


================
Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:196
+  while (true) {
+    if (close(inotifyFD) == -1 && errno == EINTR)
+      continue;
----------------
There's some fancy function for this in LLVM. `RetryAfterSignal`, I think.


================
Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher-linux.inc.h:135
+        if (!statusOpt.hasValue())
+          K = DirectoryWatcher::EventKind::Removed;
+      }
----------------
jkorous wrote:
> mgorny wrote:
> > Why? I suppose this deserves a comment.
> I'll add this comment:
> 
> // The file might have been removed just after we received the event.
Wouldn't that cause removals to be reported twice?


================
Comment at: clang/lib/DirectoryWatcher/DirectoryWatcher.cpp:95
+
+#if !defined(__has_include)
+#define __has_include(x) 0
----------------
jkorous wrote:
> mgorny wrote:
> > Why not use CMake checks? You're already partially using them for frameworks.
> Do I understand correctly that you suggest doing the check in CMake and propagating the result to preprocessor?
Yes, using `check_include_file` and declaring `HAVE_SOMETHING` in config.h or alike.


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

https://reviews.llvm.org/D58418





More information about the cfe-commits mailing list