dexonsmith added inline comments.
================ Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:158-160 + llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> PhysicalFileSystem( + llvm::vfs::createPhysicalFileSystem().release()); + RealFS = new llvm::vfs::ProxyFileSystem(PhysicalFileSystem); ---------------- saudi wrote: > dexonsmith wrote: > > I think this can just be: > > ``` > > RealFS = llvm::vfs::createPhysicalFileSystem(); > > ``` > > The `ProxyFileSystem` wrapper is a convenience for building other file > > systems; it doesn't do anything on its own IIRC. > Note that createPhysicalFileSystem() returns a `std::unique_ptr` whereas > `getRealFileSystem()` return type is `llvm::IntrusiveRfCntPtr`. > > Trying to change the type of RealFS requires more changes, as > `DependencyScanningWorkerFilesystem` is itself a `ProxyFileSystem` and thus > requires a `llvm::IntrusiveRfCntPtr` object. > > Should DependencyScanningWorkerFilesystem not be a ProxyFileSystem? @arphaman > WDYT? > However, in this case it might fall out of the scope of this patch, I'd > suggest we keep it as a separate patch. > > I updated my patch for an intermediate fix, which also passes the tests: > ``` > RealFS = > llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem>(llvm::vfs::createPhysicalFileSystem().release()); > ``` > Ah, didn't realize it returned `unique_ptr`. `RealFS` should be `IntrusiveRefCntPtr`, but you can be less verbose like this: ``` RealFS = llvm::vfs::createPhysicalFileSystem().release(); ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91204/new/ https://reviews.llvm.org/D91204 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
