[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