[PATCH] D30107: Make DynamicLibrary::getPermanentLibrary on Windows have a defined ordering.

[Vedant Kumar via Phabricator via llvm-commits]

vsk added a comment.

Some comments inline. Could you re-upload your diff with context ('git diff -U9999')?



================
Comment at: lib/Support/Windows/DynamicLibrary.inc:38
+static std::unique_ptr<ModuleHandles> OpenedHandles;
+static bool KeepFront;
 
----------------
Please use ManagedStatic.


================
Comment at: lib/Support/Windows/DynamicLibrary.inc:66
+      Current.resize(Bytes / sizeof(HMODULE));
+      if (!EnumProcessModulesEx(Self, Current.data(), Bytes, &Bytes, Flags)) {
+        MakeErrMsg(errMsg, "EnumProcessModulesEx failure");
----------------
Could you explain with some more detail why EnumProcessModulesEx needs to be repeated until its results don't change? Is there any guarantee that its results won't change again after this loop terminates?

Apologies if my Windows ignorance is showing here...


================
Comment at: lib/Support/Windows/DynamicLibrary.inc:73
+#ifndef NDEBUG
+    if (OpenedHandles) {
+      auto Begin = Current.begin(), End = Current.end();
----------------
At this point, I'd really expect OpenedHandles to be initialized.


================
Comment at: lib/Support/Windows/DynamicLibrary.inc:106
   }
-  
+
   HMODULE a_handle = LoadLibraryW(filenameUnicode.data());
----------------
Try to avoid unrelated whitespace changes.


https://reviews.llvm.org/D30107