[Lldb-commits] [lldb] r334615 - [FileSpec] Delegate common operations to llvm::sys::path

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Wed Jun 13 09:34:19 PDT 2018


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..


More information about the lldb-commits mailing list