dexonsmith added inline comments.
================ Comment at: llvm/include/llvm/Support/VirtualFileSystem.h:735 + /// Redirect each of the remapped files from first to second. + static std::unique_ptr<RedirectingFileSystem> + create(ArrayRef<std::pair<std::string, std::string>> RemappedFiles, ---------------- JDevlieghere wrote: > - Do you need to know that this is a `RedirectingFileSystem` or can it be a > `FileSystem`? > - Is there a reason this should be a `unique_ptr` instead of an > `IntrusiveRefCntPtr` (which is more prevalent across the VFS, although I'm > not sure why) For my use case it doesn't matter what kind of `FileSystem` is returned; I figure this could be convenient for a future client. I would tend toward keeping it this type to match the other overload of `RedirectingFileSystem::create`, but happy to change it if you disagree. Regarding `unique_ptr`, I chose that as a sounder alternative to the raw pointer above; leaves options open to clients if they do want unique ownership, which seems nice to support since it's trivial. My use case will ultimately assign it to an `IntrusiveRefCntPtr` so if you disagree I can change it. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91317/new/ https://reviews.llvm.org/D91317 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
