labath added a comment.
I think this looks mostly fine. See my comment about not using SB classes in
the reproducer api. I still kind of like the idea of naming the reproducer
class in some special way, to make it more obvious that it is not "just
another" SB class, but I'm not sure if that would be just more confusing.
================
Comment at: lldb/include/lldb/API/SBReproducer.h:18-20
static bool Replay();
+ static lldb::SBError InitializeCapture(const char *path);
+ static lldb::SBError InitializeReplay(const char *path);
----------------
So, in this new world, how are we expected to perform the replay?
`InitializeReplay()`, followed by `Replay()` ? Could we fold those two calls
into one? I don't see what meaningful work could the user do between those two
calls, as all sb calls (including SBDebugger::Initialize) will now be recorded
(right?).
================
Comment at: lldb/source/API/SBDebugger.cpp:127
void SBDebugger::Initialize() {
- SBInitializerOptions options;
- SBDebugger::Initialize(options);
+ SBError ignored = SBDebugger::InitializeWithErrorHandling();
}
----------------
I think the usual way to do that is to just cast the result to void.
================
Comment at: lldb/source/API/SBReproducer.cpp:49
+SBError SBReproducer::InitializeCapture(const char *path) {
+ SBError error;
+ if (auto e = Reproducer::Initialize(ReproducerMode::Capture, FileSpec(path)))
----------------
JDevlieghere wrote:
> The SBError is a problem. We can create it after the initialization, but then
> we’d need a bogus repro::Record to make sure the api boundary is correctly
> registered. Let me know if you can think of an alternative.
Maybe this function should not return an SBError at all (just like it does not
accept an SBFileSpec due to bootstrapping problems). You could just return a
std::string, and have an empty string denote success. This would be consistent
with the idea that the repro API is not a part of SB API, but rather something
sitting slightly above (or at least, besides) it.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58410/new/
https://reviews.llvm.org/D58410
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
