> On Apr 27, 2018, at 11:17 AM, Jim Ingham via lldb-commits > <lldb-commits@lists.llvm.org> 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?
Once you get past the unit test, it looks like it also broke TestBreakpointCommand.py. Please fix this quickly. Fred > 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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
