davide added a comment. Some comments. Thanks!
================ Comment at: include/lldb/Utility/FileCollector.h:5-7 +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// ---------------- You might want to update the license. ================ Comment at: include/lldb/Utility/FileCollector.h:50-54 + std::mutex m_mutex; + FileSpec m_root; + llvm::StringSet<> m_seen; + llvm::vfs::YAMLVFSWriter m_vfs_writer; + llvm::StringMap<std::string> m_symlink_map; ---------------- Can we add comments to these member variables? Some are not entirely obvious to me, e.g. what's `m_root`? What's `m_mutex` is supposed to protect . I think we should document these invariants. ================ Comment at: source/Utility/FileCollector.cpp:27-33 + // Change path to all upper case and ask for its real path, if the latter + // exists and is equal to path, it's not case sensitive. Default to case + // sensitive in the absence of real_path, since this is the YAMLVFSWriter + // default. + upper_dest = path.upper(); + if (sys::fs::real_path(upper_dest, real_dest) && path.equals(real_dest)) + return false; ---------------- should this be a function in FS to check the case-sensitiveness? ================ Comment at: source/Utility/FileCollector.cpp:84-87 + // Canonicalize the source path by removing "..", "." components. + SmallString<256> virtual_path = absolute_src; + sys::path::remove_dots(virtual_path, /*remove_dot_dot=*/true); + ---------------- Ditto (I thought there was one to canonicalize the source path?) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D54617/new/ https://reviews.llvm.org/D54617 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
