Buildbots should be fixed with 331082.
> On Apr 27, 2018, at 1:01 PM, Frédéric Riss <fr...@apple.com> wrote: > > > >> On Apr 27, 2018, at 11:17 AM, Jim Ingham via lldb-commits >> <lldb-commits@lists.llvm.org <mailto: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 >> >> <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 <mailto:lldb-commits@lists.llvm.org> >> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits >> <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
