dexonsmith added a comment. In D91204#2387050 <https://reviews.llvm.org/D91204#2387050>, @saudi wrote:
> Updated the patch. > > Followed suggestion from @dexonsmith. Indeed it simplifies the code. > Also, improved the test, to also test with -j 2 A couple of more comments. @arphaman , can you confirm this is a good direction? ================ 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); ---------------- 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. ================ 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 ---------------- Can you use `CHECK-DAG` for this? https://www.llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive ================ Comment at: clang/test/ClangScanDeps/relative_directory.cpp:19-25 +// CHECK1: relative_directory_input1.o: +// CHECK1-NEXT: relative_directory_input1.cpp +// CHECK1-NEXT: header.h + +// CHECK2: relative_directory_input2.o: +// CHECK2-NEXT: relative_directory_input2.cpp +// CHECK2-NEXT: header.h ---------------- I.e., maybe this can be: ``` // CHECK-DAG: relative_directory_input1.o: // CHECK-DAG: relative_directory_input1.cpp // CHECK-DAG: header.h // CHECK-DAG: relative_directory_input2.o: // CHECK-DAG: relative_directory_input2.cpp // CHECK-DAG: header.h ``` 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
