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

Bruno Cardoso Lopes via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 23 11:16:23 PDT 2018


bruno added a comment.

Hi Erik, thanks for improving this. Comments below.



================
Comment at: clang/include/clang/Lex/ModuleMap.h:650
   void addHeader(Module *Mod, Module::Header Header,
-                 ModuleHeaderRole Role, bool Imported = false);
+                 ModuleHeaderRole Role, StringRef OrigHeaderName = "",
+                 bool Imported = false);
----------------
While here, can you add a `\param` entry for `Imported`?


================
Comment at: clang/lib/Frontend/DependencyFile.cpp:94
+  void moduleMapAddHeader(StringRef HeaderPath, bool IsSystem) override {
+    if (!llvm::sys::path::is_absolute(HeaderPath))
+      return;
----------------
Can you add a testecase for when `HeaderPath` isn't absolute? 


================
Comment at: clang/lib/Lex/ModuleMap.cpp:1196
+    // to the actual file. The callback should be notified of both.
+
+    if (OrigHeaderName.empty())
----------------
Remove this empty line


Repository:
  rC Clang

https://reviews.llvm.org/D53522





More information about the cfe-commits mailing list