[PATCH] D53522: [Frontend] Include module map header declaration in dependency file output

Volodymyr Sapsai via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 7 19:28:02 PST 2018


vsapsai added inline comments.


================
Comment at: clang/lib/Lex/ModuleMap.cpp:1198-1203
+    // If the header in the module map refers to a symlink, Header.Entry
+    // refers to the actual file. The callback should be notified of both.
+    if (!FullPathAsWritten.empty() &&
+        !FullPathAsWritten.equals(Header.Entry->getName())) {
+      Cb->moduleMapAddHeader(FullPathAsWritten, Mod->IsSystem, Imported);
+    }
----------------
vsapsai wrote:
> It is strange but after removing this part the tests are still passing. I suspect the code is correct but the test allows some roundabout way to add symlink to dependencies. In my experiments only `DFGMMCallback::moduleMapAddHeader` affects the tests. Is it expected?
Looks like you have 3 cases:

1. Add all files in module map to dependencies, even if a file isn't `#include`d anywhere (this turned out to be `link.h`).
2. For `-fmodule-file` replace header files in dependencies with .pcm file (that's what `Imported` achieves).
3. Some scenario with symlinks. Here I haven't found a representative test case.


https://reviews.llvm.org/D53522





More information about the cfe-commits mailing list