https://github.com/labath commented:
Mostly yes. It's not as clean as I would have liked, though I suppose that is
to be expected of an attempt to retrofit a significant change like this. The
existence of Debugger::Get{Output,Error}File in particular is a very large hole
in the RAII locking scheme. From the looks of things, none of the uses of these
functions (GetErrorFile is unused already) is particularly load-bearing and
they could be replaced by something else (but don't do that here, as the patch
is big enough already).
The RAII thing doesn't make things much safer than the original implementation
did because the Printf currently translate to a single `Write` call anyway, but
they are more future-proof, as I can easily see someone "modernising" them into
a `Format` call and then someone else "optimizing" our `Format` implementation
to use the llvm raw_ostream method (which does *not* use a single write call).
The current implementation will work even with those changes.
I do want to spend some time discussing the relationship between stdout and
stderr. The current implementation uses a separate mutex for each stream, which
is not *un*reasonable, but I can see at least two other possible philosophies:
1. Since both of the streams will be going to the same terminal most of the
time, it might make sense to use a single mutex for both of them, as that's
sort of the only way to ensure consistent output. (Imagine the situation of
printing the "stderr" of a CLI command while the status line is being updated)
2. Alternatively, we could say that stderr is for immediate and exceptional
output where it is more important to get the message to the user than it being
formatted beautifully. In this world, we could keep "stderr" as a regular
unlocked stream. In specific cases, where synchronizing the output is
particularly important to us (like the beforementioned case of "stderr" of a
CLI command) we could synchronize its output by holding the "stdout" mutex
while writing it.
What do you think of that? I'm sort of leaning towards option two since stderr
might be used in contexts where holding a mutex might not be completely safe
(and the first option would make that even worse), and the current
implementation with separate mutexes doesn't really guarantee "reasonable"
stdout/err sequencing.
https://github.com/llvm/llvm-project/pull/126630
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
