[PATCH] D75372: [Support] Improve Windows widenPath and add support for long UNC paths

Reid Kleckner via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 28 13:51:12 PST 2020


rnk added inline comments.


================
Comment at: llvm/include/llvm/Support/FileSystem.h:1161
+/// File API.
+std::error_code widenPath(const Twine &Path8, SmallVectorImpl<wchar_t> &Path16,
+                          bool ForCreateDir = false);
----------------
Let's move this somewhere more obscure so clients like llvm-ar are less likely to find it and try to use it. I'd suggest WindowsSupport.h. Ideally we would make it local to Support, but we don't have any existing internal support headers.


================
Comment at: llvm/lib/Support/Windows/Path.inc:82
+  size_t MaxDirLen =
+      ForCreateDir        // Win32 CreateDirectory API has a lower limit.
+          ? MAX_PATH - 12 // Must leave room for 8.3 filename.
----------------
Why not always use the lower limit? What is the harm in using the UNC prefixes slightly more often?


================
Comment at: llvm/unittests/Support/Path.cpp:1887
+  std::string Input("C:\\foldername\\");
+  std::string Pi("π"); // UTF-8: 0xCF 0x80
+  // Add Pi up to the MAX_PATH limit.
----------------
Is this the first introduction of a real UTF-8 source character? Are you confident that this will work portably? Do you need to write `u8"pi"` to make this work where utf-8 is not the local codepage?


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

https://reviews.llvm.org/D75372





More information about the llvm-commits mailing list