dexonsmith created this revision.
dexonsmith added a reviewer: Bigcheese.
Herald added a subscriber: hiraditya.
dexonsmith requested review of this revision.
Herald added projects: clang, LLVM.
Use the new sys::path::system_style() in a few places that need to
detect the system's native path style, removing a few `_WIN32` macro
checks. Added one FIXME to a FileManagerTest that seemed fishy, but
maintained the existing behaviour.
Happy to split this up as much as makes sense. Mainly posting to show examples
for https://reviews.llvm.org/D112288.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D112289
Files:
clang/include/clang/Basic/JsonSupport.h
clang/lib/Basic/FileManager.cpp
clang/lib/Driver/Driver.cpp
clang/lib/Driver/ToolChain.cpp
clang/lib/Lex/PPDirectives.cpp
clang/unittests/Basic/FileManagerTest.cpp
clang/unittests/Driver/ToolChainTest.cpp
clang/unittests/Lex/HeaderSearchTest.cpp
clang/unittests/Tooling/RefactoringTest.cpp
llvm/include/llvm/Support/VirtualFileSystem.h
llvm/lib/Support/GraphWriter.cpp
llvm/tools/lli/lli.cpp
Index: llvm/tools/lli/lli.cpp
===================================================================
--- llvm/tools/lli/lli.cpp
+++ llvm/tools/lli/lli.cpp
@@ -355,13 +355,12 @@
return false;
std::string CacheSubdir = ModID.substr(PrefixLength);
-#if defined(_WIN32)
- // Transform "X:\foo" => "/X\foo" for convenience.
- if (isalpha(CacheSubdir[0]) && CacheSubdir[1] == ':') {
+ // Transform "X:\foo" => "/X\foo" for convenience on Windows.
+ if (sys::path::system_style() == sys::path::Style::windows &&
+ isalpha(CacheSubdir[0]) && CacheSubdir[1] == ':') {
CacheSubdir[1] = CacheSubdir[0];
CacheSubdir[0] = '/';
}
-#endif
CacheName = CacheDir + CacheSubdir;
size_t pos = CacheName.rfind('.');
Index: llvm/lib/Support/GraphWriter.cpp
===================================================================
--- llvm/lib/Support/GraphWriter.cpp
+++ llvm/lib/Support/GraphWriter.cpp
@@ -23,11 +23,12 @@
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/ErrorOr.h"
#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Path.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/raw_ostream.h"
#include <cassert>
-#include <system_error>
#include <string>
+#include <system_error>
#include <vector>
using namespace llvm;
@@ -94,11 +95,9 @@
static std::string replaceIllegalFilenameChars(std::string Filename,
const char ReplacementChar) {
-#ifdef _WIN32
- std::string IllegalChars = "\\/:?\"<>|";
-#else
- std::string IllegalChars = "/";
-#endif
+ std::string IllegalChars =
+ sys::path::system_style() == sys::path::Style::windows ? "\\/:?\"<>|"
+ : "/";
for (char IllegalChar : IllegalChars) {
std::replace(Filename.begin(), Filename.end(), IllegalChar,
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===================================================================
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -788,12 +788,7 @@
/// Whether to perform case-sensitive comparisons.
///
/// Currently, case-insensitive matching only works correctly with ASCII.
- bool CaseSensitive =
-#ifdef _WIN32
- false;
-#else
- true;
-#endif
+ bool CaseSensitive = sys::path::system_style() == sys::path::Style::posix;
/// IsRelativeOverlay marks whether a ExternalContentsPrefixDir path must
/// be prefixed in every 'external-contents' when reading from YAML files.
Index: clang/unittests/Tooling/RefactoringTest.cpp
===================================================================
--- clang/unittests/Tooling/RefactoringTest.cpp
+++ clang/unittests/Tooling/RefactoringTest.cpp
@@ -1031,18 +1031,17 @@
toReplacements({{"", 0, 3, "cc"}, {"", 3, 3, "dd"}}));
}
+static constexpr bool usesWindowsPaths() {
+ return llvm::sys::path::system_style() == llvm::sys::path::Style::windows;
+}
+
TEST(DeduplicateByFileTest, PathsWithDots) {
std::map<std::string, Replacements> FileToReplaces;
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS(
new llvm::vfs::InMemoryFileSystem());
FileManager FileMgr(FileSystemOptions(), VFS);
-#if !defined(_WIN32)
- StringRef Path1 = "a/b/.././c.h";
- StringRef Path2 = "a/c.h";
-#else
- StringRef Path1 = "a\\b\\..\\.\\c.h";
- StringRef Path2 = "a\\c.h";
-#endif
+ StringRef Path1 = usesWindowsPaths() ? "a\\b\\..\\.\\c.h" : "a/b/.././c.h";
+ StringRef Path2 = usesWindowsPaths() ? "a\\c.h" : "a/c.h";
EXPECT_TRUE(VFS->addFile(Path1, 0, llvm::MemoryBuffer::getMemBuffer("")));
EXPECT_TRUE(VFS->addFile(Path2, 0, llvm::MemoryBuffer::getMemBuffer("")));
FileToReplaces[std::string(Path1)] = Replacements();
@@ -1057,13 +1056,8 @@
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS(
new llvm::vfs::InMemoryFileSystem());
FileManager FileMgr(FileSystemOptions(), VFS);
-#if !defined(_WIN32)
- StringRef Path1 = "./a/b/c.h";
- StringRef Path2 = "a/b/c.h";
-#else
- StringRef Path1 = ".\\a\\b\\c.h";
- StringRef Path2 = "a\\b\\c.h";
-#endif
+ StringRef Path1 = usesWindowsPaths() ? ".\\a\\b\\c.h" : "./a/b/c.h";
+ StringRef Path2 = usesWindowsPaths() ? "a\\b\\c.h" : "a/b/c.h";
EXPECT_TRUE(VFS->addFile(Path1, 0, llvm::MemoryBuffer::getMemBuffer("")));
EXPECT_TRUE(VFS->addFile(Path2, 0, llvm::MemoryBuffer::getMemBuffer("")));
FileToReplaces[std::string(Path1)] = Replacements();
@@ -1078,13 +1072,8 @@
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS(
new llvm::vfs::InMemoryFileSystem());
FileManager FileMgr(FileSystemOptions(), VFS);
-#if !defined(_WIN32)
- StringRef Path1 = "./a/b/c.h";
- StringRef Path2 = "a/b/c.h";
-#else
- StringRef Path1 = ".\\a\\b\\c.h";
- StringRef Path2 = "a\\b\\c.h";
-#endif
+ StringRef Path1 = usesWindowsPaths() ? ".\\a\\b\\c.h" : "./a/b/c.h";
+ StringRef Path2 = usesWindowsPaths() ? "a\\b\\c.h" : "a/b/c.h";
FileToReplaces[std::string(Path1)] = Replacements();
FileToReplaces[std::string(Path2)] = Replacements();
FileToReplaces = groupReplacementsByFile(FileMgr, FileToReplaces);
Index: clang/unittests/Lex/HeaderSearchTest.cpp
===================================================================
--- clang/unittests/Lex/HeaderSearchTest.cpp
+++ clang/unittests/Lex/HeaderSearchTest.cpp
@@ -188,11 +188,11 @@
std::string HeaderDirName = "/tmp/Sources/Foo/Headers/";
std::string HeaderName = "Foo.h";
-#ifdef _WIN32
- // Force header path to be absolute on windows.
- // As headermap content should represent absolute locations.
- HeaderDirName = "C:" + HeaderDirName;
-#endif /*_WIN32*/
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows) {
+ // Force header path to be absolute on windows.
+ // As headermap content should represent absolute locations.
+ HeaderDirName = "C:" + HeaderDirName;
+ }
test::HMapFileMockMaker<FileTy> Maker(File);
auto a = Maker.addString("Foo/Foo.h");
Index: clang/unittests/Driver/ToolChainTest.cpp
===================================================================
--- clang/unittests/Driver/ToolChainTest.cpp
+++ clang/unittests/Driver/ToolChainTest.cpp
@@ -85,9 +85,8 @@
llvm::raw_string_ostream OS(S);
C->getDefaultToolChain().printVerboseInfo(OS);
}
-#if _WIN32
- std::replace(S.begin(), S.end(), '\\', '/');
-#endif
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows)
+ std::replace(S.begin(), S.end(), '\\', '/');
EXPECT_EQ(
"Found candidate GCC installation: "
"/usr/lib/gcc/arm-linux-gnueabihf/4.6.3\n"
@@ -110,9 +109,8 @@
llvm::raw_string_ostream OS(S);
C->getDefaultToolChain().printVerboseInfo(OS);
}
-#if _WIN32
- std::replace(S.begin(), S.end(), '\\', '/');
-#endif
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows)
+ std::replace(S.begin(), S.end(), '\\', '/');
// Test that 4.5.3 from --sysroot is not overridden by 4.6.3 (larger
// version) from /usr.
EXPECT_EQ("Found candidate GCC installation: "
@@ -153,9 +151,8 @@
llvm::raw_string_ostream OS(S);
C->getDefaultToolChain().printVerboseInfo(OS);
}
-#if _WIN32
- std::replace(S.begin(), S.end(), '\\', '/');
-#endif
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows)
+ std::replace(S.begin(), S.end(), '\\', '/');
EXPECT_EQ("Found candidate GCC installation: "
"/home/test/bin/../lib/gcc/arm-linux-gnueabi/4.6.1\n"
"Selected GCC installation: "
Index: clang/unittests/Basic/FileManagerTest.cpp
===================================================================
--- clang/unittests/Basic/FileManagerTest.cpp
+++ clang/unittests/Basic/FileManagerTest.cpp
@@ -30,18 +30,18 @@
void InjectFileOrDirectory(const char *Path, ino_t INode, bool IsFile,
const char *StatPath) {
-#ifndef _WIN32
SmallString<128> NormalizedPath(Path);
- llvm::sys::path::native(NormalizedPath);
- Path = NormalizedPath.c_str();
-
SmallString<128> NormalizedStatPath;
- if (StatPath) {
- NormalizedStatPath = StatPath;
- llvm::sys::path::native(NormalizedStatPath);
- StatPath = NormalizedStatPath.c_str();
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows) {
+ llvm::sys::path::native(NormalizedPath);
+ Path = NormalizedPath.c_str();
+
+ if (StatPath) {
+ NormalizedStatPath = StatPath;
+ llvm::sys::path::native(NormalizedStatPath);
+ StatPath = NormalizedStatPath.c_str();
+ }
}
-#endif
if (!StatPath)
StatPath = Path;
@@ -73,11 +73,11 @@
bool isFile,
std::unique_ptr<llvm::vfs::File> *F,
llvm::vfs::FileSystem &FS) override {
-#ifndef _WIN32
SmallString<128> NormalizedPath(Path);
- llvm::sys::path::native(NormalizedPath);
- Path = NormalizedPath.c_str();
-#endif
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows) {
+ llvm::sys::path::native(NormalizedPath);
+ Path = NormalizedPath.c_str();
+ }
if (StatCalls.count(Path) != 0) {
Status = StatCalls[Path];
@@ -435,13 +435,19 @@
#endif // !_WIN32
+static StringRef getSystemRoot() {
+ return llvm::sys::path::system_style() == llvm::sys::path::Style::windows
+ ? "C:/"
+ : "/";
+}
+
TEST_F(FileManagerTest, makeAbsoluteUsesVFS) {
- SmallString<64> CustomWorkingDir;
-#ifdef _WIN32
- CustomWorkingDir = "C:";
-#else
- CustomWorkingDir = "/";
-#endif
+ // FIXME: Should this be using a root path / call getSystemRoot()? For now,
+ // avoiding that and leaving the test as-is.
+ SmallString<64> CustomWorkingDir =
+ llvm::sys::path::system_style() == llvm::sys::path::Style::windows
+ ? StringRef("C:")
+ : StringRef("/");
llvm::sys::path::append(CustomWorkingDir, "some", "weird", "path");
auto FS = IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>(
@@ -463,12 +469,7 @@
// getVirtualFile should always fill the real path.
TEST_F(FileManagerTest, getVirtualFileFillsRealPathName) {
- SmallString<64> CustomWorkingDir;
-#ifdef _WIN32
- CustomWorkingDir = "C:/";
-#else
- CustomWorkingDir = "/";
-#endif
+ SmallString<64> CustomWorkingDir = getSystemRoot();
auto FS = IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>(
new llvm::vfs::InMemoryFileSystem);
@@ -496,12 +497,7 @@
}
TEST_F(FileManagerTest, getFileDontOpenRealPath) {
- SmallString<64> CustomWorkingDir;
-#ifdef _WIN32
- CustomWorkingDir = "C:/";
-#else
- CustomWorkingDir = "/";
-#endif
+ SmallString<64> CustomWorkingDir = getSystemRoot();
auto FS = IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>(
new llvm::vfs::InMemoryFileSystem);
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -2012,20 +2012,18 @@
SourceLocation FilenameLoc = FilenameTok.getLocation();
StringRef LookupFilename = Filename;
-#ifdef _WIN32
- llvm::sys::path::Style BackslashStyle = llvm::sys::path::Style::windows;
-#else
+ llvm::sys::path::Style BackslashStyle = llvm::sys::path::system_style();
+
// Normalize slashes when compiling with -fms-extensions on non-Windows. This
// is unnecessary on Windows since the filesystem there handles backslashes.
SmallString<128> NormalizedPath;
- llvm::sys::path::Style BackslashStyle = llvm::sys::path::Style::posix;
- if (LangOpts.MicrosoftExt) {
+ if (BackslashStyle == llvm::sys::path::Style::posix &&
+ LangOpts.MicrosoftExt) {
NormalizedPath = Filename.str();
llvm::sys::path::native(NormalizedPath);
LookupFilename = NormalizedPath;
BackslashStyle = llvm::sys::path::Style::windows;
}
-#endif
Optional<FileEntryRef> File = LookupHeaderIncludeOrImport(
CurDir, Filename, FilenameLoc, FilenameRange, FilenameTok,
Index: clang/lib/Driver/ToolChain.cpp
===================================================================
--- clang/lib/Driver/ToolChain.cpp
+++ clang/lib/Driver/ToolChain.cpp
@@ -169,10 +169,11 @@
/// present and lower-casing the string on Windows.
static std::string normalizeProgramName(llvm::StringRef Argv0) {
std::string ProgName = std::string(llvm::sys::path::stem(Argv0));
-#ifdef _WIN32
- // Transform to lowercase for case insensitive file systems.
- std::transform(ProgName.begin(), ProgName.end(), ProgName.begin(), ::tolower);
-#endif
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows) {
+ // Transform to lowercase for case insensitive file systems.
+ std::transform(ProgName.begin(), ProgName.end(), ProgName.begin(),
+ ::tolower);
+ }
return ProgName;
}
Index: clang/lib/Driver/Driver.cpp
===================================================================
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4891,11 +4891,11 @@
bool MultipleArchs,
StringRef OffloadingPrefix) const {
std::string BoundArch = OrigBoundArch.str();
-#if defined(_WIN32)
- // BoundArch may contains ':', which is invalid in file names on Windows,
- // therefore replace it with '%'.
- std::replace(BoundArch.begin(), BoundArch.end(), ':', '@');
-#endif
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows) {
+ // BoundArch may contains ':', which is invalid in file names on Windows,
+ // therefore replace it with '%'.
+ std::replace(BoundArch.begin(), BoundArch.end(), ':', '@');
+ }
llvm::PrettyStackTraceString CrashInfo("Computing output path");
// Output to a user requested destination?
Index: clang/lib/Basic/FileManager.cpp
===================================================================
--- clang/lib/Basic/FileManager.cpp
+++ clang/lib/Basic/FileManager.cpp
@@ -123,16 +123,16 @@
DirName != llvm::sys::path::root_path(DirName) &&
llvm::sys::path::is_separator(DirName.back()))
DirName = DirName.substr(0, DirName.size()-1);
-#ifdef _WIN32
- // Fixing a problem with "clang C:test.c" on Windows.
- // Stat("C:") does not recognize "C:" as a valid directory
- std::string DirNameStr;
- if (DirName.size() > 1 && DirName.back() == ':' &&
- DirName.equals_insensitive(llvm::sys::path::root_name(DirName))) {
- DirNameStr = DirName.str() + '.';
- DirName = DirNameStr;
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows) {
+ // Fixing a problem with "clang C:test.c" on Windows.
+ // Stat("C:") does not recognize "C:" as a valid directory
+ std::string DirNameStr;
+ if (DirName.size() > 1 && DirName.back() == ':' &&
+ DirName.equals_insensitive(llvm::sys::path::root_name(DirName))) {
+ DirNameStr = DirName.str() + '.';
+ DirName = DirNameStr;
+ }
}
-#endif
++NumDirLookups;
Index: clang/include/clang/Basic/JsonSupport.h
===================================================================
--- clang/include/clang/Basic/JsonSupport.h
+++ clang/include/clang/Basic/JsonSupport.h
@@ -12,6 +12,7 @@
#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceManager.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Path.h"
#include "llvm/Support/raw_ostream.h"
#include <iterator>
@@ -98,18 +99,19 @@
if (AddBraces)
Out << "{ ";
std::string filename(PLoc.getFilename());
-#ifdef _WIN32
- // Remove forbidden Windows path characters
- auto RemoveIt =
- std::remove_if(filename.begin(), filename.end(), [](auto Char) {
- static const char ForbiddenChars[] = "<>*?\"|";
- return std::find(std::begin(ForbiddenChars), std::end(ForbiddenChars),
- Char) != std::end(ForbiddenChars);
- });
- filename.erase(RemoveIt, filename.end());
- // Handle windows-specific path delimiters.
- std::replace(filename.begin(), filename.end(), '\\', '/');
-#endif
+ if (llvm::sys::path::system_style() == llvm::sys::path::Style::windows) {
+ // Remove forbidden Windows path characters
+ auto RemoveIt =
+ std::remove_if(filename.begin(), filename.end(), [](auto Char) {
+ static const char ForbiddenChars[] = "<>*?\"|";
+ return std::find(std::begin(ForbiddenChars),
+ std::end(ForbiddenChars),
+ Char) != std::end(ForbiddenChars);
+ });
+ filename.erase(RemoveIt, filename.end());
+ // Handle windows-specific path delimiters.
+ std::replace(filename.begin(), filename.end(), '\\', '/');
+ }
Out << "\"line\": " << PLoc.getLine()
<< ", \"column\": " << PLoc.getColumn()
<< ", \"file\": \"" << filename << "\"";
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
