kousikk added a comment. In D68193#1688838 <https://reviews.llvm.org/D68193#1688838>, @dexonsmith wrote:
> Sorry for bouncing you around, but I just had a look at the other user of > `createFileEntry` and I think the right thing to do is to somehow share the > code between `DependencyScanningWorkerFilesystem::status` and this. I > suggest splitting a function out of `status` (called > `getOrCreateFileSystemEntry`?) that returns a `CachedFileSystemEntry` (or an > error). > > The fix I just asked you to make (to only call `stat` once) could be done on > `getOrCreateFileSystemEntry` as a follow-up, using > `std::unique_ptr<llvm::vfs::File>` and changing `getFileEntry` to take that > instead of a filename. IIUC, `DependencyScanningWorkerFilesystem::status()` and `DependencyScanningWorkerFilesystem::openFileForRead()` both inturn call into `CachedFileSystemEntry::createFileEntry()`. Given that, I made `getOrCreateFileEntry()` a private-member of `DependencyScanningWorkerFilesystem` and made both these functions use it first. In a subsequent step, I passed in `llvm::vfs::Status` object to `createFileEntry()` instead of `Filename`. I hope this aligns with the code-organization you are looking for. Let me know if this aligns with how you wanted the code to look (happy to re-write if not). I also ran it against a canonical set of tests I'm maintaining internally and I didn't notice a difference in either performance and didn't get new crashes of the tool, so I think there should NOT be any regressions because of this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68193/new/ https://reviews.llvm.org/D68193 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
