labath wrote:

> > > A random idea, I don't know if it's a good one: have one object 
> > > (LockableStreamPair?) that holds the two streams and their mutex. One 
> > > less argument to pass around?
> > 
> > 
> > Sounds good, that simplifies the signatures (which this PR is already 
> > touching) and keeps the mutex abstracted away.
> 
> I did implement this and I'm not happy with the result: even though you only 
> pass one object, you still need to get the output or error stream out of it 
> which ends up being more verbose. And because of the `IOHandler` and 
> `Debugger::AdoptTopIOHandlerFilesIfInvalid` it needs to be a shared pointer. 
> Here's what that looks like: 
> [d62fdae](https://github.com/llvm/llvm-project/commit/d62fdaec4cd322574e1ab78c8cacd1effe2c29c0).
>  It compiles but some tests are failing. Probably something small but given I 
> don't like the approach I haven't looked into it.

That's okay. I thought it might not work out. Thanks for giving it a shot.

I feel bad about bringing this up after you did all the work, but there is one 
more thing going through my head. I now have a very awkward feeling about this 
because it seems like this is solving mostly the same problem that 
StreamAsynchronousIO (and Debugger::PrintAsync) was trying to solve. You 
mentioned that in the first version of the patch and said that it doesn't work 
in some cases, but it's not quite clear why is that the case. If the motivation 
is status line, then it sounds like it *might* be possible to plumb the updates 
through the async streams.

I can imagine that it might not work for some reason, but then I think it's 
important to know why, and come up with some sort of an story for which of the 
two systems should be used for a particular use case. Or, (I am dreaming, I 
know) actually deprecate/remove the async IO system.

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

Reply via email to