JDevlieghere planned changes to this revision.
JDevlieghere added a comment.
I'll add a more specific test for the collector.
================
Comment at: source/Host/common/FileSystem.cpp:443
+
+ // If VFS mapped we know the underlying FS is a RedirectingFileSystem.
+ ErrorOr<vfs::Entry *> E =
----------------
labath wrote:
> What are the chances of making this slightly more official by making
> `getVFSFromYAML` return a `IntrusiveRefCntPtr<RedirectingFileSystem>` instead
> of a generic pointer?
I'm not sure I understand what the benefit would be, we'd still store it as a
generic FileSystem in the member variable?
================
Comment at: source/Initialization/SystemInitializerCommon.cpp:82
+ if (repro::Loader *loader = r.GetLoader()) {
+ if (auto info = loader->GetProviderInfo("files")) {
+ FileSpec vfs_mapping(loader->GetRoot());
----------------
labath wrote:
> Would it be possible to avoid magic strings here, and use something like
> `GetProviderInfo<FileProvider>()` instead?
My idea was to decouple the provider (used only for capture) and the info
needed for replay, but I think we can improve things by merging them into one.
I don't think I've run into a situation yet where we don't have access to the
provider during replay.
================
Comment at: source/Initialization/SystemInitializerCommon.cpp:87
+ } else {
+ // No file provider, continue with the real filesystem.
+ FileSystem::Initialize();
----------------
labath wrote:
> When can this happen? Should this be an error or maybe even an assert?
Yes, reproducers should still work with "some" functionality disabled. The
infrastructure is not there yet, but it would be nice to test pieces
individually.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54617/new/
https://reviews.llvm.org/D54617
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
