[PATCH] D71092: [VFS] More consistent support for Windows

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth marked 3 inline comments as done.
amccarth added inline comments.


================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1111-1113
+  sys::path::Style style = sys::path::Style::posix;
+  if (sys::path::is_absolute(WorkingDir.get(), sys::path::Style::windows)) {
+    style = sys::path::Style::posix;
----------------
rnk wrote:
> It looks like you assign posix style if the working directory has windows path style. Is that what you meant to do?
Oops!  Nice catch.

Although some of the tests exercise this path, none of them caught the problem because the effect here is very minor (direction of a slash).  I've turned the logic around, which makes the more common case (posix) dependent upon the condition.


================
Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1117
+  std::string Result = WorkingDir.get();
+  if (Result.empty() || Result.back() != sys::path::get_separator(style)[0]) {
+    Result += sys::path::get_separator(style);
----------------
rnk wrote:
> `Result.endswith(sys::path::get_separator(style))` saves a boolean condition
I hadn't realized `ends_with` was added to `std::string`.  But it's a C++20 enhancement, which I think is too new for us right now.  I'll use `StringRef::endswith`.


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

https://reviews.llvm.org/D71092