[PATCH] D129389: [clang][deps] Override dependency and serialized diag files for modules

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 accepted this revision.
jansvoboda11 added a comment.
This revision is now accepted and ready to land.

LGTM! Thanks.



================
Comment at: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp:341-342
+      !MDC.OriginalInvocation.getDependencyOutputOpts().OutputFile.empty();
+  // FIXME: HadSerializedDiagnostics and HadDependencyFile should be included in
+  // the context hash since it can affect the command-line.
   MD.ID.ContextHash = MD.BuildInvocation.getModuleHash();
----------------
This is a bit unfortunate but I don't see a better alternative.

Ideally, we would compute the hash with the `.d` and `.dia` paths already filled in. That would ensure the command line we end up reporting to the build system really **does** have the context hash associated with the module. (We'd need to include every field set in `getCanonicalCommandLine()` too.) But for the path lookup, we already need some kind of (currently partial) context hash.


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

https://reviews.llvm.org/D129389