labath added a comment. Thanks for splitting these up. It makes it much more obvious what is going on.
I see now that you're making use of this SetFile method which I wanted to remove in the previous patch. I kind of like that, because it pushes the `Set` one level up (so it now happens on the Stream instead of the File), but I do wonder if we shouldn't remove it completely. It seems like we could have Debugger::SetXXXFileHandle just create a fresh stream instead of twiddling with the existing one. This has the potential to change behavior as this change will not propagate to anybody who has fetched the stream_sp previously. However, I'm not sure if this is a change we care about because if anybody was writing to a stream while we were in the process of changing it, then he would likely trigger a race condition anyway. This way, we risk some code not immediately noticing the stream change, but at least we're thread safe. @jingham, what do you think about that? Other than that, I am pretty happy with how this looks, and I think you've convinced me that the shared pointer inside the StreamFile object is really needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D67891/new/ https://reviews.llvm.org/D67891 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits