[PATCH] D45012: [Modules] Skip adding unused module maps to the dependency file

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

This feature seems reasonable to me.



================
Comment at: lib/Frontend/DependencyFile.cpp:206-223
 class DFGMMCallback : public ModuleMapCallbacks {
   DFGImpl &Parent;
 public:
   DFGMMCallback(DFGImpl &Parent) : Parent(Parent) {}
   void moduleMapFileRead(SourceLocation Loc, const FileEntry &Entry,
                          bool IsSystem) override {
+    if (Parent.skipUnusedModuleMaps())
----------------
I wonder whether it'd be clearer to have two different callbacks for the two behaviors here, rather than one callback that essentially does one of two completely different things.


================
Comment at: lib/Lex/ModuleMap.cpp:728-735
+    // Notify callbacks that we found a module map for the module.
+    if (!M->DefinitionLoc.isInvalid())
+      for (const auto &Cb : Callbacks)
+        Cb->moduleMapFoundForModule(
+            *getContainingModuleMapFile(M), M,
+            SourceMgr.getFileCharacteristic(M->DefinitionLoc) ==
+                SrcMgr::C_System_ModuleMap);
----------------
Does the right thing happen when we load module map information from a PCM file?

I'm also worried that this will fire at the wrong times (eg, more than once per module, or not at all if the module is only ever found by header lookup). Perhaps we should instead trigger the callback when the module is first imported?


Repository:
  rC Clang

https://reviews.llvm.org/D45012