JDevlieghere created this revision.
JDevlieghere added reviewers: labath, zturner, clayborg, davide.
Herald added subscribers: arichardson, emaste.
Herald added a reviewer: espindola.
With the recent changes in FileSpec to use LLVM's path style, it is
possible to delegate a bunch of common path operations to LLVM's path
implementation. This means we only have to maintain a single
implementation and can benefit from the efforts of the rest of the LLVM
community.
This is part one of a set of patches. There was no obvious way to split
this so I just worked from top to bottom.
Repository:
rL LLVM
https://reviews.llvm.org/D48084
Files:
include/lldb/Utility/FileSpec.h
source/Core/Debugger.cpp
source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
source/Plugins/Platform/Android/PlatformAndroid.cpp
source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
source/Utility/FileSpec.cpp
unittests/Utility/FileSpecTest.cpp
Index: unittests/Utility/FileSpecTest.cpp
===================================================================
--- unittests/Utility/FileSpecTest.cpp
+++ unittests/Utility/FileSpecTest.cpp
@@ -30,6 +30,16 @@
EXPECT_EQ(nullptr, fs_posix_root.GetDirectory().GetCString());
EXPECT_STREQ("/", fs_posix_root.GetFilename().GetCString());
+ FileSpec fs_net_drive("//net", false, FileSpec::Style::posix);
+ EXPECT_STREQ("//net", fs_net_drive.GetCString());
+ EXPECT_EQ(nullptr, fs_net_drive.GetDirectory().GetCString());
+ EXPECT_STREQ("//net", fs_net_drive.GetFilename().GetCString());
+
+ FileSpec fs_net_root("//net/", false, FileSpec::Style::posix);
+ EXPECT_STREQ("//net/", fs_net_root.GetCString());
+ EXPECT_STREQ("//net", fs_net_root.GetDirectory().GetCString());
+ EXPECT_STREQ("/", fs_net_root.GetFilename().GetCString());
+
FileSpec fs_windows_drive("F:", false, FileSpec::Style::windows);
EXPECT_STREQ("F:", fs_windows_drive.GetCString());
EXPECT_EQ(nullptr, fs_windows_drive.GetDirectory().GetCString());
@@ -281,7 +291,6 @@
"/a/b",
"/a/b/",
"//",
- "//a",
"//a/",
"//a/b",
"//a/b/",
Index: source/Utility/FileSpec.cpp
===================================================================
--- source/Utility/FileSpec.cpp
+++ source/Utility/FileSpec.cpp
@@ -15,9 +15,9 @@
#include "llvm/ADT/SmallString.h" // for SmallString
#include "llvm/ADT/SmallVector.h" // for SmallVectorTemplat...
#include "llvm/ADT/StringRef.h"
-#include "llvm/ADT/Triple.h" // for Triple
-#include "llvm/ADT/Twine.h" // for Twine
-#include "llvm/Support/ErrorOr.h" // for ErrorOr
+#include "llvm/ADT/Triple.h" // for Triple
+#include "llvm/ADT/Twine.h" // for Twine
+#include "llvm/Support/ErrorOr.h" // for ErrorOr
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Program.h"
#include "llvm/Support/raw_ostream.h" // for raw_ostream, fs
@@ -50,7 +50,7 @@
}
const char *GetPathSeparators(FileSpec::Style style) {
- return PathStyleIsPosix(style) ? "/" : "\\/";
+ return llvm::sys::path::get_separator(style).data();
}
char GetPreferredPathSeparator(FileSpec::Style style) {
@@ -67,30 +67,6 @@
std::replace(path.begin(), path.end(), '/', '\\');
}
-
-bool PathIsRelative(llvm::StringRef path, FileSpec::Style style) {
-
- if (path.empty())
- return false;
-
- if (PathStyleIsPosix(style)) {
- // If the path doesn't start with '/' or '~', return true
- switch (path[0]) {
- case '/':
- case '~':
- return false;
- default:
- return true;
- }
- } else {
- if (path.size() >= 2 && path[1] == ':')
- return false;
- if (path[0] == '/')
- return false;
- return true;
- }
- return false;
-}
} // end anonymous namespace
@@ -239,7 +215,7 @@
return false;
}
-
+
}
//------------------------------------------------------------------
// Assignment operator.
@@ -259,11 +235,15 @@
// up into a directory and filename and stored as uniqued string values for
// quick comparison and efficient memory usage.
//------------------------------------------------------------------
-void FileSpec::SetFile(llvm::StringRef pathname, bool resolve, Style style) {
+void FileSpec::SetFile(llvm::StringRef pathname, bool resolve,
+ llvm::Optional<Style> style) {
m_filename.Clear();
m_directory.Clear();
m_is_resolved = false;
- m_style = (style == Style::native) ? GetNativeStyle() : style;
+
+ // Only update style if explicitly requested.
+ if (style)
+ m_style = (*style == Style::native) ? GetNativeStyle() : *style;
if (pathname.empty())
return;
@@ -286,15 +266,19 @@
if (resolved.empty()) {
// If we have no path after normalization set the path to the current
// directory. This matches what python does and also a few other path
- // utilities.
+ // utilities.
m_filename.SetString(".");
return;
}
- m_filename.SetString(llvm::sys::path::filename(resolved, m_style));
- llvm::StringRef dir = llvm::sys::path::parent_path(resolved, m_style);
- if (!dir.empty())
- m_directory.SetString(dir);
+ // Split path into filename and directory. We rely on the underlying char
+ // pointer to be nullptr when the components are empty.
+ llvm::StringRef filename = llvm::sys::path::filename(resolved, m_style);
+ if(!filename.empty())
+ m_filename.SetString(filename);
+ llvm::StringRef directory = llvm::sys::path::parent_path(resolved, m_style);
+ if(!directory.empty())
+ m_directory.SetString(directory);
}
void FileSpec::SetFile(llvm::StringRef path, bool resolve,
@@ -441,16 +425,15 @@
}
bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full) {
-
// case sensitivity of equality test
const bool case_sensitive = a.IsCaseSensitive() || b.IsCaseSensitive();
-
+
const bool filenames_equal = ConstString::Equals(a.m_filename,
b.m_filename,
case_sensitive);
if (!filenames_equal)
- return false;
+ return false;
if (!full && (a.GetDirectory().IsEmpty() || b.GetDirectory().IsEmpty()))
return filenames_equal;
@@ -606,25 +589,15 @@
}
ConstString FileSpec::GetFileNameExtension() const {
- if (m_filename) {
- const char *filename = m_filename.GetCString();
- const char *dot_pos = strrchr(filename, '.');
- if (dot_pos && dot_pos[1] != '\0')
- return ConstString(dot_pos + 1);
- }
- return ConstString();
+ llvm::SmallString<64> current_path;
+ GetPath(current_path, false);
+ return ConstString(llvm::sys::path::extension(current_path, m_style));
}
ConstString FileSpec::GetFileNameStrippingExtension() const {
- const char *filename = m_filename.GetCString();
- if (filename == NULL)
- return ConstString();
-
- const char *dot_pos = strrchr(filename, '.');
- if (dot_pos == NULL)
- return m_filename;
-
- return ConstString(filename, dot_pos - filename);
+ llvm::SmallString<64> current_path;
+ GetPath(current_path, false);
+ return ConstString(llvm::sys::path::stem(current_path, m_style));
}
//------------------------------------------------------------------
@@ -760,7 +733,7 @@
std::string result =
join_path_components(m_style, {component, m_directory.GetStringRef(),
m_filename.GetStringRef()});
- SetFile(result, resolve, m_style);
+ SetFile(result, resolve);
}
void FileSpec::PrependPathComponent(const FileSpec &new_path) {
@@ -778,7 +751,7 @@
join_path_components(m_style, {m_directory.GetStringRef(),
m_filename.GetStringRef(), component});
- SetFile(result, false, m_style);
+ SetFile(result, false);
}
void FileSpec::AppendPathComponent(const FileSpec &new_path) {
@@ -789,8 +762,7 @@
llvm::SmallString<64> current_path;
GetPath(current_path, false);
if (llvm::sys::path::has_parent_path(current_path, m_style)) {
- SetFile(llvm::sys::path::parent_path(current_path, m_style), false,
- m_style);
+ SetFile(llvm::sys::path::parent_path(current_path, m_style), false);
return true;
}
return false;
@@ -810,21 +782,31 @@
return false;
static RegularExpression g_source_file_regex(llvm::StringRef(
- "^([cC]|[mM]|[mM][mM]|[cC][pP][pP]|[cC]\\+\\+|[cC][xX][xX]|[cC][cC]|["
+ "^.([cC]|[mM]|[mM][mM]|[cC][pP][pP]|[cC]\\+\\+|[cC][xX][xX]|[cC][cC]|["
"cC][pP]|[sS]|[aA][sS][mM]|[fF]|[fF]77|[fF]90|[fF]95|[fF]03|[fF][oO]["
"rR]|[fF][tT][nN]|[fF][pP][pP]|[aA][dD][aA]|[aA][dD][bB]|[aA][dD][sS])"
"$"));
return g_source_file_regex.Execute(extension.GetStringRef());
}
bool FileSpec::IsRelative() const {
- if (m_directory)
- return PathIsRelative(m_directory.GetStringRef(), m_style);
- else
- return PathIsRelative(m_filename.GetStringRef(), m_style);
+ return !IsAbsolute();
}
-bool FileSpec::IsAbsolute() const { return !FileSpec::IsRelative(); }
+bool FileSpec::IsAbsolute() const {
+ llvm::SmallString<64> current_path;
+ GetPath(current_path, false);
+
+ // Early return if the path is empty.
+ if (current_path.empty())
+ return false;
+
+ // We consider paths starting with ~ to be absolute.
+ if (current_path[0] == '~')
+ return true;
+
+ return llvm::sys::path::is_absolute(current_path, m_style);
+}
void llvm::format_provider<FileSpec>::format(const FileSpec &F,
raw_ostream &Stream,
Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
===================================================================
--- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1653,7 +1653,7 @@
// (corresponding to .dwo) so we simply skip it.
if (m_obj_file->GetFileSpec()
.GetFileNameExtension()
- .GetStringRef() == "dwo" &&
+ .GetStringRef() == ".dwo" &&
llvm::StringRef(m_obj_file->GetFileSpec().GetPath())
.endswith(dwo_module_spec.GetFileSpec().GetPath())) {
continue;
Index: source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
===================================================================
--- source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2590,9 +2590,9 @@
// strip .py or .pyc extension
ConstString extension = target_file.GetFileNameExtension();
if (extension) {
- if (::strcmp(extension.GetCString(), "py") == 0)
+ if (::strcmp(extension.GetCString(), ".py") == 0)
basename.resize(basename.length() - 3);
- else if (::strcmp(extension.GetCString(), "pyc") == 0)
+ else if (::strcmp(extension.GetCString(), ".pyc") == 0)
basename.resize(basename.length() - 4);
}
} else {
Index: source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
===================================================================
--- source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
+++ source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp
@@ -431,8 +431,8 @@
FileSpec::EnumerateDirectoryResult
PlatformDarwinKernel::FindKDKandSDKDirectoriesInDirectory(
void *baton, llvm::sys::fs::file_type ft, const FileSpec &file_spec) {
- static ConstString g_sdk_suffix = ConstString("sdk");
- static ConstString g_kdk_suffix = ConstString("kdk");
+ static ConstString g_sdk_suffix = ConstString(".sdk");
+ static ConstString g_kdk_suffix = ConstString(".kdk");
PlatformDarwinKernel *thisp = (PlatformDarwinKernel *)baton;
if (ft == llvm::sys::fs::file_type::directory_file &&
@@ -492,8 +492,8 @@
PlatformDarwinKernel::GetKernelsAndKextsInDirectoryHelper(
void *baton, llvm::sys::fs::file_type ft, const FileSpec &file_spec,
bool recurse) {
- static ConstString g_kext_suffix = ConstString("kext");
- static ConstString g_dsym_suffix = ConstString("dSYM");
+ static ConstString g_kext_suffix = ConstString(".kext");
+ static ConstString g_dsym_suffix = ConstString(".dSYM");
static ConstString g_bundle_suffix = ConstString("Bundle");
ConstString file_spec_extension = file_spec.GetFileNameExtension();
Index: source/Plugins/Platform/Android/PlatformAndroid.cpp
===================================================================
--- source/Plugins/Platform/Android/PlatformAndroid.cpp
+++ source/Plugins/Platform/Android/PlatformAndroid.cpp
@@ -305,7 +305,7 @@
const FileSpec &dst_file_spec) {
// For oat file we can try to fetch additional debug info from the device
ConstString extension = module_sp->GetFileSpec().GetFileNameExtension();
- if (extension != ConstString("oat") && extension != ConstString("odex"))
+ if (extension != ConstString(".oat") && extension != ConstString(".odex"))
return Status(
"Symbol file downloading only supported for oat and odex files");
Index: source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
===================================================================
--- source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
+++ source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp
@@ -2047,8 +2047,8 @@
// custom extension and file name makes it highly unlikely that this will
// collide with anything else.
ConstString file_extension = m_file.GetFileNameExtension();
- bool skip_oatdata_oatexec = file_extension == ConstString("oat") ||
- file_extension == ConstString("odex");
+ bool skip_oatdata_oatexec = file_extension == ConstString(".oat") ||
+ file_extension == ConstString(".odex");
ArchSpec arch;
GetArchitecture(arch);
Index: source/Core/Debugger.cpp
===================================================================
--- source/Core/Debugger.cpp
+++ source/Core/Debugger.cpp
@@ -605,8 +605,8 @@
const FileSpec &file_spec) {
Status error;
- static ConstString g_dylibext("dylib");
- static ConstString g_solibext("so");
+ static ConstString g_dylibext(".dylib");
+ static ConstString g_solibext(".so");
if (!baton)
return FileSpec::eEnumerateDirectoryResultQuit;
Index: include/lldb/Utility/FileSpec.h
===================================================================
--- include/lldb/Utility/FileSpec.h
+++ include/lldb/Utility/FileSpec.h
@@ -491,7 +491,7 @@
/// the static FileSpec::Resolve.
//------------------------------------------------------------------
void SetFile(llvm::StringRef path, bool resolve_path,
- Style style = Style::native);
+ llvm::Optional<Style> style = llvm::None);
void SetFile(llvm::StringRef path, bool resolve_path,
const llvm::Triple &Triple);
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
