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

[Sylvain Audi via Phabricator via cfe-commits]

saudi marked 2 inline comments as not done.
saudi 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);
----------------
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());
```



================
Comment at: clang/test/ClangScanDeps/relative_directory.cpp:12-13
+
+// The output order is non-deterministic when using more than one thread,
+// so check the output using two runs.
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 | FileCheck --check-prefix=CHECK1 %s
----------------
dexonsmith wrote:
> Can you use `CHECK-DAG` for this?
> https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive
If I understand correctly from the doc, it would lose the order (CHECK1, CHECK1-NEXT).
We need 2 blocks of 3 lines to be present, but only the blocks can be in any order.

I did this mimicking what is done in other tests like regular_cdb.cpp.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91204/new/

https://reviews.llvm.org/D91204