labath added a comment.
I also think it we should try to keep FileSpec in the Utility module. We should
be able to do that by deleting the "utility" functions in the FileSpec class,
and updating everything to go through the FileSystem class.
After the VFS introduction, something like `file_spec.Exists()` will not even
be well-defined anyway, as one will have to specify which file system are we
checking the existence in. So, it would be more correct to say
`file_spec.Exists(my_vfs)`, at which point it's not too much of a stretch to
change this into `my_vfs.Exists(file_spec)`.
(This is the same argument I was giving earlier, since even before VFS we kinda
were dealing with multiple file systems (host, remote device). In fact, after
this it might be a nice cleanup to change the Platform class from vending a
bunch of FS-like methods to (FileExists, Unlink, CreateSymlink, ...), to have a
single `getRemoteFS` method returning a VFS which does the right thing "under
the hood").
tl;dr: I'd propose to (in two or more patches):
- replace FileSpec "utility" methods with equivalent FileSystem functionality
- do the necessary FileSystem changes to support VFS
================
Comment at: source/Host/common/FileSystem.cpp:24-29
+ static FileSystem *g_fs;
+ static llvm::once_flag g_once_flag;
+ llvm::call_once(g_once_flag, []() {
+ if (g_fs == nullptr)
+ g_fs = new FileSystem();
+ });
----------------
replace with `static FileSystem *g_fs = new FileSystem();`
================
Comment at: source/Host/common/FileSystem.cpp:33-35
+void FileSystem::ChangeFileSystem(IntrusiveRefCntPtr<vfs::FileSystem> fs) {
+ m_fs = fs;
+}
----------------
I'm not sure what's your use case here, but would it make more sense instead of
modifying the singleton object to have more that one FileSystem instance
floating around? It could be local to each `Debugger` (?) object and everyone
would fetch it from there when needed.
https://reviews.llvm.org/D53532
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
