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@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
