[llvm] da45bd2 - [VFS] More consistent support for Windows
Adrian McCarthy via llvm-commits
llvm-commits at lists.llvm.org
Wed Feb 5 11:38:54 PST 2020
Author: Adrian McCarthy
Date: 2020-02-05T11:38:20-08:00
New Revision: da45bd232165eab5d6ec4f1f4f18db8644289142
URL: https://github.com/llvm/llvm-project/commit/da45bd232165eab5d6ec4f1f4f18db8644289142
DIFF: https://github.com/llvm/llvm-project/commit/da45bd232165eab5d6ec4f1f4f18db8644289142.diff
LOG: [VFS] More consistent support for Windows
Removed some #ifdefs specific to Windows handling of VFS paths. This
eliminates most of the differences between the Windows and non-Windows
code paths.
Making this work required some changes to account for the fact that VFS
file paths can be Posix style or Windows style, so you cannot just assume
that they use the host's native path style. In one case, this means
implementing our own version of make_absolute, since the filesystem code
in Support doesn't have styles in the sense that the path code does.
Differential Review: https://reviews.llvm.org/D71092
Added:
Modified:
clang/test/VFS/external-names.c
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Removed:
################################################################################
diff --git a/clang/test/VFS/external-names.c b/clang/test/VFS/external-names.c
index 7e24b5fdf063..c50d240a32eb 100644
--- a/clang/test/VFS/external-names.c
+++ b/clang/test/VFS/external-names.c
@@ -10,7 +10,7 @@
// Preprocessor (__FILE__ macro and # directives):
// RUN: %clang_cc1 -I %t -ivfsoverlay %t.external.yaml -E %s | FileCheck -check-prefix=CHECK-PP-EXTERNAL %s
-// CHECK-PP-EXTERNAL: # {{[0-9]*}} "[[NAME:.*Inputs.external-names.h]]"
+// CHECK-PP-EXTERNAL: # {{[0-9]*}} "[[NAME:.*Inputs(/|\\\\)external-names.h]]"
// CHECK-PP-EXTERNAL-NEXT: void foo(char **c) {
// CHECK-PP-EXTERNAL-NEXT: *c = "[[NAME]]";
diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h
index e45e6e756786..9b8d0574f008 100644
--- a/llvm/include/llvm/Support/VirtualFileSystem.h
+++ b/llvm/include/llvm/Support/VirtualFileSystem.h
@@ -705,16 +705,6 @@ class RedirectingFileSystem : public vfs::FileSystem {
bool IsFallthrough = true;
/// @}
- /// Virtual file paths and external files could be canonicalized without "..",
- /// "." and "./" in their paths. FIXME: some unittests currently fail on
- /// win32 when using remove_dots and remove_leading_dotslash on paths.
- bool UseCanonicalizedPaths =
-#ifdef _WIN32
- false;
-#else
- true;
-#endif
-
RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> ExternalFS);
/// Looks up the path <tt>[Start, End)</tt> in \p From, possibly
diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp
index 321ce144b675..737116ce5ad2 100644
--- a/llvm/lib/Support/VirtualFileSystem.cpp
+++ b/llvm/lib/Support/VirtualFileSystem.cpp
@@ -990,6 +990,28 @@ std::error_code InMemoryFileSystem::isLocal(const Twine &Path, bool &Result) {
// RedirectingFileSystem implementation
//===-----------------------------------------------------------------------===/
+namespace {
+
+/// Removes leading "./" as well as path components like ".." and ".".
+static llvm::SmallString<256> canonicalize(llvm::StringRef Path) {
+ // First detect the path style in use by checking the first separator.
+ llvm::sys::path::Style style = llvm::sys::path::Style::native;
+ const size_t n = Path.find_first_of("/\\");
+ if (n != static_cast<size_t>(-1))
+ style = (Path[n] == '/') ? llvm::sys::path::Style::posix
+ : llvm::sys::path::Style::windows;
+
+ // Now remove the dots. Explicitly specifying the path style prevents the
+ // direction of the slashes from changing.
+ llvm::SmallString<256> result =
+ llvm::sys::path::remove_leading_dotslash(Path, style);
+ llvm::sys::path::remove_dots(result, /*remove_dot_dot=*/true, style);
+ return result;
+}
+
+} // anonymous namespace
+
+
RedirectingFileSystem::RedirectingFileSystem(IntrusiveRefCntPtr<FileSystem> FS)
: ExternalFS(std::move(FS)) {
if (ExternalFS)
@@ -1083,7 +1105,23 @@ std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char> &Path)
if (!WorkingDir)
return WorkingDir.getError();
- llvm::sys::fs::make_absolute(WorkingDir.get(), Path);
+ // We can't use sys::fs::make_absolute because that assumes the path style
+ // is native and there is no way to override that. Since we know WorkingDir
+ // is absolute, we can use it to determine which style we actually have and
+ // append Path ourselves.
+ sys::path::Style style = sys::path::Style::windows;
+ if (sys::path::is_absolute(WorkingDir.get(), sys::path::Style::posix)) {
+ style = sys::path::Style::posix;
+ }
+
+ std::string Result = WorkingDir.get();
+ StringRef Dir(Result);
+ if (!Dir.endswith(sys::path::get_separator(style))) {
+ Result += sys::path::get_separator(style);
+ }
+ Result.append(Path.data(), Path.size());
+ Path.assign(Result.begin(), Result.end());
+
return {};
}
@@ -1318,8 +1356,8 @@ class llvm::vfs::RedirectingFileSystemParser {
bool HasContents = false; // external or otherwise
std::vector<std::unique_ptr<RedirectingFileSystem::Entry>>
EntryArrayContents;
- std::string ExternalContentsPath;
- std::string Name;
+ SmallString<256> ExternalContentsPath;
+ SmallString<256> Name;
yaml::Node *NameValueNode = nullptr;
auto UseExternalName =
RedirectingFileSystem::RedirectingFileEntry::NK_NotSet;
@@ -1342,16 +1380,9 @@ class llvm::vfs::RedirectingFileSystemParser {
return nullptr;
NameValueNode = I.getValue();
- if (FS->UseCanonicalizedPaths) {
- SmallString<256> Path(Value);
- // Guarantee that old YAML files containing paths with ".." and "."
- // are properly canonicalized before read into the VFS.
- Path = sys::path::remove_leading_dotslash(Path);
- sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
- Name = std::string(Path.str());
- } else {
- Name = std::string(Value);
- }
+ // Guarantee that old YAML files containing paths with ".." and "."
+ // are properly canonicalized before read into the VFS.
+ Name = canonicalize(Value).str();
} else if (Key == "type") {
if (!parseScalarString(I.getValue(), Value, Buffer))
return nullptr;
@@ -1404,13 +1435,10 @@ class llvm::vfs::RedirectingFileSystemParser {
FullPath = Value;
}
- if (FS->UseCanonicalizedPaths) {
- // Guarantee that old YAML files containing paths with ".." and "."
- // are properly canonicalized before read into the VFS.
- FullPath = sys::path::remove_leading_dotslash(FullPath);
- sys::path::remove_dots(FullPath, /*remove_dot_dot=*/true);
- }
- ExternalContentsPath = std::string(FullPath.str());
+ // Guarantee that old YAML files containing paths with ".." and "."
+ // are properly canonicalized before read into the VFS.
+ FullPath = canonicalize(FullPath);
+ ExternalContentsPath = FullPath.str();
} else if (Key == "use-external-name") {
bool Val;
if (!parseScalarBool(I.getValue(), Val))
@@ -1654,14 +1682,10 @@ RedirectingFileSystem::lookupPath(const Twine &Path_) const {
if (std::error_code EC = makeAbsolute(Path))
return EC;
- // Canonicalize path by removing ".", "..", "./", etc components. This is
- // a VFS request, do bot bother about symlinks in the path components
+ // Canonicalize path by removing ".", "..", "./", components. This is
+ // a VFS request, do not bother about symlinks in the path components
// but canonicalize in order to perform the correct entry search.
- if (UseCanonicalizedPaths) {
- Path = sys::path::remove_leading_dotslash(Path);
- sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
- }
-
+ Path = canonicalize(Path);
if (Path.empty())
return make_error_code(llvm::errc::invalid_argument);
@@ -1680,16 +1704,9 @@ ErrorOr<RedirectingFileSystem::Entry *>
RedirectingFileSystem::lookupPath(sys::path::const_iterator Start,
sys::path::const_iterator End,
RedirectingFileSystem::Entry *From) const {
-#ifndef _WIN32
assert(!isTraversalComponent(*Start) &&
!isTraversalComponent(From->getName()) &&
"Paths should not contain traversal components");
-#else
- // FIXME: this is here to support windows, remove it once canonicalized
- // paths become globally default.
- if (Start->equals("."))
- ++Start;
-#endif
StringRef FromName = From->getName();
diff --git a/llvm/unittests/Support/VirtualFileSystemTest.cpp b/llvm/unittests/Support/VirtualFileSystemTest.cpp
index 7c0689745e26..acd526409f2f 100644
--- a/llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ b/llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -2148,11 +2148,9 @@ TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthrough) {
ASSERT_FALSE(Status.getError());
EXPECT_TRUE(Status->exists());
-#if !defined(_WIN32)
Status = FS->status("../bar/baz/a");
ASSERT_FALSE(Status.getError());
EXPECT_TRUE(Status->exists());
-#endif
}
TEST_F(VFSFromYAMLTest, WorkingDirectoryFallthroughInvalid) {
More information about the llvm-commits
mailing list