This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
JDevlieghere marked 2 inline comments as done.
Closed by commit rL351501: [Reproducers] Refactor reproducer info (authored by
JDevlieghere, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D56814?vs=182159&id=182420#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56814/new/
https://reviews.llvm.org/D56814
Files:
lldb/trunk/include/lldb/Utility/Reproducer.h
lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
lldb/trunk/source/Utility/Reproducer.cpp
Index: lldb/trunk/source/Utility/Reproducer.cpp
===================================================================
--- lldb/trunk/source/Utility/Reproducer.cpp
+++ lldb/trunk/source/Utility/Reproducer.cpp
@@ -178,10 +178,13 @@
sys::fs::OpenFlags::F_None);
yaml::Output yout(*strm);
+ std::vector<std::string> files;
+ files.reserve(m_providers.size());
for (auto &provider : m_providers) {
- auto &provider_info = provider.second->GetInfo();
- yout << const_cast<ProviderInfo &>(provider_info);
+ files.emplace_back(provider.second->GetFile());
}
+
+ yout << files;
}
Loader::Loader(const FileSpec &root) : m_root(root), m_loaded(false) {}
@@ -196,29 +199,24 @@
if (auto err = error_or_file.getError())
return make_error<StringError>("unable to load reproducer index", err);
- std::vector<ProviderInfo> provider_info;
yaml::Input yin((*error_or_file)->getBuffer());
- yin >> provider_info;
-
+ yin >> m_files;
if (auto err = yin.error())
return make_error<StringError>("unable to read reproducer index", err);
- for (auto &info : provider_info)
- m_provider_info[info.name] = info;
+ // Sort files to speed up search.
+ llvm::sort(m_files);
+ // Remember that we've loaded the index.
m_loaded = true;
return llvm::Error::success();
}
-llvm::Optional<ProviderInfo> Loader::GetProviderInfo(StringRef name) {
+bool Loader::HasFile(StringRef file) {
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()) && (*it == file);
}
void ProviderBase::anchor() {}
Index: lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
===================================================================
--- lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ lldb/trunk/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -157,17 +157,24 @@
return g_settings_sp;
}
+struct ProcessGDBRemoteInfo {
+ static const char *name;
+ static const char *file;
+};
+
+const char *ProcessGDBRemoteInfo::name = "gdb-remote";
+const char *ProcessGDBRemoteInfo::file = "gdb-remote.yaml";
+
class ProcessGDBRemoteProvider
: public repro::Provider<ProcessGDBRemoteProvider> {
public:
+ typedef ProcessGDBRemoteInfo info;
+
ProcessGDBRemoteProvider(const FileSpec &directory) : Provider(directory) {
- m_info.name = "gdb-remote";
- m_info.files.push_back("gdb-remote.yaml");
}
raw_ostream *GetHistoryStream() {
- FileSpec history_file =
- GetRoot().CopyByAppendingPathComponent("gdb-remote.yaml");
+ FileSpec history_file = GetRoot().CopyByAppendingPathComponent(info::file);
std::error_code EC;
m_stream_up = llvm::make_unique<raw_fd_ostream>(history_file.GetPath(), EC,
@@ -3432,16 +3439,10 @@
if (!loader)
return Status("No loader provided.");
- auto provider_info = loader->GetProviderInfo("gdb-remote");
- if (!provider_info)
- return Status("No provider for gdb-remote.");
-
- if (provider_info->files.empty())
- return Status("Provider for gdb-remote contains no files.");
-
// Construct replay history path.
- FileSpec history_file = loader->GetRoot().CopyByAppendingPathComponent(
- provider_info->files.front());
+ FileSpec history_file = loader->GetFile<ProcessGDBRemoteInfo>();
+ if (!history_file)
+ return Status("No provider for gdb-remote.");
// Enable replay mode.
m_replay_mode = true;
Index: lldb/trunk/include/lldb/Utility/Reproducer.h
===================================================================
--- lldb/trunk/include/lldb/Utility/Reproducer.h
+++ lldb/trunk/include/lldb/Utility/Reproducer.h
@@ -31,24 +31,14 @@
Off,
};
-/// Abstraction for information associated with a provider. This information
-/// is serialized into an index which is used by the loader.
-struct ProviderInfo {
- std::string name;
- std::vector<std::string> files;
-};
-
/// The provider defines an interface for generating files needed for
-/// reproducing. The provider must populate its ProviderInfo to communicate
-/// its name and files to the index, before registering with the generator,
-/// i.e. in the constructor.
+/// reproducing.
///
/// Different components will implement different providers.
class ProviderBase {
public:
virtual ~ProviderBase() = default;
- const ProviderInfo &GetInfo() const { return m_info; }
const FileSpec &GetRoot() const { return m_root; }
/// The Keep method is called when it is decided that we need to keep the
@@ -65,11 +55,12 @@
// Returns the class ID for the dynamic type of this Provider instance.
virtual const void *DynamicClassID() const = 0;
+ virtual llvm::StringRef GetName() const = 0;
+ virtual llvm::StringRef GetFile() const = 0;
+
protected:
ProviderBase(const FileSpec &root) : m_root(root) {}
- /// Every provider keeps track of its own files.
- ProviderInfo m_info;
private:
/// Every provider knows where to dump its potential files.
FileSpec m_root;
@@ -84,6 +75,9 @@
const void *DynamicClassID() const override { return &ThisProviderT::ID; }
+ llvm::StringRef GetName() const override { return ThisProviderT::info::name; }
+ llvm::StringRef GetFile() const override { return ThisProviderT::info::file; }
+
protected:
using ProviderBase::ProviderBase; // Inherit constructor.
};
@@ -152,14 +146,22 @@
public:
Loader(const FileSpec &root);
- llvm::Optional<ProviderInfo> GetProviderInfo(llvm::StringRef name);
+ template <typename T> FileSpec GetFile() {
+ if (!HasFile(T::file))
+ return {};
+
+ return GetRoot().CopyByAppendingPathComponent(T::file);
+ }
+
llvm::Error LoadIndex();
const FileSpec &GetRoot() const { return m_root; }
private:
- llvm::StringMap<ProviderInfo> m_provider_info;
+ bool HasFile(llvm::StringRef file);
+
FileSpec m_root;
+ std::vector<std::string> m_files;
bool m_loaded;
};
@@ -198,18 +200,4 @@
} // namespace repro
} // namespace lldb_private
-LLVM_YAML_IS_DOCUMENT_LIST_VECTOR(lldb_private::repro::ProviderInfo)
-
-namespace llvm {
-namespace yaml {
-
-template <> struct MappingTraits<lldb_private::repro::ProviderInfo> {
- static void mapping(IO &io, lldb_private::repro::ProviderInfo &info) {
- io.mapRequired("name", info.name);
- io.mapOptional("files", info.files);
- }
-};
-} // namespace yaml
-} // namespace llvm
-
#endif // LLDB_UTILITY_REPRODUCER_H
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
