[Lldb-commits] [PATCH] D67891: remove File::SetStream(), make new files instead.

[Pavel Labath via Phabricator via lldb-commits]

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