amccarth updated this revision to Diff 242190.
amccarth marked an inline comment as done.
amccarth added a comment.
Addressed rnk's comments: fixed path style detection bug, and used
`StringRef::endswith`.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71092/new/
https://reviews.llvm.org/D71092
Files:
clang/test/VFS/external-names.c
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/VirtualFileSystem.cpp
llvm/unittests/Support/VirtualFileSystemTest.cpp
Index: llvm/unittests/Support/VirtualFileSystemTest.cpp
===================================================================
--- llvm/unittests/Support/VirtualFileSystemTest.cpp
+++ llvm/unittests/Support/VirtualFileSystemTest.cpp
@@ -2148,11 +2148,9 @@
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) {
Index: llvm/lib/Support/VirtualFileSystem.cpp
===================================================================
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -990,6 +990,28 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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();
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===================================================================
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -705,16 +705,6 @@
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
Index: clang/test/VFS/external-names.c
===================================================================
--- clang/test/VFS/external-names.c
+++ 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]]";
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
