[Lldb-commits] [lldb] r334615 - [FileSpec] Delegate common operations to llvm::sys::path
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Wed Jun 13 09:35:27 PDT 2018
Good catch, thanks!
On Wed, Jun 13, 2018 at 9:34 AM Pavel Labath <labath at google.com> wrote:
> On Wed, 13 Jun 2018 at 17:27, Jonas Devlieghere via lldb-commits
> <lldb-commits at 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..
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20180613/24be6768/attachment-0001.html>
More information about the lldb-commits
mailing list