[PATCH] Private Headers for Modules

Doug Gregor dgregor at apple.com
Fri Jun 7 15:48:07 PDT 2013


  This is looking really good. Only minor change requests above.


================
Comment at: lib/Lex/PPDirectives.cpp:573-582
@@ +572,12 @@
+        ModuleMap::ModuleHeaderRole Role = SuggestedModule->getRole();
+        // // Check for consistency between the module header role
+        // // as obtained from the lookup and as obtained from the module.
+        // // This check is expensive, so enable it when needed.
+        // SmallVectorImpl<const FileEntry *> &PvtHdrs
+        //     = RequestedModule->PrivateHeaders;
+        // SmallVectorImpl<const FileEntry *>::iterator Look
+	//     = std::find(PvtHdrs.begin(), PvtHdrs.end(), FE);
+        // bool IsPrivate = Look != PvtHdrs.end();
+        // assert((IsPrivate && Role == ModuleMap::PrivateHeader)
+        //        || (!IsPrivate && Role != ModuleMap::PrivateHeader));
+        if (Role == ModuleMap::PrivateHeader) {
----------------
Manuel Klimek wrote:
> This looks unintentionally left as a comment?
If this is really that expensive, it can go behind ENABLE_EXPENSIVE_CHECKS, but we'd rather not have it commented out.

================
Comment at: test/Modules/private1.cpp:8
@@ +7,2 @@
+
+struct use_this client_variable;
----------------
Please merge these three tests into a single ".cpp" file, since there's a nontrivial cost to testing each individual file.

Also, please add an #include "private.h" with an expected-error to test the new diagnostic.


http://llvm-reviews.chandlerc.com/D834



More information about the cfe-commits mailing list