labath added a comment. This looks much better. LGTM, just make sure to do something with the lower_bound search, as I don't think that's right.
================ Comment at: include/lldb/Utility/Reproducer.h:58-59 + virtual const char *GetName() const = 0; + virtual const char *GetFile() const = 0; + ---------------- return StringRef ? ================ Comment at: include/lldb/Utility/Reproducer.h:164 FileSpec m_root; + std::vector<std::string> m_files; bool m_loaded; ---------------- This might even be a vector of StringRefs, if you're ok with saying that the Provider class is responsible for these strings with sufficient lifetime. In the current setup it kind of already is because there's no telling who will use the returned c strings and when. ================ Comment at: source/Utility/Reproducer.cpp:218-219 assert(m_loaded); - - auto it = m_provider_info.find(name); - if (it == m_provider_info.end()) - return llvm::None; - - return it->second; + auto it = std::lower_bound(m_files.begin(), m_files.end(), file.str()); + return it != m_files.end(); } ---------------- This doesn't seem right. This will only return false if `file` is lexicographically after all files in the `m_files` array. Perhaps you meant `return it != m_files.end() && *it == file;` ? Repository: rLLDB LLDB CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56814/new/ https://reviews.llvm.org/D56814 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
