On Wed, 13 Jun 2018 at 17:27, Jonas Devlieghere via lldb-commits <lldb-commits@lists.llvm.org> wrote: > > Author: jdevlieghere > Date: Wed Jun 13 09:23:21 2018 > New Revision: 334615 > > URL: http://llvm.org/viewvc/llvm-project?rev=334615&view=rev > Log: > [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 > > Modified: > lldb/trunk/include/lldb/Utility/FileSpec.h > lldb/trunk/source/Core/Debugger.cpp > lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp > lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp > lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp > > lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp > lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp > lldb/trunk/source/Utility/FileSpec.cpp > lldb/trunk/unittests/Utility/FileSpecTest.cpp > > Modified: lldb/trunk/include/lldb/Utility/FileSpec.h > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/FileSpec.h?rev=334615&r1=334614&r2=334615&view=diff > ============================================================================== > --- lldb/trunk/include/lldb/Utility/FileSpec.h (original) > +++ lldb/trunk/include/lldb/Utility/FileSpec.h Wed Jun 13 09:23:21 2018 > @@ -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<lldb_ > static void format(const lldb_private::FileSpec &F, llvm::raw_ostream > &Stream, > StringRef Style); > }; > -} > +} // namespace llvm > > #endif // liblldb_FileSpec_h_ > > Modified: lldb/trunk/source/Core/Debugger.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/Debugger.cpp?rev=334615&r1=334614&r2=334615&view=diff > ============================================================================== > --- lldb/trunk/source/Core/Debugger.cpp (original) > +++ lldb/trunk/source/Core/Debugger.cpp Wed Jun 13 09:23:21 2018 > @@ -605,8 +605,8 @@ LoadPluginCallback(void *baton, llvm::sy > 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; > > Modified: lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp?rev=334615&r1=334614&r2=334615&view=diff > ============================================================================== > --- lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp (original) > +++ lldb/trunk/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp Wed Jun 13 > 09:23:21 2018 > @@ -2047,8 +2047,8 @@ unsigned ObjectFileELF::ParseSymbols(Sym > // 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); > > Modified: lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp?rev=334615&r1=334614&r2=334615&view=diff > ============================================================================== > --- lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp (original) > +++ lldb/trunk/source/Plugins/Platform/Android/PlatformAndroid.cpp Wed Jun 13 > 09:23:21 2018 > @@ -305,7 +305,7 @@ Status PlatformAndroid::DownloadSymbolFi > 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"); > > > Modified: lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp?rev=334615&r1=334614&r2=334615&view=diff > ============================================================================== > --- lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp > (original) > +++ lldb/trunk/source/Plugins/Platform/MacOSX/PlatformDarwinKernel.cpp Wed > Jun 13 09:23:21 2018 > @@ -431,8 +431,8 @@ void PlatformDarwinKernel::AddSDKSubdirs > 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(); > > > Modified: > lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp?rev=334615&r1=334614&r2=334615&view=diff > ============================================================================== > --- > lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp > (original) > +++ > lldb/trunk/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp > Wed Jun 13 09:23:21 2018 > @@ -2590,9 +2590,9 @@ bool ScriptInterpreterPython::LoadScript > // 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 { > > Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp?rev=334615&r1=334614&r2=334615&view=diff > ============================================================================== > --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original) > +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Wed Jun 13 > 09:23:21 2018 > @@ -1653,7 +1653,7 @@ void SymbolFileDWARF::UpdateExternalModu > // (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; > > Modified: lldb/trunk/source/Utility/FileSpec.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/FileSpec.cpp?rev=334615&r1=334614&r2=334615&view=diff > ============================================================================== > --- lldb/trunk/source/Utility/FileSpec.cpp (original) > +++ lldb/trunk/source/Utility/FileSpec.cpp Wed Jun 13 09:23:21 2018 > @@ -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 <algorithm> // for replace, min, unique > #include <system_error> // for error_code > @@ -50,7 +50,7 @@ bool PathStyleIsPosix(FileSpec::Style st > } > > 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<c > > 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::Stri > return false; > } > > - > + > } > //------------------------------------------------------------------ > // Assignment operator. > @@ -286,15 +262,19 @@ void FileSpec::SetFile(llvm::StringRef p > 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, > } > > 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::SmallVector > } > > 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)); > }
Why do you need the temporary current_path here? It should be possible to pass the m_filename.GetStringRef() straight to llvm::sys::path::extension.. _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
