vsapsai added inline comments.
================
Comment at: clang/lib/Frontend/DependencyFile.cpp:100
+
+ DepCollector.maybeAddDependency(HeaderPath, /*FromModule=*/false, IsSystem,
+ /*IsModuleFile*/ false,
----------------
This `/*FromModule=*/false` looks confusing and suspicious but seems like we
don't use this argument, so it doesn't really matter and worth cleaning up (out
of scope for this change).
================
Comment at: clang/lib/Frontend/DependencyFile.cpp:105-109
+ void moduleMapAddUmbrellaHeader(FileManager *FileMgr, const FileEntry
*Header,
+ bool IsSystem, bool Imported) override {
+ DepCollectorMMCallbacks::moduleMapAddHeader(Header->getName(), IsSystem,
+ Imported);
+ }
----------------
What is the purpose of this method and the one in `DFGMMCallback`? It looks
correct-ish but after removing this callback, the tests are still passing.
================
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);
+ }
----------------
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?
https://reviews.llvm.org/D53522
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
