Good catch, thanks! On Wed, Jun 13, 2018 at 9:34 AM Pavel Labath <lab...@google.com> wrote:
> 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
