[PATCH] Private Headers for Modules

[Doug Gregor]

  There are some comments on the code for this specific patch, which I think covers the right first chunk of functionality.

  That said, I'd like to understand better where you're going with this feature. Specifically, is the intent that all declarations that come from private headers be marked "__module_private__", i.e., only accessible within the module itself?


================
Comment at: include/clang/Basic/DiagnosticLexKinds.td:583
@@ -582,1 +582,3 @@
+def error_use_of_private_header_outside_module : Error<
+  "use of private header from outside its module">;
   
----------------
How about including the header name in the diagnostic?

================
Comment at: include/clang/Lex/ModuleMap.h:316
@@ +315,3 @@
+  /// (PrivateHeader) This header is included but private.
+  /// (ExcludedHeader) This header is explicitly excluded from the module.
+  void addHeader(Module *Mod, const FileEntry *Header,
----------------
I think the NormalHeader/PrivateHeader/ExcludedHeader documentation should go on the enumerators up in the enum declaration, because these values aren't specific to the addHeader function.

================
Comment at: lib/Lex/ModuleMap.cpp:571
@@ -569,2 +570,3 @@
+      Mod->PrivateHeaders.push_back(Header);
     Mod->Headers.push_back(Header);
     HeaderInfo.MarkFileModuleHeader(Header);
----------------
This puts private headers into both PrivateHeaders and Headers; I don't think that makes sense. Missing "else"?

================
Comment at: lib/Lex/ModuleMap.cpp:574
@@ -571,3 +573,3 @@
   }
-  Headers[Header] = KnownHeader(Mod, Excluded);
+  Headers[Header] = KnownHeader(Mod, Role == ExcludedHeader);
 }
----------------
I think you'll want to extend KnownHeader to store a ModuleHeaderRole, rather than just a bool. It'll be useful later on... (see comment below).

================
Comment at: lib/Lex/PPDirectives.cpp:556
@@ +555,3 @@
+        // not enough information is coming back with the module.
+        SmallVectorImpl<const FileEntry *> &PvtHdrs
+            = RequestedModule->PrivateHeaders;
----------------
One way to address this inefficiency is to SuggestedModule from a Module** to a KnownHeader*, which will have information about both the module and the how this header is a part of the module. Of course, both findModuleForHeader()'s will need to be updated accordingly, but that'll propagate the information back efficiently.

================
Comment at: lib/Lex/PPDirectives.cpp:563
@@ +562,3 @@
+          Module *IncludingModule = HeaderInfo.findModuleForHeader(CurFileEnt);
+          if (IncludingModule != *SuggestedModule)
+            Diag(FilenameLoc, diag::error_use_of_private_header_outside_module);
----------------
A better way to check this is to check whether (*SuggestedModule)->getTopLevelModule() == getCurrentModule()

================
Comment at: lib/Serialization/ASTReader.cpp:1290
@@ -1289,2 +1289,3 @@
           Reader.getPreprocessor().getHeaderSearchInfo().getModuleMap();
-      ModMap.addHeader(Mod, FileMgr.getFile(key.Filename), /*Excluded=*/false);
+      // FIXME: The following call fails to account for private headers.
+      ModMap.addHeader(Mod, FileMgr.getFile(key.Filename),
----------------
Right. To handle this appropriately, you'll need to serialize another bit into the flags byte indicating whether this is a normal header vs. a private header.


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