[Support, Windows] Handle long paths with unix separators

[pirama]

Summary

The function widenPath() for Windows also normalizes long path names by
iterating over the path's components and calling append(). The
assumption during the iteration that separators are not returned by the
iterator doesn't hold because the iterators do return a separator when
the path has a drive name. Handle this case by ignoring separators
during iteration.

[pirama]

Merged two calls to SmallString::append() in the test

[rnk]

unittests/Support/Path.cpp

702–703

The test process shouldn't create new directories in C:, or assume that C: exists. You can instead create the directory inside TestDirectory by calling fs::make_absolute on it and replacing all backslashes with slashes.

[pirama]

Test under FileSystemTest's TestDirectory and cleanup.

[pirama]

Ping.

[rnk]

lgtm

[pirama]

Committed. Thanks for the review.

lib/Support/Windows/Path.inc

if ((Path8Str.size() + CurPathLen) >= MaxDirLen &&
       !Path8Str.startswith("\\\\?\\")) {
     SmallString<2*MAX_PATH> FullPath("\\\\?\\");
     if (CurPathLen) {
       SmallString<80> CurPath;
       if (std::error_code EC = llvm::sys::fs::current_path(CurPath))
         return EC;
       FullPath.append(CurPath);
     }
-    // Traverse the requested path, canonicalizing . and .. as we go (because
-    // the \\?\ prefix is documented to treat them as real components).
-    // The iterators don't report separators and append() always attaches
-    // preferred_separator so we don't need to call native() on the result.
+    // Traverse the requested path, canonicalizing . and .. (because the \\?\
+    // prefix is documented to treat them as real components).  Ignore
+    // separators, which can be returned from the iterator if the path has a
+    // drive name.  We don't need to call native() on the result since append()
+    // always attaches preferred_separator.
     for (llvm::sys::path::const_iterator I = llvm::sys::path::begin(Path8Str),
                                          E = llvm::sys::path::end(Path8Str);
                                          I != E; ++I) {
+      if (I->size() == 1 && is_separator((*I)[0]))
+        continue;
       if (I->size() == 1 && *I == ".")
         continue;
       if (I->size() == 2 && *I == "..")
         llvm::sys::path::remove_filename(FullPath);
       else
         llvm::sys::path::append(FullPath, *I);
     }
     return UTF8ToUTF16(FullPath, Path16);

unittests/Support/Path.cpp

ASSERT_EQ(fs::create_directories(Twine(LongDir), false),
             errc::file_exists);
   // Tidy up, "recursively" removing the directories.
   StringRef ThisDir(LongDir);
   for (size_t J = 0; J < NLevels; ++J) {
     ASSERT_NO_ERROR(fs::remove(ThisDir));
     ThisDir = path::parent_path(ThisDir);
   }
 
+  // Also verify that paths with Unix separators are handled correctly.
+  std::string LongPathWithUnixSeparators(TestDirectory.str());
+  // Add at least one subdirectory to TestDirectory, and replace slashes with
+  // backslashes
+  do {
+    LongPathWithUnixSeparators.append("/DirNameWith19Charss");
+  } while (LongPathWithUnixSeparators.size() < 260);
+  std::replace(LongPathWithUnixSeparators.begin(),
+               LongPathWithUnixSeparators.end(),
+               '\\', '/');
+  ASSERT_NO_ERROR(fs::create_directories(Twine(LongPathWithUnixSeparators)));
+  // cleanup
+  ASSERT_NO_ERROR(fs::remove_directories(Twine(TestDirectory) +
+                                         "/DirNameWith19Charss"));
+
   // Similarly for a relative pathname.  Need to set the current directory to
   // TestDirectory so that the one we create ends up in the right place.
   char PreviousDir[260];
   size_t PreviousDirLen = ::GetCurrentDirectoryA(260, PreviousDir);
   ASSERT_GT(PreviousDirLen, 0U);
   ASSERT_LT(PreviousDirLen, 260U);
   ASSERT_NE(::SetCurrentDirectoryA(TestDirectory.c_str()), 0);
   LongDir.clear();