[Lldb-commits] [PATCH] D67891: remove File::SetStream(), make new files instead.
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 25 05:33:53 PDT 2019
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.
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the lldb-commits