[Lldb-commits] [lldb] r331049 - Always normalize FileSpec paths.

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Fri Apr 27 13:53:53 PDT 2018


I will fix this ASAP

> On Apr 27, 2018, at 11:17 AM, Jim Ingham <jingham at apple.com> wrote:
> 
> Greg,
> 
> Your new FileSpecTest unit tests are failing in the Xcode build of lldb, e.g.:
> 
> http://lab.llvm.org:8080/green/view/LLDB/job/lldb-xcode/6271/consoleFull#-1083450927b825e790-484f-4586-af29-73c4754ff671
> 
> Can you figure out what's up with this?
> 
> Jim
> 
> BTW, the "reply to" for lldb-commits mails for you is still your apple.com address.  I don't know where that gets set but you should probably update that at some point.
> 
> 
> 
> 
>> On Apr 27, 2018, at 8:45 AM, Greg Clayton via lldb-commits <lldb-commits at lists.llvm.org> wrote:
>> 
>> Author: gclayton
>> Date: Fri Apr 27 08:45:58 2018
>> New Revision: 331049
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=331049&view=rev
>> Log:
>> Always normalize FileSpec paths.
>> 
>> Always normalizing lldb_private::FileSpec paths will help us get a consistent results from comparisons when setting breakpoints and when looking for source files. This also removes a lot of complexity from the comparison routines. Modified the DWARF line table parser to use the normalized compile unit directory if needed.
>> 
>> Differential Revision: https://reviews.llvm.org/D45977
>> 
>> 
>> Modified:
>> lldb/trunk/include/lldb/Core/FileSpecList.h
>> lldb/trunk/include/lldb/Utility/FileSpec.h
>> lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
>> lldb/trunk/source/Core/FileSpecList.cpp
>> lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
>> lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
>> lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h
>> lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
>> lldb/trunk/source/Symbol/CompileUnit.cpp
>> lldb/trunk/source/Symbol/Declaration.cpp
>> lldb/trunk/source/Utility/FileSpec.cpp
>> lldb/trunk/unittests/Utility/FileSpecTest.cpp
>> 
>> Modified: lldb/trunk/include/lldb/Core/FileSpecList.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Core/FileSpecList.h?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Core/FileSpecList.h (original)
>> +++ lldb/trunk/include/lldb/Core/FileSpecList.h Fri Apr 27 08:45:58 2018
>> @@ -119,16 +119,11 @@ public:
>> /// @param[in] full
>> ///     Should FileSpec::Equal be called with "full" true or false.
>> ///
>> -  /// @param[in] remove_backup_dots
>> -  ///     Should FileSpec::Equal be called with "remove_backup_dots" true or
>> -  ///     false.
>> -  ///
>> /// @return
>> ///     The index of the file that matches \a file if it is found,
>> ///     else UINT32_MAX is returned.
>> //------------------------------------------------------------------
>> -  size_t FindFileIndex(size_t idx, const FileSpec &file, bool full,
>> -                       bool remove_backup_dots = false) const;
>> +  size_t FindFileIndex(size_t idx, const FileSpec &file, bool full) const;
>> 
>> //------------------------------------------------------------------
>> /// Get file at index.
>> 
>> Modified: lldb/trunk/include/lldb/Utility/FileSpec.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Utility/FileSpec.h?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/include/lldb/Utility/FileSpec.h (original)
>> +++ lldb/trunk/include/lldb/Utility/FileSpec.h Fri Apr 27 08:45:58 2018
>> @@ -256,8 +256,7 @@ public:
>> //------------------------------------------------------------------
>> static int Compare(const FileSpec &lhs, const FileSpec &rhs, bool full);
>> 
>> -  static bool Equal(const FileSpec &a, const FileSpec &b, bool full,
>> -                    bool remove_backups = false);
>> +  static bool Equal(const FileSpec &a, const FileSpec &b, bool full);
>> 
>> //------------------------------------------------------------------
>> /// Case sensitivity of path.
>> @@ -488,12 +487,6 @@ public:
>> size_t MemorySize() const;
>> 
>> //------------------------------------------------------------------
>> -  /// Normalize a pathname by collapsing redundant separators and
>> -  /// up-level references.
>> -  //------------------------------------------------------------------
>> -  FileSpec GetNormalizedPath() const;
>> -
>> -  //------------------------------------------------------------------
>> /// Change the file specified with a new path.
>> ///
>> /// Update the contents of this object with a new path. The path will
>> 
>> Modified: lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp (original)
>> +++ lldb/trunk/source/Breakpoint/BreakpointResolverFileLine.cpp Fri Apr 27 08:45:58 2018
>> @@ -122,7 +122,7 @@ void BreakpointResolverFileLine::FilterC
>> 
>> llvm::StringRef relative_path;
>> if (is_relative)
>> -    relative_path = m_file_spec.GetNormalizedPath().GetDirectory().GetStringRef();
>> +    relative_path = m_file_spec.GetDirectory().GetStringRef();
>> 
>> Log * log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_BREAKPOINTS);
>> for(uint32_t i = 0; i < sc_list.GetSize(); ++i) {
>> 
>> Modified: lldb/trunk/source/Core/FileSpecList.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Core/FileSpecList.cpp?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Core/FileSpecList.cpp (original)
>> +++ lldb/trunk/source/Core/FileSpecList.cpp Fri Apr 27 08:45:58 2018
>> @@ -82,7 +82,7 @@ void FileSpecList::Dump(Stream *s, const
>> // it is found, else std::numeric_limits<uint32_t>::max() is returned.
>> //------------------------------------------------------------------
>> size_t FileSpecList::FindFileIndex(size_t start_idx, const FileSpec &file_spec,
>> -                                   bool full, bool remove_dots) const {
>> +                                   bool full) const {
>> const size_t num_files = m_files.size();
>> 
>> // When looking for files, we will compare only the filename if the
>> @@ -96,7 +96,7 @@ size_t FileSpecList::FindFileIndex(size_
>>            file_spec.IsCaseSensitive() || m_files[idx].IsCaseSensitive()))
>>      return idx;
>>  } else {
>> -      if (FileSpec::Equal(m_files[idx], file_spec, full, remove_dots))
>> +      if (FileSpec::Equal(m_files[idx], file_spec, full))
>>      return idx;
>>  }
>> }
>> 
>> Modified: lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp (original)
>> +++ lldb/trunk/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp Fri Apr 27 08:45:58 2018
>> @@ -5098,9 +5098,6 @@ uint32_t ObjectFileMachO::GetDependentMo
>>        // It is OK to resolve this path because we must find a file on
>>        // disk for us to accept it anyway if it is rpath relative.
>>        FileSpec file_spec(path, true);
>> -          // Remove any redundant parts of the path (like "../foo") since
>> -          // LC_RPATH values often contain "..".
>> -          file_spec = file_spec.GetNormalizedPath();
>>        if (file_spec.Exists() && files.AppendIfUnique(file_spec)) {
>>          count++;
>>          break;
>> @@ -5118,7 +5115,6 @@ uint32_t ObjectFileMachO::GetDependentMo
>>    for (const auto &at_exec_relative_path : at_exec_relative_paths) {
>>      FileSpec file_spec = 
>>          exec_dir.CopyByAppendingPathComponent(at_exec_relative_path);
>> -        file_spec = file_spec.GetNormalizedPath();
>>      if (file_spec.Exists() && files.AppendIfUnique(file_spec))
>>        count++;
>>    }
>> 
>> Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp (original)
>> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.cpp Fri Apr 27 08:45:58 2018
>> @@ -444,7 +444,7 @@ bool DWARFDebugLine::ParsePrologue(const
>> 
>> bool DWARFDebugLine::ParseSupportFiles(
>>  const lldb::ModuleSP &module_sp, const DWARFDataExtractor &debug_line_data,
>> -    const char *cu_comp_dir, dw_offset_t stmt_list,
>> +    const lldb_private::FileSpec &cu_comp_dir, dw_offset_t stmt_list,
>>  FileSpecList &support_files) {
>> lldb::offset_t offset = stmt_list;
>> 
>> @@ -861,8 +861,8 @@ void DWARFDebugLine::Prologue::Dump(Log
>> //  buff.Append8(0);    // Terminate the file names section with empty string
>> //}
>> 
>> -bool DWARFDebugLine::Prologue::GetFile(uint32_t file_idx, const char *comp_dir,
>> -                                       FileSpec &file) const {
>> +bool DWARFDebugLine::Prologue::GetFile(uint32_t file_idx,
>> +    const lldb_private::FileSpec &comp_dir, FileSpec &file) const {
>> uint32_t idx = file_idx - 1; // File indexes are 1 based...
>> if (idx < file_names.size()) {
>>  file.SetFile(file_names[idx].name, false);
>> @@ -876,7 +876,7 @@ bool DWARFDebugLine::Prologue::GetFile(u
>>      }
>>    }
>> 
>> -      if (comp_dir && comp_dir[0])
>> +      if (comp_dir)
>>      file.PrependPathComponent(comp_dir);
>>  }
>>  return true;
>> 
>> Modified: lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h (original)
>> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/DWARFDebugLine.h Fri Apr 27 08:45:58 2018
>> @@ -90,7 +90,7 @@ public:
>>    include_directories.clear();
>>    file_names.clear();
>>  }
>> -    bool GetFile(uint32_t file_idx, const char *comp_dir,
>> +    bool GetFile(uint32_t file_idx, const lldb_private::FileSpec &cu_comp_dir,
>>               lldb_private::FileSpec &file) const;
>> };
>> 
>> @@ -199,7 +199,8 @@ public:
>> static bool
>> ParseSupportFiles(const lldb::ModuleSP &module_sp,
>>                  const lldb_private::DWARFDataExtractor &debug_line_data,
>> -                    const char *cu_comp_dir, dw_offset_t stmt_list,
>> +                    const lldb_private::FileSpec &cu_comp_dir,
>> +                    dw_offset_t stmt_list,
>>                  lldb_private::FileSpecList &support_files);
>> static bool
>> ParsePrologue(const lldb_private::DWARFDataExtractor &debug_line_data,
>> 
>> 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=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (original)
>> +++ lldb/trunk/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp Fri Apr 27 08:45:58 2018
>> @@ -174,38 +174,39 @@ static const char *removeHostnameFromPat
>> return colon_pos + 1;
>> }
>> 
>> -static const char *resolveCompDir(const char *path_from_dwarf) {
>> +static FileSpec resolveCompDir(const char *path_from_dwarf) {
>> if (!path_from_dwarf)
>> -    return nullptr;
>> +    return FileSpec();
>> 
>> // DWARF2/3 suggests the form hostname:pathname for compilation directory.
>> // Remove the host part if present.
>> const char *local_path = removeHostnameFromPathname(path_from_dwarf);
>> if (!local_path)
>> -    return nullptr;
>> +    return FileSpec();
>> 
>> bool is_symlink = false;
>> -  FileSpec local_path_spec(local_path, false);
>> +  // Always normalize our compile unit directory to get rid of redundant
>> +  // slashes and other path anomalies before we use it for path prepending
>> +  FileSpec local_spec(local_path, false);
>> const auto &file_specs = GetGlobalPluginProperties()->GetSymLinkPaths();
>> for (size_t i = 0; i < file_specs.GetSize() && !is_symlink; ++i)
>>  is_symlink = FileSpec::Equal(file_specs.GetFileSpecAtIndex(i),
>> -                                 local_path_spec, true);
>> +                                 local_spec, true);
>> 
>> if (!is_symlink)
>> -    return local_path;
>> +    return local_spec;
>> 
>> namespace fs = llvm::sys::fs;
>> -  if (fs::get_file_type(local_path_spec.GetPath(), false) !=
>> +  if (fs::get_file_type(local_spec.GetPath(), false) !=
>>    fs::file_type::symlink_file)
>> -    return local_path;
>> +    return local_spec;
>> 
>> -  FileSpec resolved_local_path_spec;
>> -  const auto error =
>> -      FileSystem::Readlink(local_path_spec, resolved_local_path_spec);
>> +  FileSpec resolved_symlink;
>> +  const auto error = FileSystem::Readlink(local_spec, resolved_symlink);
>> if (error.Success())
>> -    return resolved_local_path_spec.GetCString();
>> +    return resolved_symlink;
>> 
>> -  return nullptr;
>> +  return local_spec;
>> }
>> 
>> DWARFUnit *SymbolFileDWARF::GetBaseCompileUnit() {
>> @@ -912,7 +913,7 @@ bool SymbolFileDWARF::ParseCompileUnitSu
>>  const DWARFDIE cu_die = dwarf_cu->GetUnitDIEOnly();
>> 
>>  if (cu_die) {
>> -      const char *cu_comp_dir = resolveCompDir(
>> +      FileSpec cu_comp_dir = resolveCompDir(
>>        cu_die.GetAttributeValueAsString(DW_AT_comp_dir, nullptr));
>>    const dw_offset_t stmt_list = cu_die.GetAttributeValueAsUnsigned(
>>        DW_AT_stmt_list, DW_INVALID_OFFSET);
>> 
>> Modified: lldb/trunk/source/Symbol/CompileUnit.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/CompileUnit.cpp?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Symbol/CompileUnit.cpp (original)
>> +++ lldb/trunk/source/Symbol/CompileUnit.cpp Fri Apr 27 08:45:58 2018
>> @@ -287,9 +287,8 @@ uint32_t CompileUnit::ResolveSymbolConte
>> // when finding file indexes
>> std::vector<uint32_t> file_indexes;
>> const bool full_match = (bool)file_spec.GetDirectory();
>> -  const bool remove_backup_dots = true;
>> bool file_spec_matches_cu_file_spec =
>> -      FileSpec::Equal(file_spec, *this, full_match, remove_backup_dots);
>> +      FileSpec::Equal(file_spec, *this, full_match);
>> 
>> // If we are not looking for inlined functions and our file spec doesn't
>> // match then we are done...
>> @@ -297,11 +296,10 @@ uint32_t CompileUnit::ResolveSymbolConte
>>  return 0;
>> 
>> uint32_t file_idx =
>> -      GetSupportFiles().FindFileIndex(1, file_spec, true, remove_backup_dots);
>> +      GetSupportFiles().FindFileIndex(1, file_spec, true);
>> while (file_idx != UINT32_MAX) {
>>  file_indexes.push_back(file_idx);
>> -    file_idx = GetSupportFiles().FindFileIndex(file_idx + 1, file_spec, true,
>> -                                               remove_backup_dots);
>> +    file_idx = GetSupportFiles().FindFileIndex(file_idx + 1, file_spec, true);
>> }
>> 
>> const size_t num_file_indexes = file_indexes.size();
>> 
>> Modified: lldb/trunk/source/Symbol/Declaration.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/Declaration.cpp?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Symbol/Declaration.cpp (original)
>> +++ lldb/trunk/source/Symbol/Declaration.cpp Fri Apr 27 08:45:58 2018
>> @@ -91,7 +91,7 @@ bool lldb_private::operator==(const Decl
>>    return lhs.GetFile() == rhs.GetFile();
>> #else
>> if (lhs.GetLine() == rhs.GetLine())
>> -    return FileSpec::Equal(lhs.GetFile(), rhs.GetFile(), true, true);
>> +    return FileSpec::Equal(lhs.GetFile(), rhs.GetFile(), true);
>> #endif
>> return false;
>> }
>> 
>> Modified: lldb/trunk/source/Utility/FileSpec.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Utility/FileSpec.cpp?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/source/Utility/FileSpec.cpp (original)
>> +++ lldb/trunk/source/Utility/FileSpec.cpp Fri Apr 27 08:45:58 2018
>> @@ -62,17 +62,18 @@ bool IsPathSeparator(char value, FileSpe
>> return value == '/' || (!PathSyntaxIsPosix(syntax) && value == '\\');
>> }
>> 
>> -void Normalize(llvm::SmallVectorImpl<char> &path, FileSpec::PathSyntax syntax) {
>> -  if (PathSyntaxIsPosix(syntax))
>> -    return;
>> -
>> -  std::replace(path.begin(), path.end(), '\\', '/');
>> -  // Windows path can have \\ slashes which can be changed by replace
>> -  // call above to //. Here we remove the duplicate.
>> -  auto iter = std::unique(path.begin(), path.end(), [](char &c1, char &c2) {
>> -    return (c1 == '/' && c2 == '/');
>> -  });
>> -  path.erase(iter, path.end());
>> +inline llvm::sys::path::Style
>> +LLVMPathSyntax(FileSpec::PathSyntax lldb_syntax) {
>> +  switch (lldb_syntax) {
>> +    case FileSpec::ePathSyntaxPosix:
>> +      return llvm::sys::path::Style::posix;
>> +    case FileSpec::ePathSyntaxWindows:
>> +      return llvm::sys::path::Style::windows;
>> +    default:
>> +    case FileSpec::ePathSyntaxHostNative:
>> +      return llvm::sys::path::Style::native;
>> +  };
>> +  return llvm::sys::path::Style::native;
>> }
>> 
>> void Denormalize(llvm::SmallVectorImpl<char> &path,
>> @@ -199,6 +200,104 @@ FileSpec::FileSpec(const FileSpec *rhs)
>> //------------------------------------------------------------------
>> FileSpec::~FileSpec() {}
>> 
>> +namespace {
>> +//------------------------------------------------------------------
>> +/// Safely get a character at the specified index.
>> +///
>> +/// @param[in] path
>> +///     A full, partial, or relative path to a file.
>> +///
>> +/// @param[in] i
>> +///     An index into path which may or may not be valid.
>> +///
>> +/// @return
>> +///   The character at index \a i if the index is valid, or 0 if
>> +///   the index is not valid.
>> +//------------------------------------------------------------------
>> +inline char safeCharAtIndex(const llvm::StringRef &path, size_t i) {
>> +  if (i < path.size())
>> +    return path[i];
>> +  return 0;
>> +}
>> +
>> +//------------------------------------------------------------------
>> +/// Check if a path needs to be normalized.
>> +///
>> +/// Check if a path needs to be normalized. We currently consider a
>> +/// path to need normalization if any of the following are true
>> +///  - path contains "/./"
>> +///  - path contains "/../"
>> +///  - path contains "//"
>> +///  - path ends with "/"
>> +/// Paths that start with "./" or with "../" are not considered to
>> +/// need normalization since we aren't trying to resolve the path,
>> +/// we are just trying to remove redundant things from the path.
>> +///
>> +/// @param[in] path
>> +///     A full, partial, or relative path to a file.
>> +///
>> +/// @param[in] syntax
>> +///     The syntax enumeration for the path in \a path.
>> +///
>> +/// @return
>> +///   Returns \b true if the path needs to be normalized.
>> +//------------------------------------------------------------------
>> +bool needsNormalization(const llvm::StringRef &path,
>> +                        FileSpec::PathSyntax syntax) {
>> +  const auto separator = GetPreferredPathSeparator(syntax);
>> +  for (auto i = path.find(separator); i != llvm::StringRef::npos;
>> +       i = path.find(separator, i + 1)) {
>> +    const auto next = safeCharAtIndex(path, i+1);
>> +    switch (next) {
>> +      case 0:
>> +        // path separator char at the end of the string which should be
>> +        // stripped unless it is the one and only character
>> +        return i > 0;
>> +      case '/':
>> +      case '\\':
>> +        // two path separator chars in the middle of a path needs to be
>> +        // normalized
>> +        if (next == separator && i > 0)
>> +          return true;
>> +        ++i;
>> +        break;
>> +
>> +      case '.': {
>> +          const auto next_next = safeCharAtIndex(path, i+2);
>> +          switch (next_next) {
>> +            default: break;
>> +            case 0: return true; // ends with "/."
>> +            case '/':
>> +            case '\\':
>> +              if (next_next == separator)
>> +                return true; // contains "/./"
>> +              break;
>> +            case '.': {
>> +              const auto next_next_next = safeCharAtIndex(path, i+3);
>> +              switch (next_next_next) {
>> +                default: break;
>> +                case 0: return true; // ends with "/.."
>> +                case '/':
>> +                case '\\':
>> +                  if (next_next_next == separator)
>> +                    return true; // contains "/../"
>> +                  break;
>> +              }
>> +              break;
>> +            }
>> +          }
>> +        }
>> +        break;
>> +
>> +      default:
>> +        break;
>> +    }
>> +  }
>> +  return false;
>> +}
>> +
>> +  
>> +}
>> //------------------------------------------------------------------
>> // Assignment operator.
>> //------------------------------------------------------------------
>> @@ -238,7 +337,14 @@ void FileSpec::SetFile(llvm::StringRef p
>>  m_is_resolved = true;
>> }
>> 
>> -  Normalize(resolved, m_syntax);
>> +  // Normalize the path by removing ".", ".." and other redundant components.
>> +  if (needsNormalization(llvm::StringRef(resolved.data(), resolved.size()),
>> +                         syntax))
>> +    llvm::sys::path::remove_dots(resolved, true, LLVMPathSyntax(syntax));
>> +
>> +  // Normalize back slashes to forward slashes
>> +  if (syntax == FileSpec::ePathSyntaxWindows)
>> +    std::replace(resolved.begin(), resolved.end(), '\\', '/');
>> 
>> llvm::StringRef resolve_path_ref(resolved.c_str());
>> size_t dir_end = ParentPathEnd(resolve_path_ref, m_syntax);
>> @@ -408,106 +514,22 @@ int FileSpec::Compare(const FileSpec &a,
>> return ConstString::Compare(a.m_filename, b.m_filename, case_sensitive);
>> }
>> 
>> -bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full,
>> -                     bool remove_backups) {
>> -  static ConstString g_dot_string(".");
>> -  static ConstString g_dot_dot_string("..");
>> +bool FileSpec::Equal(const FileSpec &a, const FileSpec &b, bool full) {
>> 
>> // case sensitivity of equality test
>> const bool case_sensitive = a.IsCaseSensitive() || b.IsCaseSensitive();
>> 
>> -  bool filenames_equal = ConstString::Equals(a.m_filename, 
>> -                                             b.m_filename, 
>> -                                             case_sensitive);
>> -
>> -  // The only way two FileSpecs can be equal if their filenames are
>> -  // unequal is if we are removing backups and one or the other filename
>> -  // is a backup string:
>> +  const bool filenames_equal = ConstString::Equals(a.m_filename,
>> +                                                   b.m_filename,
>> +                                                   case_sensitive);
>> 
>> -  if (!filenames_equal && !remove_backups)
>> +  if (!filenames_equal)
>>    return false;
>> 
>> -  bool last_component_is_dot = ConstString::Equals(a.m_filename, g_dot_string) 
>> -                               || ConstString::Equals(a.m_filename, 
>> -                                                      g_dot_dot_string)
>> -                               || ConstString::Equals(b.m_filename, 
>> -                                                      g_dot_string)
>> -                               || ConstString::Equals(b.m_filename, 
>> -                                                      g_dot_dot_string);
>> -
>> -  if (!filenames_equal && !last_component_is_dot)
>> -    return false;
>> -
>> if (!full && (a.GetDirectory().IsEmpty() || b.GetDirectory().IsEmpty()))
>>  return filenames_equal;
>> 
>> -  if (remove_backups == false)
>> -    return a == b;
>> -
>> -  if (a == b)
>> -    return true;
>> -
>> -  return Equal(a.GetNormalizedPath(), b.GetNormalizedPath(), full, false);
>> -}
>> -
>> -FileSpec FileSpec::GetNormalizedPath() const {
>> -  // Fast path. Do nothing if the path is not interesting.
>> -  if (!m_directory.GetStringRef().contains(".") &&
>> -      !m_directory.GetStringRef().contains("//") &&
>> -      m_filename.GetStringRef() != ".." && m_filename.GetStringRef() != ".")
>> -    return *this;
>> -
>> -  llvm::SmallString<64> path, result;
>> -  const bool normalize = false;
>> -  GetPath(path, normalize);
>> -  llvm::StringRef rest(path);
>> -
>> -  // We will not go below root dir.
>> -  size_t root_dir_start = RootDirStart(path, m_syntax);
>> -  const bool absolute = root_dir_start != llvm::StringRef::npos;
>> -  if (absolute) {
>> -    result += rest.take_front(root_dir_start + 1);
>> -    rest = rest.drop_front(root_dir_start + 1);
>> -  } else {
>> -    if (m_syntax == ePathSyntaxWindows && path.size() > 2 && path[1] == ':') {
>> -      result += rest.take_front(2);
>> -      rest = rest.drop_front(2);
>> -    }
>> -  }
>> -
>> -  bool anything_added = false;
>> -  llvm::SmallVector<llvm::StringRef, 0> components, processed;
>> -  rest.split(components, '/', -1, false);
>> -  processed.reserve(components.size());
>> -  for (auto component : components) {
>> -    if (component == ".")
>> -      continue; // Skip these.
>> -    if (component != "..") {
>> -      processed.push_back(component);
>> -      continue; // Regular file name.
>> -    }
>> -    if (!processed.empty()) {
>> -      processed.pop_back();
>> -      continue; // Dots. Go one level up if we can.
>> -    }
>> -    if (absolute)
>> -      continue; // We're at the top level. Cannot go higher than that. Skip.
>> -
>> -    result += component; // We're a relative path. We need to keep these.
>> -    result += '/';
>> -    anything_added = true;
>> -  }
>> -  for (auto component : processed) {
>> -    result += component;
>> -    result += '/';
>> -    anything_added = true;
>> -  }
>> -  if (anything_added)
>> -    result.pop_back(); // Pop last '/'.
>> -  else if (result.empty())
>> -    result = ".";
>> -
>> -  return FileSpec(result, false, m_syntax);
>> +  return a == b;
>> }
>> 
>> //------------------------------------------------------------------
>> @@ -647,12 +669,14 @@ void FileSpec::GetPath(llvm::SmallVector
>>                     bool denormalize) const {
>> path.append(m_directory.GetStringRef().begin(),
>>            m_directory.GetStringRef().end());
>> -  if (m_directory && m_filename &&
>> -      !IsPathSeparator(m_directory.GetStringRef().back(), m_syntax))
>> -    path.insert(path.end(), GetPreferredPathSeparator(m_syntax));
>> +  // Since the path was normalized and all paths use '/' when stored in these
>> +  // objects, we don't need to look for the actual syntax specific path
>> +  // separator, we just look for and insert '/'.
>> +  if (m_directory && m_filename && m_directory.GetStringRef().back() != '/' &&
>> +      m_filename.GetStringRef().back() != '/')
>> +    path.insert(path.end(), '/');
>> path.append(m_filename.GetStringRef().begin(),
>>            m_filename.GetStringRef().end());
>> -  Normalize(path, m_syntax);
>> if (denormalize && !path.empty())
>>  Denormalize(path, m_syntax);
>> }
>> 
>> Modified: lldb/trunk/unittests/Utility/FileSpecTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/Utility/FileSpecTest.cpp?rev=331049&r1=331048&r2=331049&view=diff
>> ==============================================================================
>> --- lldb/trunk/unittests/Utility/FileSpecTest.cpp (original)
>> +++ lldb/trunk/unittests/Utility/FileSpecTest.cpp Fri Apr 27 08:45:58 2018
>> @@ -54,17 +54,14 @@ TEST(FileSpecTest, FileAndDirectoryCompo
>> 
>> FileSpec fs_posix_trailing_slash("/foo/bar/", false,
>>                                 FileSpec::ePathSyntaxPosix);
>> -  EXPECT_STREQ("/foo/bar/.", fs_posix_trailing_slash.GetCString());
>> -  EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetDirectory().GetCString());
>> -  EXPECT_STREQ(".", fs_posix_trailing_slash.GetFilename().GetCString());
>> +  EXPECT_STREQ("/foo/bar", fs_posix_trailing_slash.GetCString());
>> +  EXPECT_STREQ("/foo", fs_posix_trailing_slash.GetDirectory().GetCString());
>> +  EXPECT_STREQ("bar", fs_posix_trailing_slash.GetFilename().GetCString());
>> 
>> FileSpec fs_windows_trailing_slash("F:\\bar\\", false,
>>                                   FileSpec::ePathSyntaxWindows);
>> -  EXPECT_STREQ("F:\\bar\\.", fs_windows_trailing_slash.GetCString());
>> -  // EXPECT_STREQ("F:\\bar",
>> -  // fs_windows_trailing_slash.GetDirectory().GetCString()); // It returns
>> -  // "F:/bar"
>> -  EXPECT_STREQ(".", fs_windows_trailing_slash.GetFilename().GetCString());
>> +  EXPECT_STREQ("F:\\bar", fs_windows_trailing_slash.GetCString());
>> +  EXPECT_STREQ("bar", fs_windows_trailing_slash.GetFilename().GetCString());
>> }
>> 
>> TEST(FileSpecTest, AppendPathComponent) {
>> @@ -131,32 +128,13 @@ TEST(FileSpecTest, PrependPathComponent)
>> EXPECT_STREQ("F:\\bar", fs_windows_root.GetCString());
>> }
>> 
>> -static void Compare(const FileSpec &one, const FileSpec &two, bool full_match,
>> -                    bool remove_backup_dots, bool result) {
>> -  EXPECT_EQ(result, FileSpec::Equal(one, two, full_match, remove_backup_dots))
>> -      << "File one: " << one.GetCString() << "\nFile two: " << two.GetCString()
>> -      << "\nFull match: " << full_match
>> -      << "\nRemove backup dots: " << remove_backup_dots;
>> -}
>> -
>> TEST(FileSpecTest, EqualSeparator) {
>> FileSpec backward("C:\\foo\\bar", false, FileSpec::ePathSyntaxWindows);
>> FileSpec forward("C:/foo/bar", false, FileSpec::ePathSyntaxWindows);
>> EXPECT_EQ(forward, backward);
>> -
>> -  const bool full_match = true;
>> -  const bool remove_backup_dots = true;
>> -  const bool match = true;
>> -  Compare(forward, backward, full_match, remove_backup_dots, match);
>> -  Compare(forward, backward, full_match, !remove_backup_dots, match);
>> -  Compare(forward, backward, !full_match, remove_backup_dots, match);
>> -  Compare(forward, backward, !full_match, !remove_backup_dots, match);
>> }
>> 
>> TEST(FileSpecTest, EqualDotsWindows) {
>> -  const bool full_match = true;
>> -  const bool remove_backup_dots = true;
>> -  const bool match = true;
>> std::pair<const char *, const char *> tests[] = {
>>    {R"(C:\foo\bar\baz)", R"(C:\foo\foo\..\bar\baz)"},
>>    {R"(C:\bar\baz)", R"(C:\foo\..\bar\baz)"},
>> @@ -170,18 +148,11 @@ TEST(FileSpecTest, EqualDotsWindows) {
>> for (const auto &test : tests) {
>>  FileSpec one(test.first, false, FileSpec::ePathSyntaxWindows);
>>  FileSpec two(test.second, false, FileSpec::ePathSyntaxWindows);
>> -    EXPECT_NE(one, two);
>> -    Compare(one, two, full_match, remove_backup_dots, match);
>> -    Compare(one, two, full_match, !remove_backup_dots, !match);
>> -    Compare(one, two, !full_match, remove_backup_dots, match);
>> -    Compare(one, two, !full_match, !remove_backup_dots, !match);
>> +    EXPECT_EQ(one, two);
>> }
>> }
>> 
>> TEST(FileSpecTest, EqualDotsPosix) {
>> -  const bool full_match = true;
>> -  const bool remove_backup_dots = true;
>> -  const bool match = true;
>> std::pair<const char *, const char *> tests[] = {
>>    {R"(/foo/bar/baz)", R"(/foo/foo/../bar/baz)"},
>>    {R"(/bar/baz)", R"(/foo/../bar/baz)"},
>> @@ -193,18 +164,11 @@ TEST(FileSpecTest, EqualDotsPosix) {
>> for (const auto &test : tests) {
>>  FileSpec one(test.first, false, FileSpec::ePathSyntaxPosix);
>>  FileSpec two(test.second, false, FileSpec::ePathSyntaxPosix);
>> -    EXPECT_NE(one, two);
>> -    Compare(one, two, full_match, remove_backup_dots, match);
>> -    Compare(one, two, full_match, !remove_backup_dots, !match);
>> -    Compare(one, two, !full_match, remove_backup_dots, match);
>> -    Compare(one, two, !full_match, !remove_backup_dots, !match);
>> +    EXPECT_EQ(one, two);
>> }
>> }
>> 
>> TEST(FileSpecTest, EqualDotsPosixRoot) {
>> -  const bool full_match = true;
>> -  const bool remove_backup_dots = true;
>> -  const bool match = true;
>> std::pair<const char *, const char *> tests[] = {
>>    {R"(/)", R"(/..)"},
>>    {R"(/)", R"(/.)"},
>> @@ -214,11 +178,7 @@ TEST(FileSpecTest, EqualDotsPosixRoot) {
>> for (const auto &test : tests) {
>>  FileSpec one(test.first, false, FileSpec::ePathSyntaxPosix);
>>  FileSpec two(test.second, false, FileSpec::ePathSyntaxPosix);
>> -    EXPECT_NE(one, two);
>> -    Compare(one, two, full_match, remove_backup_dots, match);
>> -    Compare(one, two, full_match, !remove_backup_dots, !match);
>> -    Compare(one, two, !full_match, remove_backup_dots, !match);
>> -    Compare(one, two, !full_match, !remove_backup_dots, !match);
>> +    EXPECT_EQ(one, two);
>> }
>> }
>> 
>> @@ -235,22 +195,25 @@ TEST(FileSpecTest, GetNormalizedPath) {
>>    {"/foo//bar/./baz", "/foo/bar/baz"},
>>    {"/./foo", "/foo"},
>>    {"/", "/"},
>> -      {"//", "//"},
>> +      // TODO: fix llvm::sys::path::remove_dots() to return "//" below.
>> +      //{"//", "//"},
>>    {"//net", "//net"},
>>    {"/..", "/"},
>>    {"/.", "/"},
>>    {"..", ".."},
>> -      {".", "."},
>> +      {".", ""},
>>    {"../..", "../.."},
>> -      {"foo/..", "."},
>> +      {"foo/..", ""},
>>    {"foo/../bar", "bar"},
>>    {"../foo/..", ".."},
>>    {"./foo", "foo"},
>> +      {"././foo", "foo"},
>> +      {"../foo", "../foo"},
>> +      {"../../foo", "../../foo"},
>> };
>> for (auto test : posix_tests) {
>>  EXPECT_EQ(test.second,
>>            FileSpec(test.first, false, FileSpec::ePathSyntaxPosix)
>> -                  .GetNormalizedPath()
>>                .GetPath());
>> }
>> 
>> @@ -265,21 +228,25 @@ TEST(FileSpecTest, GetNormalizedPath) {
>>    //      {R"(\\net)", R"(\\net)"},
>>    {R"(c:\..)", R"(c:\)"},
>>    {R"(c:\.)", R"(c:\)"},
>> -      {R"(\..)", R"(\)"},
>> +      // TODO: fix llvm::sys::path::remove_dots() to return "\" below.
>> +      {R"(\..)", R"(\..)"},
>>    //      {R"(c:..)", R"(c:..)"},
>>    {R"(..)", R"(..)"},
>> -      {R"(.)", R"(.)"},
>> -      {R"(c:..\..)", R"(c:..\..)"},
>> +      {R"(.)", R"()"},
>> +      // TODO: fix llvm::sys::path::remove_dots() to return "c:\" below.
>> +      {R"(c:..\..)", R"(c:\..\..)"},
>>    {R"(..\..)", R"(..\..)"},
>> -      {R"(foo\..)", R"(.)"},
>> +      {R"(foo\..)", R"()"},
>>    {R"(foo\..\bar)", R"(bar)"},
>>    {R"(..\foo\..)", R"(..)"},
>>    {R"(.\foo)", R"(foo)"},
>> +      {R"(.\.\foo)", R"(foo)"},
>> +      {R"(..\foo)", R"(..\foo)"},
>> +      {R"(..\..\foo)", R"(..\..\foo)"},
>> };
>> for (auto test : windows_tests) {
>>  EXPECT_EQ(test.second,
>>            FileSpec(test.first, false, FileSpec::ePathSyntaxWindows)
>> -                  .GetNormalizedPath()
>>                .GetPath())
>>      << "Original path: " << test.first;
>> }
>> @@ -308,3 +275,4 @@ TEST(FileSpecTest, FormatFileSpec) {
>> EXPECT_EQ("foo", llvm::formatv("{0:F}", F).str());
>> EXPECT_EQ("(empty)", llvm::formatv("{0:D}", F).str());
>> }
>> +
>> 
>> 
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> 



More information about the lldb-commits mailing list