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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to