[PATCH] D91204: [clang-scan-deps] Fix for input file given as relative path in compilation database "command" entry

Duncan P. N. Exon Smith via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 11 13:57:26 PST 2020


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



More information about the cfe-commits mailing list