labath accepted this revision.
labath added a comment.
I have a couple of more comments, including some things I missed on the
previous pass, but I don't want to hold this up any more. Feel free to commit
after taking the last batch into consideration.
================
Comment at: lldb/include/lldb/Utility/Reproducer.h:116
+public:
+ DataRecorder(FileSpec filename, std::error_code ec)
+ : m_filename(std::move(filename)),
----------------
The error_code will need to be a reference argument for this to have any effect.
================
Comment at: lldb/source/API/SBDebugger.cpp:63
+public:
+ CommandLoader() {
+ repro::Loader *loader = repro::Reproducer::Instance().GetLoader();
----------------
Use a factory function here too?
================
Comment at: lldb/source/API/SBDebugger.cpp:74
+ if (auto err = error_or_file.getError())
+ return;
+
----------------
It might be nice to log these errors even if we don't plan to do anything about
them.
================
Comment at: lldb/source/Core/Debugger.cpp:887
+
void Debugger::SetInputFileHandle(FILE *fh, bool tranfer_ownership) {
if (m_input_file_sp)
----------------
Sticking with the "shadow" idea, I think it would be better if the input
recorder is set as an extra argument to this function (instead of as a separate
call), because they should always be changed together. Setting one without the
other is almost certainly a mistake.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58564/new/
https://reviews.llvm.org/D58564
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
