[Lldb-commits] [PATCH] D67996: Convert FileSystem::Open() to return Expected<FileUP>

[Lawrence D'Anna via Phabricator via lldb-commits]

lawrence_danna marked 2 inline comments as done.
lawrence_danna added inline comments.


================
Comment at: lldb/include/lldb/Core/StreamFile.h:48-53
+  void SetFile(std::shared_ptr<File> file) {
+    if (file)
+      m_file_sp = file;
+    else
+      m_file_sp = std::make_shared<File>();
+  }
----------------
labath wrote:
> Could we remove this method? It would make things easier to reason about if we could disallow swapping of a File object backing a stream midway through its lifetime. Looking at the existing callers, it does not seem it should be hard to do that -- this is always called immediately after a stream is constructed via patterns like:
> ```
> auto *stream = new StreamFile();
> auto file = create_a_file();
> if (file.is_ok()) {
>   stream->SetFile(file);
>   use(stream);
> }
> ```
> It should be easy to change that so that the stream is constructed only after we have a valid File object. It would also avoid the need to construct a fake File object just to guarantee it is always initialized.
Yea, it wasn't' too complicated to get rid of it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67996/new/

https://reviews.llvm.org/D67996