labath added a comment.
The code looks fine to me, though it sounds like there are still issues to be
sorted out w.r.t commands run from inside data formatters, through sb apis,
etc. And it needs tests, of course.
================
Comment at: lldb/source/Commands/CommandObjectSession.h:22-26
+ ~CommandObjectSession() override;
+
+private:
+ CommandObjectSession(const CommandObjectSession &) = delete;
+ const CommandObjectSession &operator=(const CommandObjectSession &) = delete;
----------------
You don't have to do this, but personally I'd delete all of this stuff -- the
copy operations are implicitly deleted due to them being deleted in the base
class, and the destructor will be implicitly overridden anyway.
================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2945
+ auto string_stream =
+ std::static_pointer_cast<StreamString>(m_session_transcripts);
+ size_t stream_size = string_stream->GetSize();
----------------
labath wrote:
> Why not make `m_session_transcripts` a `shared_ptr<StreamString>` (or even a
> plain `StreamString`) in the first place?
Much better, though a plain `StreamString` would be even nicer (and AFAICT
there is nothing preventing that).
================
Comment at: lldb/source/Interpreter/CommandInterpreter.cpp:2928
+ if (!opened_file)
+ return error_out("Unable to create file");
+
----------------
It would be nice if this error message actually contained the reason why the
open operation failed (similar to what we did with the core file patch a while
back).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82155/new/
https://reviews.llvm.org/D82155
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
