[Lldb-commits] [PATCH] D56814: [Reproducers] Refactor reproducer info
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Thu Jan 17 00:14:22 PST 2019
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
+ std::vector<std::string> m_files;
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
- 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;` ?
CHANGES SINCE LAST ACTION
More information about the lldb-commits