https://github.com/qiongsiwu commented:
Thanks for working on the PR! This overall looks great! > reasons to put some functions to the header file while you move them to the > implementation file (`initVFSForTUBufferScanning` for example). I believe the reason I needed these `init` function declarations in the header file was that these functions could be called in `DependencyScanningWorker.cpp` and `DependencyScannerImpl.cpp`. I can see that we are now moving the initializations to `DependencyScanningTool.cpp`, so I think it is good to remove these decls from the header file. I concur with @jansvoboda11 that we can partition this into a few different PRs. I am thinking maybe we can partition the changes into three PRs: 1. NFC: cleaning up function parameter types. There are a few places where we are replacing `std::vector &`s with `ArrayRef`s. The places Jan pointed out could go into this PR. Other NFC changes can go in here as well. 2. Refactor the initialization and finalization logic. We can use this PR to move the initialization and finalization code out of the impl/worker files to `DependencyScanningTool.cpp`. I suspect it is difficult to make a completely clean cut since things are entangled, but I think we can give this a shot. 3. Feature: we can now use the new APIs moved to `DependencyScanningTool.cpp` to remove `DependencyScanningWorker`'s dependency on the driver. Does this sound reasonable? https://github.com/llvm/llvm-project/pull/169964 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
