[llvm] Support: unlock Windows API support, switch to Windows 10 RS1+ APIs (PR #102240)

[Martin Storsjö via llvm-commits]

================
@@ -466,21 +466,41 @@ static std::error_code rename_internal(HANDLE FromHandle, const Twine &To,
                                   (ToWide.size() * sizeof(wchar_t)));
   FILE_RENAME_INFO &RenameInfo =
       *reinterpret_cast<FILE_RENAME_INFO *>(RenameInfoBuf.data());
-  RenameInfo.ReplaceIfExists = ReplaceIfExists;
-  RenameInfo.RootDirectory = 0;
-  RenameInfo.FileNameLength = ToWide.size() * sizeof(wchar_t);
-  std::copy(ToWide.begin(), ToWide.end(), &RenameInfo.FileName[0]);
-
-  SetLastError(ERROR_SUCCESS);
-  if (!SetFileInformationByHandle(FromHandle, FileRenameInfo, &RenameInfo,
-                                  RenameInfoBuf.size())) {
-    unsigned Error = GetLastError();
-    if (Error == ERROR_SUCCESS)
-      Error = ERROR_CALL_NOT_IMPLEMENTED; // Wine doesn't always set error code.
-    return mapWindowsError(Error);
+
+  static const struct RenameStyle {
+    DWORD dwFlag;
+    FILE_INFO_BY_HANDLE_CLASS Class;
+  } kRenameStyles[] = {
+    { FILE_RENAME_FLAG_POSIX_SEMANTICS, FileRenameInfoEx },
+    { 0, FileRenameInfo },
+  };
+
+  DWORD dwError = ERROR_SUCCESS;
+  for (const auto &style : kRenameStyles) {
+    RenameInfo.Flags = style.dwFlag |
+                       (ReplaceIfExists ? FILE_RENAME_FLAG_REPLACE_IF_EXISTS
+                                        : 0);
+    RenameInfo.RootDirectory = 0;
+    RenameInfo.FileNameLength = ToWide.size() * sizeof(wchar_t);
+    std::copy(ToWide.begin(), ToWide.end(), &RenameInfo.FileName[0]);
+
+    SetLastError(ERROR_SUCCESS);
+    if (!SetFileInformationByHandle(FromHandle, style.Class, &RenameInfo,
+                                    RenameInfoBuf.size())) {
+      dwError = GetLastError();
+      if (dwError == ERROR_SUCCESS)
+        dwError = ERROR_CALL_NOT_IMPLEMENTED; // Wine doesn't always set error code.
+      // Retry with a different style if we do not support this style.
+      if (dwError == ERROR_INVALID_PARAMETER ||
+          dwError == ERROR_CALL_NOT_IMPLEMENTED)
+        continue;
+      return mapWindowsError(Error);
+    }
   }
 
-  return std::error_code();
+  if (dwError == ERROR_SUCCESS)
+    return std::error_code();
+  return mapWindowsError(Error);
----------------
mstorsjo wrote:

Shouldn't you move the `return std::error_code();` (without a need for checking `dwError`) to within the for loop? If `SetFileInformationByHandle` did succeed, we should return success, not proceed to the next iteration in the for loop.

And if we did fail on the first iteration, due to `ERROR_INVALID_PARAMETER`, then we won't touch `dwError` on the second iteration, so we'd leave `dwError` untouched (i.e. containing the error value) until the end. So I think we should keep the `dwError` variable within the `if (!SetFileInformationByHandle())` block.

(Also, does this compile at all? You have mixed references to both `Error` and `dwError`.)

https://github.com/llvm/llvm-project/pull/102240