manmanren marked an inline comment as done.
manmanren added inline comments.


================
Comment at: include/clang/Serialization/Module.h:19
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/Module.h"
 #include "clang/Basic/SourceLocation.h"
----------------
rsmith wrote:
> Redundant include?
Definition of ASTFileSignature is moved from Serialization/Module.h to 
Basic/Module.h so we need to have this include to get the definition.


================
Comment at: lib/Serialization/ASTWriter.cpp:1356
+  ASTFileSignature RetCode{{0}};
+  if (WritingModule && Context.getLangOpts().ImplicitModules)
+    RetCode = WriteHash(0);
----------------
rsmith wrote:
> I don't think it makes sense to use `ImplicitModules` to control this. Doing 
> that for the old signature computation was at best a hack.
> 
> If there is no measurable performance difference from the hashing, we should 
> just do it unconditionally in all modes. Otherwise, there should be a 
> separate flag to control it; we should probably force that flag to be enabled 
> when the frontend implicitly builds a module and inserts it into the module 
> cache, but otherwise let the user control it as they see fit.
Use HSOpts.ModulesHashContent to guard the hashing of module file content. This 
flag is set to true when we implicitly build a module.


https://reviews.llvm.org/D27689



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

Reply via email to