jansvoboda11 accepted this revision.
jansvoboda11 added inline comments.
This revision is now accepted and ready to land.
================
Comment at:
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h:64-66
+/// Dependency scanner callbacks that are used during scanning to influence the
+/// behaviour of the scan - for example, to customize the scanned invocations.
+class DependencyActions {
----------------
benlangmuir wrote:
> jansvoboda11 wrote:
> > What do the downstream callbacks do? The class name sounds a bit generic
> > for something you can call `lookupModuleOutput()` on, but maybe that's the
> > right name with more context. It's also very similar to the existing
> > `DependencyScanningAction`.
> They are callbacks that modify the ScanInstance itself, or one of the
> CompilerInvocations being constructed (either for the TU or for module
> dependencies). In practice, we use this to add caching support - modifying
> invocations to use inputs from a CAS. I chose "actions" based on the fact
> they are acting on the scanner/invocations, rather than simply consuming the
> information. While lookupModuleOutput does not directly modify the
> invcation, it does indirectly via the ModuleDepCollector incorporating its
> results into the invocation. If you're interested, here are our downstream
> callbacks that I would move into this interface:
> https://github.com/apple/llvm-project/blob/next/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h#L49
>
> RE: DependencyScanningAction: I agree it's not ideal that these names are
> "close". On the other hand, they're not too likely to be confused in
> practice since they have very different roles -- the DependencyScanningAction
> is not exposed to clients, and it's a tooling action not something you use to
> control the scan. We also have other "actions" like the FrontendAction we
> create, which again have a different role.
>
> Happy to consider suggestions for a better name!
Thanks, that's helpful. My preference would be slightly towards something more
specific-sounding, maybe `ScanningAdjuster`, `ScannerAndDependencyAdjuster`,
but LGTM as-is too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D144058/new/
https://reviews.llvm.org/D144058
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
