From ad8d48f9036f92de1cb2ab1109236809d2243796 Mon Sep 17 00:00:00 2001 From: Jonas Devlieghere Date: Wed, 13 Jun 2018 16:23:21 +0000 Subject: [PATCH] [FileSpec] Delegate common operations to llvm::sys::path 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 helpers. This means we only have to maintain a single implementation and at the same time can benefit from the efforts made by 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. Differential revision: https://reviews.llvm.org/D48084 llvm-svn: 334615 --- lldb/include/lldb/Utility/FileSpec.h | 4 +- lldb/source/Core/Debugger.cpp | 4 +- .../Plugins/ObjectFile/ELF/ObjectFileELF.cpp | 4 +- .../Platform/Android/PlatformAndroid.cpp | 2 +- .../Platform/MacOSX/PlatformDarwinKernel.cpp | 8 +- .../Python/ScriptInterpreterPython.cpp | 4 +- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 2 +- lldb/source/Utility/FileSpec.cpp | 109 ++++++++---------- lldb/unittests/Utility/FileSpecTest.cpp | 11 +- 9 files changed, 69 insertions(+), 79 deletions(-) diff --git a/lldb/include/lldb/Utility/FileSpec.h b/lldb/include/lldb/Utility/FileSpec.h index a32dab96b40c..df21472bbf08 100644 --- a/lldb/include/lldb/Utility/FileSpec.h +++ b/lldb/include/lldb/Utility/FileSpec.h @@ -19,7 +19,7 @@ // Project includes #include "lldb/Utility/ConstString.h" -#include "llvm/ADT/StringRef.h" // for StringRef +#include "llvm/ADT/StringRef.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/FormatVariadic.h" #include "llvm/Support/Path.h" @@ -605,6 +605,6 @@ template <> struct format_provider { static void format(const lldb_private::FileSpec &F, llvm::raw_ostream &Stream, StringRef Style); }; -} +} // namespace llvm #endif // liblldb_FileSpec_h_ diff --git a/lldb/source/Core/Debugger.cpp b/lldb/source/Core/Debugger.cpp index c8f3b92dce1e..9d68d8470758 100644 --- a/lldb/source/Core/Debugger.cpp +++ b/lldb/source/Core/Debugger.cpp @@ -605,8 +605,8 @@ LoadPluginCallback(void *baton, llvm::sys::fs::file_type ft, 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; diff --git a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp index dc48c2581891..cd1c89ca8d81 100644 --- a/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ b/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -2047,8 +2047,8 @@ unsigned ObjectFileELF::ParseSymbols(Symtab *symtab, user_id_t start_id, // 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); diff --git a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp index 27cb708d383d..b7fd5064765c 100644 --- a/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp +++ b/lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp @@ -305,7 +305,7 @@ Status PlatformAndroid::DownloadSymbolFile(const lldb::ModuleSP &module_sp, 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"); diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp index a7eb82d37152..9aa7995e5537 100644 --- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp +++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp @@ -431,8 +431,8 @@ void PlatformDarwinKernel::AddSDKSubdirsToSearchPaths(const std::string &dir) { 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 @@ FileSpec::EnumerateDirectoryResult 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(); diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp index 5bb6e17c5f46..8c7517fd30c4 100644 --- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp +++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp @@ -2590,9 +2590,9 @@ bool ScriptInterpreterPython::LoadScriptingModule( // strip .py or .pyc extension ConstString extension = target_file.GetFileNameExtension(); if (extension) { - if (::strcmp(extension.GetCString(), "py") == 0) + if (llvm::StringRef(extension.GetCString()) == ".py") basename.resize(basename.length() - 3); - else if (::strcmp(extension.GetCString(), "pyc") == 0) + else if (llvm::StringRef(extension.GetCString()) == ".pyc") basename.resize(basename.length() - 4); } } else { diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index b7dd461c564f..203c22ee8b18 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -1653,7 +1653,7 @@ void SymbolFileDWARF::UpdateExternalModuleListIfNeeded() { // (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; diff --git a/lldb/source/Utility/FileSpec.cpp b/lldb/source/Utility/FileSpec.cpp index cc36f47e48d6..b3436e431c3e 100644 --- a/lldb/source/Utility/FileSpec.cpp +++ b/lldb/source/Utility/FileSpec.cpp @@ -12,15 +12,15 @@ #include "lldb/Utility/Stream.h" #include "lldb/Utility/TildeExpressionResolver.h" -#include "llvm/ADT/SmallString.h" // for SmallString -#include "llvm/ADT/SmallVector.h" // for SmallVectorTemplat... +#include "llvm/ADT/SmallString.h" +#include "llvm/ADT/SmallVector.h" #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" +#include "llvm/ADT/Twine.h" +#include "llvm/Support/ErrorOr.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Program.h" -#include "llvm/Support/raw_ostream.h" // for raw_ostream, fs +#include "llvm/Support/raw_ostream.h" #include // for replace, min, unique #include // for error_code @@ -50,7 +50,7 @@ bool PathStyleIsPosix(FileSpec::Style style) { } 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 @@ void Denormalize(llvm::SmallVectorImpl &path, FileSpec::Style style) { 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 @@ bool needsNormalization(const llvm::StringRef &path) { return false; } - + } //------------------------------------------------------------------ // Assignment operator. @@ -286,15 +262,19 @@ void FileSpec::SetFile(llvm::StringRef pathname, bool resolve, Style style) { 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 +421,15 @@ int FileSpec::Compare(const FileSpec &a, const FileSpec &b, bool full) { } 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; @@ -523,7 +502,7 @@ bool FileSpec::ResolvePath() { return true; // We have already resolved this path // SetFile(...) will set m_is_resolved correctly if it can resolve the path - SetFile(GetPath(false), true); + SetFile(GetPath(false), true, m_style); return m_is_resolved; } @@ -606,25 +585,17 @@ void FileSpec::GetPath(llvm::SmallVectorImpl &path, } 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; + current_path.append(m_filename.GetStringRef().begin(), + m_filename.GetStringRef().end()); + 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; + current_path.append(m_filename.GetStringRef().begin(), + m_filename.GetStringRef().end()); + return ConstString(llvm::sys::path::stem(current_path, m_style)); } //------------------------------------------------------------------ @@ -753,7 +724,7 @@ void FileSpec::PrependPathComponent(llvm::StringRef component) { const bool resolve = false; if (m_filename.IsEmpty() && m_directory.IsEmpty()) { - SetFile(component, resolve); + SetFile(component, resolve, m_style); return; } @@ -810,7 +781,7 @@ bool FileSpec::IsSourceImplementationFile() const { 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])" "$")); @@ -818,13 +789,23 @@ bool FileSpec::IsSourceImplementationFile() const { } 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::format(const FileSpec &F, raw_ostream &Stream, diff --git a/lldb/unittests/Utility/FileSpecTest.cpp b/lldb/unittests/Utility/FileSpecTest.cpp index 2e02579797ad..a271229b2e03 100644 --- a/lldb/unittests/Utility/FileSpecTest.cpp +++ b/lldb/unittests/Utility/FileSpecTest.cpp @@ -30,6 +30,16 @@ TEST(FileSpecTest, FileAndDirectoryComponents) { 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 @@ TEST(FileSpecTest, IsRelative) { "/a/b", "/a/b/", "//", - "//a", "//a/", "//a/b", "//a/b/",